Daniel Erez has uploaded a new change for review. Change subject: core: remove snapshot - validate snapshot type ......................................................................
core: remove snapshot - validate snapshot type * Validate snapshot type on RemoveSnapshotCommand (e.g. Active snapshots cannot be removed). * Removed the obsolete validation based on a representative image. * Added appropriate tests accordingly. Change-Id: I90d9144e9d0f7db4f165b7d6945bb27d387c4f66 Bug-Url: https://bugzilla.redhat.com/1140207 Signed-off-by: Daniel Erez <de...@redhat.com> --- 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 M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 7 files changed, 29 insertions(+), 23 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/88/32888/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 e280758..d9aa8d1 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 @@ -305,6 +305,7 @@ !validateVmNotDuringSnapshot() || !validateVmNotInPreview() || !validateSnapshotExists() || + !validateSnapshotType() || (FeatureSupported.liveMerge(getVm().getVdsGroupCompatibilityVersion()) ? (!validate(vmValidator.vmQualifiedForSnapshotMerge()) || !validate(vmValidator.vmHostCanLiveMerge())) @@ -322,11 +323,6 @@ // check that we are not deleting the template if (!validateImageNotInTemplate()) { return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_REMOVE_IMAGE_TEMPLATE); - } - - // check that we are not deleting the vm working snapshot - if (!validateImageNotActive()) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_REMOVE_ACTIVE_IMAGE); } if (!validateStorageDomains()) { @@ -397,6 +393,12 @@ return validate(createSnapshotValidator().snapshotExists(getVmId(), getParameters().getSnapshotId())); } + protected boolean validateSnapshotType() { + Snapshot snapshot = getSnapshotDao().get(getParameters().getSnapshotId()); + return validate(createSnapshotValidator().snapshotTypeSupported(snapshot, + Collections.singletonList(Snapshot.SnapshotType.REGULAR))); + } + protected boolean validateImages() { List<DiskImage> imagesToValidate = ImagesHandler.filterImageDisks(getDiskDao().getAllForVm(getVmId()), true, false, true); @@ -408,10 +410,6 @@ protected boolean validateImageNotInTemplate() { return getVmTemplateDAO().get(getRepresentativeSourceImageId()) == null; - } - - protected boolean validateImageNotActive() { - return getDiskImageDao().get(getRepresentativeSourceImageId()) == null; } private boolean hasImages() { 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 66d7466..b2549a1 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 @@ -23,6 +23,8 @@ import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.action.RemoveSnapshotParameters; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.Snapshot; +import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StorageDomainType; @@ -36,6 +38,7 @@ import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.Version; import org.ovirt.engine.core.dao.DiskImageDAO; +import org.ovirt.engine.core.dao.SnapshotDao; import org.ovirt.engine.core.dao.StorageDomainDAO; import org.ovirt.engine.core.dao.StoragePoolDAO; import org.ovirt.engine.core.dao.VmTemplateDAO; @@ -65,6 +68,8 @@ private StoragePoolDAO spDao; @Mock + private SnapshotDao snapshotDao; + private SnapshotsValidator snapshotValidator; private VmValidator vmValidator; @@ -87,12 +92,15 @@ doReturn(vmTemplateDAO).when(cmd).getVmTemplateDAO(); doReturn(diskImageDAO).when(cmd).getDiskImageDao(); doReturn(sdDAO).when(cmd).getStorageDomainDAO(); - doReturn(snapshotValidator).when(cmd).createSnapshotValidator(); + doReturn(snapshotDao).when(cmd).getSnapshotDao(); mockVm(); vmValidator = spy(new VmValidator(cmd.getVm())); doReturn(ValidationResult.VALID).when(vmValidator).vmNotHavingDeviceSnapshotsAttachedToOtherVms(anyBoolean()); doReturn(vmValidator).when(cmd).createVmValidator(any(VM.class)); doReturn(STORAGE_POOLD_ID).when(cmd).getStoragePoolId(); + mockSnapshot(SnapshotType.REGULAR); + snapshotValidator = spy(new SnapshotsValidator()); + doReturn(snapshotValidator).when(cmd).createSnapshotValidator(); mockConfigSizeDefaults(); } @@ -103,6 +111,13 @@ vm.setStoragePoolId(STORAGE_POOLD_ID); vm.setVdsGroupCompatibilityVersion(Version.v3_5); doReturn(vm).when(cmd).getVm(); + } + + private void mockSnapshot(SnapshotType snapshotType) { + Snapshot snapshot = new Snapshot(); + snapshot.setId(cmd.getParameters().getSnapshotId()); + snapshot.setType(snapshotType); + doReturn(snapshot).when(snapshotDao).get(snapshot.getId()); } private void mockConfigSizeRequirements(int requiredSpaceBufferInGB) { @@ -132,15 +147,15 @@ } @Test - public void testValidateImageNotActiveTrue() { - when(diskImageDAO.get(mockSourceImage())).thenReturn(null); - assertTrue("validation should succeed", cmd.validateImageNotActive()); + public void testValidateSnapshotNotActiveTrue() { + mockSnapshot(SnapshotType.REGULAR); + assertTrue("validation should succeed", cmd.validateSnapshotType()); } @Test - public void testValidateImageNotActiveFalse() { - when(diskImageDAO.get(mockSourceImage())).thenReturn(new DiskImage()); - assertFalse("validation should succeed", cmd.validateImageNotActive()); + public void testValidateSnapshotNotActiveFalse() { + mockSnapshot(SnapshotType.ACTIVE); + assertFalse("validation should fail", cmd.validateSnapshotType()); } @Test diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index a3b983d..d5fdac7 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -220,7 +220,6 @@ ACTION_TYPE_FAILED_NO_HA_VDS(ErrorType.CONFLICT), ACTION_TYPE_FAILED_NO_VDS_AVAILABLE_IN_CLUSTER(ErrorType.CONFLICT), ACTION_TYPE_FAILED_CANNOT_REMOVE_IMAGE_TEMPLATE(ErrorType.CONFLICT), - ACTION_TYPE_FAILED_CANNOT_REMOVE_ACTIVE_IMAGE(ErrorType.CONFLICT), ACTION_TYPE_FAILED_CPU_NOT_FOUND(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_CLUSTER_CAN_NOT_BE_EMPTY(ErrorType.BAD_PARAMETERS), diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index 0ba7011..77e713a 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -243,7 +243,6 @@ - Check your configuration parameters for Host Swap Percentage. ACTION_TYPE_FAILED_NO_VDS_AVAILABLE_IN_CLUSTER=Cannot ${action} ${type}. There is no available operational Host (in UP state) in the relevant Cluster. ACTION_TYPE_FAILED_CANNOT_REMOVE_IMAGE_TEMPLATE=Cannot ${action} ${type}. Removing the Template Snapshot is not allowed. -ACTION_TYPE_FAILED_CANNOT_REMOVE_ACTIVE_IMAGE=Cannot ${action} ${type}. Removing the VM active Snapshot is not allowed. ACTION_TYPE_FAILED_PROBLEM_WITH_CANDIDATE_INFO=Cannot ${action} ${type}. Failed to get data for Import operation.\n\ - Check your Import Domain. ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST=Cannot ${action} ${type}. The relevant Template doesn't exist. diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index 6709876..5585a15 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -658,9 +658,6 @@ @DefaultStringValue("Cannot ${action} ${type}. Removing the Template Snapshot is not allowed.") String ACTION_TYPE_FAILED_CANNOT_REMOVE_IMAGE_TEMPLATE(); - @DefaultStringValue("Cannot ${action} ${type}. Removing the VM active Snapshot is not allowed.") - String ACTION_TYPE_FAILED_CANNOT_REMOVE_ACTIVE_IMAGE(); - @DefaultStringValue("Cannot ${action} ${type}. Failed to get data for Import operation.\n- Check your Import Domain.") String ACTION_TYPE_FAILED_PROBLEM_WITH_CANDIDATE_INFO(); diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index e3b21ec..366a8ab 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -227,7 +227,6 @@ - Check your configuration parameters for Host Swap Percentage. ACTION_TYPE_FAILED_NO_VDS_AVAILABLE_IN_CLUSTER=Cannot ${action} ${type}. There is no available operational Host (in UP state) in the relevant Cluster. ACTION_TYPE_FAILED_CANNOT_REMOVE_IMAGE_TEMPLATE=Cannot ${action} ${type}. Removing the Template Snapshot is not allowed. -ACTION_TYPE_FAILED_CANNOT_REMOVE_ACTIVE_IMAGE=Cannot ${action} ${type}. Removing the VM active Snapshot is not allowed. ACTION_TYPE_FAILED_PROBLEM_WITH_CANDIDATE_INFO=Cannot ${action} ${type}. Failed to get data for Import operation.\n\ - Check your Import Domain. ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST=Cannot ${action} ${type}. The relevant Template doesn't exist. diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 83b64e3..7720ea6 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -240,7 +240,6 @@ - Check your configuration parameters for Host Swap Percentage. ACTION_TYPE_FAILED_NO_VDS_AVAILABLE_IN_CLUSTER=Cannot ${action} ${type}. There is no available operational Host (in UP state) in the relevant Cluster. ACTION_TYPE_FAILED_CANNOT_REMOVE_IMAGE_TEMPLATE=Cannot ${action} ${type}. Removing the Template Snapshot is not allowed. -ACTION_TYPE_FAILED_CANNOT_REMOVE_ACTIVE_IMAGE=Cannot ${action} ${type}. Removing the VM active Snapshot is not allowed. ACTION_TYPE_FAILED_PROBLEM_WITH_CANDIDATE_INFO=Cannot ${action} ${type}. Failed to get data for Import operation.\n\ - Check your Import Domain. ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST=Cannot ${action} ${type}. The relevant Template doesn't exist. -- To view, visit http://gerrit.ovirt.org/32888 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I90d9144e9d0f7db4f165b7d6945bb27d387c4f66 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
