Allon Mureinik has posted comments on this change.
Change subject: core: Remove image when VDSM returns not exist.
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java
Line 209: // Setting the image as the monitored entity, so there
will not be dependency
Line 210: VdcReturnValueBase returnValue =
Line 211:
checkAndPerformRollbackUsingCommand(VdcActionType.RemoveImage,
removeImageParams);
Line 212: if (returnValue.getFault().getError() ==
VdcBllErrors.ImageDoesNotExistInDomainError
Line 213: && getParameters().getParentCommand() !=
VdcActionType.ImportVm) {
I know this entire command looks like this, but this is disgusting.
Perhaps the time has come to refactor this ridiculous mechanism of checking who
the parent was?
Line 214: endSuccessfully();
Line 215: } else if (returnValue.getSucceeded()) {
Line 216: // Starting to monitor the the tasks - RemoveImage is
an internal command
Line 217: // which adds the taskId on the internal task ID list
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveAllVmImagesCommand.java
Line 64:
Backend.getInstance().runInternalAction(VdcActionType.RemoveImage,
Line 65: tempVar,
Line 66:
ExecutionHandler.createDefaultContexForTasks(getExecutionContext()));
Line 67:
Line 68: if (vdcReturnValue.getFault().getError() ==
VdcBllErrors.ImageDoesNotExistInDomainError) {
I'd put this part in the else condition - lead with the positive instead of an
edge case - it's more readable.
Line 69: imageDoesNotExistsFault =
vdcReturnValue.getFault();
Line 70: } else if (vdcReturnValue.getSucceeded()) {
Line 71: isAllDisksNotExistsInDomain = false;
Line 72:
getReturnValue().getInternalTaskIdList().addAll(vdcReturnValue.getInternalTaskIdList());
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveAllVmTemplateImageTemplatesCommand.java
Line 57: tempVar,
Line 58:
ExecutionHandler.createDefaultContexForTasks(getExecutionContext()));
Line 59:
Line 60: if (vdcReturnValue.getFault().getError() ==
VdcBllErrors.ImageDoesNotExistInDomainError) {
Line 61: imageDoesNotExistsFault =
vdcReturnValue.getFault();
Same comment as previous file.
Line 62: } else if (vdcReturnValue.getSucceeded()) {
Line 63: isAllTemplateDisksNotExistsInDomain = false;
Line 64:
getReturnValue().getInternalTaskIdList().addAll(vdcReturnValue.getInternalTaskIdList());
Line 65: } else {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveTemplateSnapshotCommand.java
Line 18: }
Line 19:
Line 20: @Override
Line 21: protected void executeCommand() {
Line 22: VDSReturnValue vdsReturnValue =
@Maor - I too have trouble following here.
Is there anything here besides formatting? If so - please remove it from this
patch. If not - please elaborate.
Line 23: Backend
Line 24: .getInstance()
Line 25: .getResourceManager()
Line 26: .RunVdsCommand(
--
To view, visit http://gerrit.ovirt.org/12077
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I36915c44e65e20e3ce222683650a562603e4bb05
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: liron aravot <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
