Allon Mureinik has uploaded a new change for review. Change subject: core: RemoveSnapshotCommad: move image validation ......................................................................
core: RemoveSnapshotCommad: move image validation RemoveSnapshotCommand's canDoAction performs several validations only if the VM has images, and several regardless. Since ImagesHandler.PerformImagesChecks originally also validated VM states, it was called regardless of whether the VM has images or not. Since this is no longer the case, it can now be moved to its rightful place. Change-Id: I5e491f97134c3017fd53bfc159cf47f83ba74ae7 Signed-off-by: Allon Mureinik <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java 2 files changed, 8 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/32/12132/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java index 97776bd..24cb3b4 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java @@ -167,12 +167,16 @@ !validateVmNotDuringSnapshot() || !validateVmNotInPreview() || !validateSnapshotExists() || - !validate(new VmValidator(getVm()).vmDown()) || - !validateImagesAndVMStates()) { + !validate(new VmValidator(getVm()).vmDown())) { return false; } if (hasImages()) { + // Check the VM's images + if (!validateImages()) { + return false; + } + // check that we are not deleting the template if (!validateImageNotInTemplate()) { return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_REMOVE_IMAGE_TEMPLATE); @@ -205,9 +209,7 @@ return validate(createSnapshotValidator().snapshotExists(getVmId(), getParameters().getSnapshotId())); } - // TODO: this is a temporary method used until ImagesHandler.PerformImagesChecks will get decoupeld to several tests - // Until then, this method is called and passes hasImages() onwards so the VM validations are done even for diskless VMs - protected boolean validateImagesAndVMStates() { + protected boolean validateImages() { return ImagesHandler.PerformImagesChecks(getReturnValue().getCanDoActionMessages(), getVm().getStoragePoolId(), Guid.Empty, hasImages(), hasImages(), hasImages(), hasImages(), true, true, getDiskDao().getAllForVm(getVmId())); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java index b143162..c65da56 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java @@ -100,7 +100,7 @@ doReturn(ValidationResult.VALID).when(snapshotValidator).vmNotDuringSnapshot(any(Guid.class)); doReturn(ValidationResult.VALID).when(snapshotValidator).vmNotInPreview(any(Guid.class)); doReturn(ValidationResult.VALID).when(snapshotValidator).snapshotExists(any(Guid.class), any(Guid.class)); - doReturn(true).when(cmd).validateImagesAndVMStates(); + doReturn(true).when(cmd).validateImages(); doReturn(vm).when(cmd).getVm(); doReturn(sp).when(spDao).get(spId); doReturn(Collections.emptyList()).when(cmd).getSourceImages(); -- To view, visit http://gerrit.ovirt.org/12132 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5e491f97134c3017fd53bfc159cf47f83ba74ae7 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Allon Mureinik <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
