Liron Ar has uploaded a new change for review. Change subject: core: fail deletion of template/template disk with derived disks ......................................................................
core: fail deletion of template/template disk with derived disks *When a template disk has derived disks on the selected domain for deletion, the deletion should fail with a proper message. *When deleting a template with derived disks, the deletion should fail with proper message. Change-Id: Ifca48993f40f3c5f7b603f33add7049e78e6c4e9 Bug-Url: https://bugzilla.redhat.com/953492 Signed-off-by: Liron Aravot <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.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 9 files changed, 100 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/76/26076/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java index 6106e1b..1dfc686 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java @@ -180,7 +180,7 @@ return failCanDoAction(VdcBllMessages.VM_TEMPLATE_IMAGE_LAST_DOMAIN); } - if (!checkDerivedVmFromTemplateExists(diskImage)){ + if (!checkDerivedVmFromTemplateExists(diskImage) || !checkDerivedDisksFromDiskNotExist(diskImage)){ return false; } @@ -197,6 +197,14 @@ return true; } + private DiskImagesValidator createDiskImagesValidator(DiskImage disk) { + return new DiskImagesValidator(Arrays.asList(disk)); + } + + private boolean checkDerivedDisksFromDiskNotExist(DiskImage diskImage) { + return validate(createDiskImagesValidator(diskImage).diskImagesHaveNoDerivedDisks(getParameters().getStorageDomainId())); + } + private List<String> getNamesOfDerivedVmsFromTemplate(DiskImage diskImage) { List<String> result = new ArrayList<String>(); for (VM vm : getVmDAO().getAllWithTemplate(getVmTemplateId())) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateCommand.java index 30eb11d..9256996 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateCommand.java @@ -14,6 +14,7 @@ import org.ovirt.engine.core.bll.quota.QuotaStorageConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageDependent; import org.ovirt.engine.core.bll.storage.StoragePoolValidator; +import org.ovirt.engine.core.bll.validator.DiskImagesValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.VdcActionType; @@ -167,7 +168,12 @@ return false; } } - return true; + + return validate(checkNoDisksBasedOnTemplateDisks()); + } + + private ValidationResult checkNoDisksBasedOnTemplateDisks() { + return new DiskImagesValidator(imageTemplates).diskImagesHaveNoDerivedDisks(null); } private void fetchImageTemplates() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java index 5897326..e94769f 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java @@ -14,6 +14,7 @@ import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.dao.DiskImageDAO; import org.ovirt.engine.core.dao.SnapshotDao; import org.ovirt.engine.core.dao.VmDAO; import org.ovirt.engine.core.dao.VmDeviceDAO; @@ -154,6 +155,33 @@ return ValidationResult.VALID; } + /** + * checks that the given disks has no derived disks on the given storage domain. + * if the provided storage domain id is null, it will be checked that there are no + * derived disks on any storage domain. + */ + public ValidationResult diskImagesHaveNoDerivedDisks(Guid storageDomainId) { + List<String> disksInfo = new LinkedList<>(); + for (DiskImage diskImage : diskImages) { + if (diskImage.getVmEntityType() != null && diskImage.getVmEntityType().isTemplateType()) { + List<DiskImage> basedDisks = getDiskImageDAO().getAllSnapshotsForParent(diskImage.getImageId()); + for (DiskImage basedDisk : basedDisks) { + if (storageDomainId == null || basedDisk.getStorageIds().contains(storageDomainId)) { + disksInfo.add(String.format("%s (%s) ", basedDisk.getDiskAlias(), basedDisk.getId())); + } + } + } + } + + if (!disksInfo.isEmpty()) { + return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DETECTED_DERIVED_DISKS, + String.format("$disksInfo %s", + String.format(StringUtils.join(disksInfo, "%n")))); + } + + return ValidationResult.VALID; + } + private DbFacade getDbFacade() { return DbFacade.getInstance(); } @@ -169,4 +197,8 @@ protected SnapshotDao getSnapshotDAO() { return getDbFacade().getSnapshotDao(); } + + protected DiskImageDAO getDiskImageDAO() { + return getDbFacade().getDiskImageDao(); + } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java index a62b4bf..df7051e 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java @@ -15,6 +15,7 @@ import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.isValid; import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.replacements; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -32,8 +33,10 @@ import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VmDevice; import org.ovirt.engine.core.common.businessentities.VmDeviceId; +import org.ovirt.engine.core.common.businessentities.VmEntityType; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dao.DiskImageDAO; import org.ovirt.engine.core.dao.SnapshotDao; import org.ovirt.engine.core.dao.VmDAO; import org.ovirt.engine.core.dao.VmDeviceDAO; @@ -58,6 +61,9 @@ @Mock private SnapshotDao snapshotDao; + @Mock + private DiskImageDAO diskImageDao; + @Before public void setUp() { disk1 = createDisk(); @@ -68,6 +74,7 @@ doReturn(vmDAO).when(validator).getVmDAO(); doReturn(vmDeviceDAO).when(validator).getVmDeviceDAO(); doReturn(snapshotDao).when(validator).getSnapshotDAO(); + doReturn(diskImageDao).when(validator).getDiskImageDAO(); } private static DiskImage createDisk() { @@ -218,6 +225,44 @@ } @Test + public void diskImagesHasDerivedDisksNoStorageDomainSpecifiedSuccess() { + disk1.setVmEntityType(VmEntityType.TEMPLATE); + when(diskImageDao.getAllSnapshotsForParent(disk1.getImageId())).thenReturn(Collections.<DiskImage>emptyList()); + assertThat(validator.diskImagesHaveNoDerivedDisks(null), + isValid()); + } + + @Test + public void diskImagesHasDerivedDisksNoStorageDomainSpecifiedFailure() { + disk1.setVmEntityType(VmEntityType.TEMPLATE); + when(diskImageDao.getAllSnapshotsForParent(disk1.getImageId())).thenReturn(Arrays.asList(disk2)); + assertThat(validator.diskImagesHaveNoDerivedDisks(null), + failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DETECTED_DERIVED_DISKS)); + } + + @Test + public void diskImagesHasDerivedDisksOnStorageDomain() { + Guid storageDomainId = Guid.Empty; + disk1.setVmEntityType(VmEntityType.TEMPLATE); + ArrayList<Guid> storageDomainIds = new ArrayList<>(); + storageDomainIds.add(storageDomainId); + disk2.setStorageIds(storageDomainIds); + when(diskImageDao.getAllSnapshotsForParent(disk1.getImageId())).thenReturn(Arrays.asList(disk2)); + assertThat(validator.diskImagesHaveNoDerivedDisks(storageDomainId), + failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DETECTED_DERIVED_DISKS)); + } + + @Test + public void diskImagesNoDerivedDisksOnStorageDomain() { + disk1.setVmEntityType(VmEntityType.TEMPLATE); + ArrayList<Guid> storageDomainIds = new ArrayList<>(); + storageDomainIds.add(Guid.newGuid()); + disk2.setStorageIds(storageDomainIds); + when(diskImageDao.getAllSnapshotsForParent(disk1.getImageId())).thenReturn(Arrays.asList(disk2)); + assertThat(validator.diskImagesHaveNoDerivedDisks(Guid.Empty), isValid()); + } + + @Test public void diskImagesSnapshotsNotAttachedToOtherVmsOneDiskSnapshotAttached() { List<VmDevice> createdDevices = prepareForCheckingIfDisksSnapshotsAttachedToOtherVms(); createdDevices.get(1).setSnapshotId(Guid.newGuid()); 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 30f1da5..7fee305 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 @@ -376,6 +376,7 @@ // "Cannot delete the template, there are desktop(s) created from template"; VMT_CANNOT_REMOVE_DETECTED_DERIVED_VM(ErrorType.CONFLICT), VMT_CANNOT_REMOVE_BASE_WITH_VERSIONS(ErrorType.CONFLICT), + ACTION_TYPE_FAILED_DETECTED_DERIVED_DISKS(ErrorType.CONFLICT), VMT_CANNOT_CREATE_TEMPLATE_FROM_DOWN_VM(ErrorType.CONFLICT), VMT_CANNOT_REMOVE_BLANK_TEMPLATE(ErrorType.CONFLICT), VMT_CANNOT_EDIT_BLANK_TEMPLATE(ErrorType.CONFLICT), 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 df003ba..db39256 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -11,6 +11,7 @@ MAC_POOL_NOT_ENOUGH_MAC_ADDRESSES=Not enough MAC addresses left in MAC Address Pool. VMT_CANNOT_REMOVE_DETECTED_DERIVED_VM=Cannot delete Template. Template is being used by the following VMs: ${vmsList}. VMT_CANNOT_REMOVE_BASE_WITH_VERSIONS=Cannot delete Base Template that has Template Versions, please first remove all Template Versions for this Template: ${versionsList}. +ACTION_TYPE_FAILED_DETECTED_DERIVED_DISKS=Cannot ${action} ${type}. The following Disk(s) are based on it: \n ${disksInfo}. VMT_CANNOT_REMOVE_DOMAINS_LIST_MISMATCH=Cannot delete Template. The Template does not exist on the following Storage Domains: ${domainsList}.\nEither verify that Template exists on all Storage Domains listed on the domains list,\nor do not send domains list in order to delete all instances of the Template from the system. ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Image does not exist. ACTION_TYPE_FAILED_VM_SNAPSHOT_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Snapshot does not 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 7e07159..f6bb69f 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 @@ -43,6 +43,9 @@ @DefaultStringValue("Cannot delete Base Template that has Template Versions, please first remove all Template Versions for this Template: ${versionsList}.") String VMT_CANNOT_REMOVE_BASE_WITH_VERSIONS(); + @DefaultStringValue("Cannot ${action} ${type}. The following Disk(s) are based on it: \n ${disksInfo}.") + String ACTION_TYPE_FAILED_DETECTED_DERIVED_DISKS(); + @DefaultStringValue("Cannot delete Template. The Template does not exist on the following Storage Domains: ${domainsList}.\nEither verify that Template exists on all Storage Domains listed on the domains list,\nor do not send domains list in order to delete all instances of the Template from the system.") String VMT_CANNOT_REMOVE_DOMAINS_LIST_MISMATCH(); 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 5fd729e..adcc612 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 @@ -11,6 +11,7 @@ MAC_POOL_NOT_ENOUGH_MAC_ADDRESSES=Not enough MAC addresses left in MAC Address Pool. VMT_CANNOT_REMOVE_DETECTED_DERIVED_VM=Cannot delete Template. Template is being used by the following VMs: ${vmsList}. VMT_CANNOT_REMOVE_BASE_WITH_VERSIONS=Cannot delete Base Template that has Template Versions, please first remove all Template Versions for this Template: ${versionsList}. +ACTION_TYPE_FAILED_DETECTED_DERIVED_DISKS=Cannot ${action} ${type}. The following Disk(s) are based on it: \n ${disksInfo}. VMT_CANNOT_REMOVE_DOMAINS_LIST_MISMATCH=Cannot delete Template. The Template does not exist on the following Storage Domains: ${domainsList}.\nEither verify that Template exists on all Storage Domains listed on the domains list,\nor do not send domains list in order to delete all instances of the Template from the system. ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Image does not exist. ACTION_TYPE_FAILED_VM_SNAPSHOT_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Snapshot does not 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 acd47f2..556e879 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 @@ -11,6 +11,7 @@ MAC_POOL_NOT_ENOUGH_MAC_ADDRESSES=Not enough MAC addresses left in MAC Address Pool. VMT_CANNOT_REMOVE_DETECTED_DERIVED_VM=Cannot delete Template. Template is being used by the following VMs: ${vmsList}. VMT_CANNOT_REMOVE_BASE_WITH_VERSIONS=Cannot delete Base Template that has Template Versions, please first remove all Template Versions for this Template: ${versionsList}. +ACTION_TYPE_FAILED_DETECTED_DERIVED_DISKS=Cannot ${action} ${type}. The following Disk(s) are based on it: \n ${disksInfo}. VMT_CANNOT_REMOVE_DOMAINS_LIST_MISMATCH=Cannot delete Template. The Template does not exist on the following Storage Domains: ${domainsList}.\nEither verify that Template exists on all Storage Domains listed on the domains list,\nor do not send domains list in order to delete all instances of the Template from the system. ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Image does not exist. ACTION_TYPE_FAILED_VM_SNAPSHOT_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Snapshot does not exist. -- To view, visit http://gerrit.ovirt.org/26076 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ifca48993f40f3c5f7b603f33add7049e78e6c4e9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Liron Ar <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
