Arik Hadas has uploaded a new change for review. Change subject: core: extract null check from VmRunHandler#canRunVm ......................................................................
core: extract null check from VmRunHandler#canRunVm This patch extract the check whether the given VM is null from the VmRunHandler#canRunVm method. the reason for that is that in both places that call this method it makes sense to check it before making the call. So now as part of the "contract" of this method, the given VM must not be null (and the method's documentation was updated to reflect it). Change-Id: I42fd8274577ba94accf16a4f944de07ca9ea5419 Signed-off-by: Arik Hadas <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java 3 files changed, 38 insertions(+), 24 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/91/12791/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java index a78bc41..f0a929c 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java @@ -660,14 +660,12 @@ return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_FOUND); } - boolean canDoAction; - if (vm.getStatus() == VMStatus.Paused) { // if VM is paused, it was already checked before that it is capable to run - canDoAction = true; + return true; } else { - canDoAction = canRunVm() && validateNetworkInterfaces(); + boolean canDoAction = canRunVm(vm) && validateNetworkInterfaces(); // check for Vm Payload if (canDoAction && getParameters().getVmPayload() != null) { @@ -683,12 +681,13 @@ getVm().setVmPayload(getParameters().getVmPayload()); } } + + return canDoAction; } - return canDoAction; } - protected boolean canRunVm() { - return getVmRunHandler().canRunVm(getVm(), + protected boolean canRunVm(VM vm) { + return getVmRunHandler().canRunVm(vm, getReturnValue().getCanDoActionMessages(), getParameters(), getVdsSelector(), diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java index 376dd27..9220603 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java @@ -248,16 +248,24 @@ protected static boolean canRunPoolVm(Guid vmId, ArrayList<String> messages) { VM vm = DbFacade.getInstance().getVmDao().get(vmId); + if (vm == null) { + messages.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_FOUND.name()); + return false; + } + // TODO: This is done to keep consistency with VmDAO.getById. // It can probably be removed, but that requires some more research VmHandler.updateNetworkInterfacesFromDb(vm); - RunVmParams tempVar = new RunVmParams(vmId); - tempVar.setUseVnc(vm.getVmOs().isLinux() || vm.getVmType() == VmType.Server); - RunVmParams runVmParams = tempVar; - VdsSelector vdsSelector = new VdsSelector(vm, - ((runVmParams.getDestinationVdsId()) != null) ? runVmParams.getDestinationVdsId() - : vm.getDedicatedVmForVds(), true, new VdsFreeMemoryChecker(new NonWaitingDelayer())); + RunVmParams runVmParams = new RunVmParams(vmId); + runVmParams.setUseVnc(vm.getVmOs().isLinux() || vm.getVmType() == VmType.Server); + VdsSelector vdsSelector = + new VdsSelector(vm, + runVmParams.getDestinationVdsId() != null ? + runVmParams.getDestinationVdsId() : vm.getDedicatedVmForVds(), + true, + new VdsFreeMemoryChecker(new NonWaitingDelayer())); + return VmRunHandler.getInstance().canRunVm(vm, messages, runVmParams, diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java index 853787f..273ad02 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java @@ -57,27 +57,34 @@ return instance; } + /** + * This method checks whether the given VM is capable to run. + * + * @param vm not null {@link VM} + * @param message + * @param runParams + * @param vdsSelector + * @param snapshotsValidator + * @param vmPropsUtils + * @return true if the given VM can run with the given properties, false otherwise + */ public boolean canRunVm(VM vm, ArrayList<String> message, RunVmParams runParams, VdsSelector vdsSelector, SnapshotsValidator snapshotsValidator, VmPropertiesUtils vmPropsUtils) { boolean retValue = true; - List<VmPropertiesUtils.ValidationError> validationErrors = null; + List<VmPropertiesUtils.ValidationError> validationErrors = vmPropsUtils.validateVMProperties( + vm.getVdsGroupCompatibilityVersion(), + vm.getStaticData()); - if (vm == null) { - retValue = false; - message.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_FOUND.toString()); - } else if (!(validationErrors = - vmPropsUtils.validateVMProperties(vm.getVdsGroupCompatibilityVersion(), - vm.getStaticData())).isEmpty()) { + if (!validationErrors.isEmpty()) { VmHandler.handleCustomPropertiesError(validationErrors, message); retValue = false; } else { - BootSequence boot_sequence = (runParams.getBootSequence() != null) ? runParams.getBootSequence() : vm - .getDefaultBootSequence(); + BootSequence boot_sequence = (runParams.getBootSequence() != null) ? + runParams.getBootSequence() : vm.getDefaultBootSequence(); Guid storagePoolId = vm.getStoragePoolId(); // Block from running a VM with no HDD when its first boot device is - // HD - // and no other boot devices are configured + // HD and no other boot devices are configured List<Disk> vmDisks = getPluggedDisks(vm); if (boot_sequence == BootSequence.C && vmDisks.size() == 0) { String messageStr = !vmDisks.isEmpty() ? -- To view, visit http://gerrit.ovirt.org/12791 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I42fd8274577ba94accf16a4f944de07ca9ea5419 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
