Liron Ar has uploaded a new change for review.
Change subject: core: fail deletion of template disk with derived disks
......................................................................
core: fail deletion of template disk with derived disks
When a template disk has derived disks on the selected domain for
deletion, the delete should fail with a proper message.
Change-Id: Ifca48993f40f3c5f7b603f33add7049e78e6c4e9
Signed-off-by: Liron Aravot <lara...@redhat.com>
---
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, 95 insertions(+), 28 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/28/22428/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 c7e874b..fc56f9c 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 368caec..629af6b 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
@@ -4,7 +4,6 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -15,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;
@@ -154,28 +154,11 @@
return false;
}
- return checkNoDisksBasedOnTemplateDisks();
+ return validate(checkNoDisksBasedOnTemplateDisks());
}
- private boolean checkNoDisksBasedOnTemplateDisks() {
- List<String> disksInfo = null;
- for (DiskImage diskImage : imageTemplates) {
- List<DiskImage> basedDisks =
getDiskImageDAO().getAllSnapshotsForParent(diskImage.getImageId());
- for (DiskImage basedDisk : basedDisks) {
- if (disksInfo == null) {
- disksInfo = new LinkedList<>();
- }
- disksInfo.add(String.format("%s (%s) ",
basedDisk.getDiskAlias(), basedDisk.getId()));
- }
- }
-
- if (disksInfo != null) {
- return
failCanDoAction(VdcBllMessages.VMT_CANNOT_REMOVE_DETECTED_DERIVED_DISKS,
- String.format("$disksInfo %s",
- String.format(StringUtils.join(disksInfo, "%n"))));
- }
-
- return true;
+ 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 a0ccb68..eccf567 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;
@@ -128,6 +129,32 @@
return ValidationResult.VALID;
}
+
+ public ValidationResult diskImagesHaveNoDerivedDisks(Guid storageDomainId)
{
+ List<String> disksInfo = null;
+ 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)) {
+ if (disksInfo == null) {
+ disksInfo = new LinkedList<>();
+ }
+ disksInfo.add(String.format("%s (%s) ",
basedDisk.getDiskAlias(), basedDisk.getId()));
+ }
+ }
+ }
+ }
+
+ if (disksInfo != null) {
+ 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();
}
@@ -143,4 +170,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 5a271ff..bcaca58 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() {
@@ -182,6 +189,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 4cc803a..4e76593 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
@@ -353,7 +353,7 @@
// internal const string VMT_CANNOT_REMOVE_DETECTED_DERIVED_VM =
// "Cannot delete the template, there are desktop(s) created from
template";
VMT_CANNOT_REMOVE_DETECTED_DERIVED_VM(ErrorType.CONFLICT),
- VMT_CANNOT_REMOVE_DETECTED_DERIVED_DISKS(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 027402f..8f7974e 100644
---
a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
+++
b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
@@ -10,7 +10,7 @@
IRS_RESPONSE_ERROR=Storage Manager response error.
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_DETECTED_DERIVED_DISKS=Cannot delete Template. The following
Disk(s) are based on the Template disk(s): \n ${disksInfo}
+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 598dead..d6522ef 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
@@ -40,8 +40,8 @@
@DefaultStringValue("Cannot delete Template. Template is being used by the
following VMs: ${vmsList}.")
String VMT_CANNOT_REMOVE_DETECTED_DERIVED_VM();
- @DefaultStringValue("Cannot delete Template. The following Disk(s) are
based on the Template disk(s): \n ${disksInfo}")
- String VMT_CANNOT_REMOVE_DETECTED_DERIVED_DISKS();
+ @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 8b6fc7b..c63e704 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
@@ -10,7 +10,7 @@
IRS_RESPONSE_ERROR=Storage Manager response error.
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_DETECTED_DERIVED_DISKS=Cannot delete Template. The following
Disk(s) are based on the Template disk(s): \n ${disksInfo}
+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 8b8047d..eee490b 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
@@ -10,7 +10,7 @@
IRS_RESPONSE_ERROR=Storage Manager response error.
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_DETECTED_DERIVED_DISKS=Cannot delete Template. The following
Disk(s) are based on the Template disk(s): \n ${disksInfo}
+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/22428
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifca48993f40f3c5f7b603f33add7049e78e6c4e9
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <lara...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
