Allon Mureinik has uploaded a new change for review. Change subject: core: ImportVm cleanup: canDoAction_afterCloneVm ......................................................................
core: ImportVm cleanup: canDoAction_afterCloneVm A slight cleanup to canDoAction_afterCloneVm so it uses the early return pattern, which is (a bit) more readable. Change-Id: I5c1eba4ca3e39b4b3ed2142e9e0a964bda7e6a5c Signed-off-by: Allon Mureinik <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java 1 file changed, 57 insertions(+), 65 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/22/9622/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java index ccd8a22..0f74cc7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java @@ -136,7 +136,7 @@ initImportClonedVm(); } - retVal = retVal && canDoAction_afterCloneVm(retVal, canDoActionMessages, domainsMap); + retVal = retVal && canDoAction_afterCloneVm(canDoActionMessages, domainsMap); if (!retVal) { addCanDoActionMessage(VdcBllMessages.VAR__ACTION__IMPORT); @@ -273,106 +273,98 @@ canDoActionMessages); } - private boolean canDoAction_afterCloneVm(boolean retVal, - List<String> canDoActionMessages, - Map<Guid, storage_domains> domainsMap) { + private boolean canDoAction_afterCloneVm(List<String> canDoActionMessages, Map<Guid, storage_domains> domainsMap) { VM vm = getParameters().getVm(); + // check that the imported vm guid is not in engine - if (retVal) { - VmStatic duplicateVm = getVmStaticDAO().get(getVm().getId()); - if (duplicateVm != null) { - addCanDoActionMessage(VdcBllMessages.VM_CANNOT_IMPORT_VM_EXISTS); - addCanDoActionMessage(String.format("$VmName %1$s", duplicateVm.getvm_name())); - retVal = false; - } + VmStatic duplicateVm = getVmStaticDAO().get(getVm().getId()); + if (duplicateVm != null) { + addCanDoActionMessage(VdcBllMessages.VM_CANNOT_IMPORT_VM_EXISTS); + addCanDoActionMessage(String.format("$VmName %1$s", duplicateVm.getvm_name())); + return false; } setVmTemplateId(getVm().getvmt_guid()); - if (retVal) { - if (!templateExists() || !checkTemplateInStorageDomain() || !checkImagesGUIDsLegal() || !canAddVm()) { - retVal = false; - } + if (!templateExists() || !checkTemplateInStorageDomain() || !checkImagesGUIDsLegal() || !canAddVm()) { + return false; } - if (retVal && !VmTemplateHandler.BlankVmTemplateId.equals(getVm().getvmt_guid()) && getVmTemplate() != null + + if (!VmTemplateHandler.BlankVmTemplateId.equals(getVm().getvmt_guid()) + && getVmTemplate() != null && getVmTemplate().getstatus() == VmTemplateStatus.Locked) { - addCanDoActionMessage(VdcBllMessages.VM_TEMPLATE_IMAGE_IS_LOCKED); - retVal = false; + return failCanDoAction(VdcBllMessages.VM_TEMPLATE_IMAGE_IS_LOCKED); } - if (retVal && getParameters().getCopyCollapse() && vm.getDiskMap() != null) { + + if (getParameters().getCopyCollapse() && vm.getDiskMap() != null) { for (Disk disk : vm.getDiskMap().values()) { if (disk.getDiskStorageType() == DiskStorageType.IMAGE) { - DiskImage key = (DiskImage) getVm().getDiskMap() - .get(disk.getId()); - if (key != null) { - retVal = - ImagesHandler.CheckImageConfiguration(domainsMap.get(imageToDestinationDomainMap.get(key.getId())) - .getStorageStaticData(), - (DiskImageBase) disk, - canDoActionMessages); - if (!retVal) { - break; + DiskImage key = (DiskImage) getVm().getDiskMap().get(disk.getId()); + + if (key != null) { + if (!ImagesHandler.CheckImageConfiguration(domainsMap.get(imageToDestinationDomainMap.get(key.getId())) + .getStorageStaticData(), + (DiskImageBase) disk, + canDoActionMessages)) { + return false; + } } - } } } } + // if collapse true we check that we have the template on source // (backup) domain - if (retVal && getParameters().getCopyCollapse() && !templateExistsOnExportDomain()) { + if (getParameters().getCopyCollapse() && !templateExistsOnExportDomain()) { addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_IMPORTED_TEMPLATE_IS_MISSING); addCanDoActionMessage(String.format("$DomainName %1$s", getStorageDomainStaticDAO().get(getParameters().getSourceDomainId()).getstorage_name())); - retVal = false; + return false; } - if (retVal) { - boolean inCluster = false; - List<VDSGroup> groups = getVdsGroupDAO().getAllForStoragePool( - getParameters().getStoragePoolId()); - for (VDSGroup group : groups) { - if (group.getId().equals(getParameters().getVdsGroupId())) { - inCluster = true; - break; - } - } - if (!inCluster) { - addCanDoActionMessage(VdcBllMessages.VDS_CLUSTER_IS_NOT_VALID); - retVal = false; - } - } - if (retVal) { - Map<storage_domains, Integer> domainMap = getSpaceRequirementsForStorageDomains(imageList); - - for (Map.Entry<storage_domains, Integer> entry : domainMap.entrySet()) { - retVal = StorageDomainSpaceChecker.hasSpaceForRequest(entry.getKey(), entry.getValue()); - if (!retVal) { - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW); - break; - } + boolean inCluster = false; + List<VDSGroup> groups = getVdsGroupDAO().getAllForStoragePool(getParameters().getStoragePoolId()); + for (VDSGroup group : groups) { + if (group.getId().equals(getParameters().getVdsGroupId())) { + inCluster = true; + break; } } - if (retVal && Config.<Boolean> GetValue(ConfigValues.LimitNumberOfNetworkInterfaces, + if (!inCluster) { + return failCanDoAction(VdcBllMessages.VDS_CLUSTER_IS_NOT_VALID); + } + + Map<storage_domains, Integer> domainMap = getSpaceRequirementsForStorageDomains(imageList); + + for (Map.Entry<storage_domains, Integer> entry : domainMap.entrySet()) { + if (!StorageDomainSpaceChecker.hasSpaceForRequest(entry.getKey(), entry.getValue())) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW); + } + } + + if (Config.<Boolean> GetValue(ConfigValues.LimitNumberOfNetworkInterfaces, getVdsGroup().getcompatibility_version().toString())) { // check that we have no more then 8 interfaces (kvm limitation in // version 2.x) if (!VmCommand.validateNumberOfNics(vm.getInterfaces(), null)) { - addCanDoActionMessage(VdcBllMessages.NETWORK_INTERFACE_EXITED_MAX_INTERFACES); - retVal = false; + return failCanDoAction(VdcBllMessages.NETWORK_INTERFACE_EXITED_MAX_INTERFACES); } } // Check that the USB policy is legal - if (retVal) { - VmHandler.updateImportedVmUsbPolicy(vm.getStaticData()); - retVal = VmHandler.isUsbPolicyLegal(vm.getusb_policy(), vm.getos(), getVdsGroup(), getReturnValue().getCanDoActionMessages()); + VmHandler.updateImportedVmUsbPolicy(vm.getStaticData()); + if (!VmHandler.isUsbPolicyLegal(vm.getusb_policy(), + vm.getos(), + getVdsGroup(), + getReturnValue().getCanDoActionMessages())) { + return false; } - if (retVal) { - retVal = validateMacAddress(getVm().getInterfaces()); + if (!validateMacAddress(getVm().getInterfaces())) { + return false; } - return retVal; + return true; } private boolean templateExistsOnExportDomain() { -- To view, visit http://gerrit.ovirt.org/9622 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5c1eba4ca3e39b4b3ed2142e9e0a964bda7e6a5c 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
