Arik Hadas has uploaded a new change for review. Change subject: core: organize AddVmCommand#canDoAction ......................................................................
core: organize AddVmCommand#canDoAction Refactoring AddVmCommand#canDoAction method, such that it won't use the local variable returnValue to store the result - instead, we return false as soon as we detect a required condition that is false. those changes make the code more readable and compact. Change-Id: Ie4eda8868b50fd4a99462c0a649bba58e52f9bc0 Signed-off-by: Arik Hadas <aha...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java 1 file changed, 35 insertions(+), 30 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/41/15841/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java index c70a1a8..373abf5 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java @@ -280,44 +280,45 @@ @Override protected boolean canDoAction() { - boolean returnValue = true; if (getVmTemplate() == null) { - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST); - return false; + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST); } + if (getVmTemplate().isDisabled()) { return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_IS_DISABLED); } - returnValue = buildAndCheckDestStorageDomains(); - if (returnValue) { - storageToDisksMap = - ImagesHandler.buildStorageToDiskMap(getImagesToCheckDestinationStorageDomains(), - diskInfoDestinationMap); - returnValue = canDoAddVmCommand(); + + if (!buildAndCheckDestStorageDomains()) { + return false; + } + + storageToDisksMap = + ImagesHandler.buildStorageToDiskMap(getImagesToCheckDestinationStorageDomains(), + diskInfoDestinationMap); + if (!canDoAddVmCommand()) { + return false; } String vmName = getParameters().getVm().getName(); if (vmName == null || vmName.isEmpty()) { - returnValue = false; - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NAME_MAY_NOT_BE_EMPTY); - } else { - // check that VM name is not too long - boolean vmNameValidLength = isVmNameValidLength(getParameters().getVm()); - if (!vmNameValidLength) { - returnValue = false; - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NAME_LENGTH_IS_TOO_LONG); - } + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NAME_MAY_NOT_BE_EMPTY); + } + + // check that VM name is not too long + if (!isVmNameValidLength(getParameters().getVm())) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NAME_LENGTH_IS_TOO_LONG); } // check for Vm Payload - if (returnValue && getParameters().getVmPayload() != null) { - returnValue = checkPayload(getParameters().getVmPayload(), - getParameters().getVmStaticData().getIsoPath()); - if (returnValue) { - // we save the content in base64 string - getParameters().getVmPayload().setContent(Base64.encodeBase64String( - getParameters().getVmPayload().getContent().getBytes())); + if (getParameters().getVmPayload() != null) { + if (!checkPayload(getParameters().getVmPayload(), + getParameters().getVmStaticData().getIsoPath())) { + return false; } + + // otherwise, we save the content in base64 string + getParameters().getVmPayload().setContent(Base64.encodeBase64String( + getParameters().getVmPayload().getContent().getBytes())); } // Check that the USB policy is legal @@ -325,13 +326,13 @@ getParameters().getVm().getOs(), getVdsGroup(), getReturnValue().getCanDoActionMessages())) { - returnValue = false; + return false; } // check cpuPinning if the check haven't failed yet - if (returnValue) { - VM vmFromParams = getParameters().getVm(); - returnValue = isCpuPinningValid(vmFromParams.getCpuPinning(), vmFromParams.getStaticData()); + VM vmFromParams = getParameters().getVm(); + if (!isCpuPinningValid(vmFromParams.getCpuPinning(), vmFromParams.getStaticData())) { + return false; } if (getParameters().getVm().isUseHostCpuFlags() @@ -339,7 +340,11 @@ return failCanDoAction(VdcBllMessages.VM_HOSTCPU_MUST_BE_PINNED_TO_HOST); } - return returnValue && checkCpuSockets(); + if (!checkCpuSockets()){ + return false; + } + + return true; } protected boolean checkCpuSockets() { -- To view, visit http://gerrit.ovirt.org/15841 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie4eda8868b50fd4a99462c0a649bba58e52f9bc0 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches