Maor Lipchuk has posted comments on this change.
Change subject: core: revert when failing to export a template
......................................................................
Patch Set 2: (3 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CopyImageGroupCommand.java
Line 191:
removeImageParams.setParentCommand(VdcActionType.RemoveImage);
Line 192:
removeImageParams.setStorageDomainId(getParameters().getStorageDomainId());
Line 193:
removeImageParams.setDbOperationScope(getParameters().getRevertDbOperationScope());
Line 194: removeImageParams.setEntityId(getDestinationImageId());
Line 195:
removeImageParams.setShouldLockImage(getParameters().getShouldLockImageOnRevert());
1) Now every revert will lock the image (except export), what if we will fail
to run removeImage, who is responsible to unlock the image?
2) Please Add empty line before comment
Line 196: // Setting the image as the monitored entity, so there
will not be dependency
Line 197: VdcReturnValueBase returnValue =
Line 198:
checkAndPerformRollbackUsingCommand(VdcActionType.RemoveImage,
removeImageParams);
Line 199: if (returnValue.getSucceeded()) {
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java
Line 18: private boolean addImageDomainMapping;
Line 19: private boolean forceOverride;
Line 20: private NGuid sourceDomainId;
Line 21: private Guid destImageGroupId;
Line 22: private ImageDbOperationScope revertDbOperationScope;
Not related to this patch, but i would suggest re-examine ImageDbOperationScope
None - should be don't do anything
All - Should indicate all image should be deleted
Map - Should be, only mapping should be delted
Line 23: private boolean shouldLockImageOnRevert = true;
Line 24:
Line 25: public MoveOrCopyImageGroupParameters() {
Line 26: }
Line 139: public void setRevertDbOperationScope(ImageDbOperationScope
revertDbOperationScope) {
Line 140: this.revertDbOperationScope = revertDbOperationScope;
Line 141: }
Line 142:
Line 143: public boolean getShouldLockImageOnRevert() {
/s/getShouldLockImageOnRevert/isShouldLockImageOnRevert
also I think that this parameter is redundant. the developer can decide him
self to lock/unlock to image on revert
Line 144: return shouldLockImageOnRevert;
Line 145: }
Line 146:
Line 147: public void setShouldLockImageOnRevert(boolean
shouldLockImageOnRevert) {
--
To view, visit http://gerrit.ovirt.org/15781
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d80c5d15f7321f67d54e7593cb2da1fd7186a25
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
