Arik Hadas has uploaded a new change for review. Change subject: core: organize RemoveDiskCommand#canRemoveTemplateDisk ......................................................................
core: organize RemoveDiskCommand#canRemoveTemplateDisk 1. Return false as soon as we detect condition that is not met, instead of using the 'retValue' variable 2. Extract some code to seperate method in order to make this method more concise and readable Change-Id: I7ddc1e6abd85c61ac1486cdd60744124ebfb27b9 Signed-off-by: Arik Hadas <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java 1 file changed, 35 insertions(+), 26 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/76/15876/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 5493719..c023f09 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 @@ -170,40 +170,49 @@ } private boolean canRemoveTemplateDisk() { - boolean retValue = true; - DiskImage diskImage = getDiskImage(); if (getVmTemplate().getStatus() == VmTemplateStatus.Locked) { - retValue = false; - addCanDoActionMessage(VdcBllMessages.VM_TEMPLATE_IMAGE_IS_LOCKED); + return failCanDoAction(VdcBllMessages.VM_TEMPLATE_IMAGE_IS_LOCKED); } - if (retValue && diskImage.getStorageIds().size() == 1) { - retValue = false; - addCanDoActionMessage(VdcBllMessages.VM_TEMPLATE_IMAGE_LAST_DOMAIN); + + DiskImage diskImage = getDiskImage(); + + if (diskImage.getStorageIds().size() == 1) { + return failCanDoAction(VdcBllMessages.VM_TEMPLATE_IMAGE_LAST_DOMAIN); } - if (retValue) { - List<String> problematicVmNames = new ArrayList<String>(); - List<VM> vms = DbFacade.getInstance().getVmDao().getAllWithTemplate(getVmTemplateId()); - for (VM vm : vms) { - List<Disk> vmDisks = DbFacade.getInstance().getDiskDao().getAllForVm(vm.getId()); - for (Disk vmDisk : vmDisks) { - if (vmDisk.getDiskStorageType() == DiskStorageType.IMAGE) { - DiskImage vmDiskImage = (DiskImage) vmDisk; - if (vmDiskImage.getImageTemplateId().equals(diskImage.getImageId())) { - if (vmDiskImage.getStorageIds().contains(getParameters().getStorageDomainId())) { - retValue = false; - problematicVmNames.add(vm.getName()); - } - break; + + if (!checkDerivedVmFromTemplateExists(diskImage)){ + return false; + } + + return true; + } + + private boolean checkDerivedVmFromTemplateExists(DiskImage diskImage) { + List<String> vmNames = getNamesOfDerivedVmsFromTemplate(diskImage); + if (!vmNames.isEmpty()) { + addCanDoActionMessage(VdcBllMessages.VMT_CANNOT_REMOVE_DETECTED_DERIVED_VM); + addCanDoActionMessage(String.format("$vmsList %1$s", StringUtils.join(vmNames, ","))); + return false; + } + return true; + } + + private List<String> getNamesOfDerivedVmsFromTemplate(DiskImage diskImage) { + List<String> result = new ArrayList<String>(); + for (VM vm : getDbFacade().getVmDao().getAllWithTemplate(getVmTemplateId())) { + for (Disk vmDisk : getDbFacade().getDiskDao().getAllForVm(vm.getId())) { + if (vmDisk.getDiskStorageType() == DiskStorageType.IMAGE) { + DiskImage vmDiskImage = (DiskImage) vmDisk; + if (vmDiskImage.getImageTemplateId().equals(diskImage.getImageId())) { + if (vmDiskImage.getStorageIds().contains(getParameters().getStorageDomainId())) { + result.add(vm.getName()); } + break; } } } - if (!retValue) { - addCanDoActionMessage(VdcBllMessages.VMT_CANNOT_REMOVE_DETECTED_DERIVED_VM); - addCanDoActionMessage(String.format("$vmsList %1$s", StringUtils.join(problematicVmNames, ","))); - } } - return retValue; + return result; } private boolean canRemoveVmImageDisk() { -- To view, visit http://gerrit.ovirt.org/15876 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7ddc1e6abd85c61ac1486cdd60744124ebfb27b9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
