Arik Hadas has uploaded a new change for review. Change subject: core: remove redundant checks when running vm ......................................................................
core: remove redundant checks when running vm Three checks are removed from RunVmCommand#initVm: 1. Check if the VM is null 2. Check if snapshot is being taken for the VM 3. Check if the VM status is not imageLocked or imageIllegal Those checks are already tested in the canDoAction method (by the validations that are carried-out in RunVmValidator), and since the VM is locked in the execute phase, it should be enough. Change-Id: I396b3322c551af3414268f5fe16a1ccaf5ccf1bd 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/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllErrors.java 2 files changed, 49 insertions(+), 71 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/75/22075/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 bcdc1e5..da5b9c8 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 @@ -271,31 +271,28 @@ // configuration option change VmDeviceUtils.updateVmDevices(getVm().getStaticData()); setActionReturnValue(VMStatus.Down); - if (initVm()) { - if (getVm().getStatus() == VMStatus.Paused) { // resume - resumeVm(); - } else { // run vm - if (!_isRerun && Boolean.TRUE.equals(getParameters().getRunAsStateless()) - && getVm().getStatus() != VMStatus.Suspended) { - if (getVm().getDiskList().isEmpty()) { // If there are no snappable disks, there is no meaning for - // running as stateless, log a warning and run normally - warnIfNotAllDisksPermitSnapshots(); - runVm(); - } - else { - statelessVmTreatment(); - } - } else if (!isInternalExecution() && !_isRerun - && getVm().getStatus() != VMStatus.Suspended - && isStatelessSnapshotExistsForVm() - && !isVMPartOfManualPool()) { - removeVmStatlessImages(); - } else { + initVm(); + if (getVm().getStatus() == VMStatus.Paused) { // resume + resumeVm(); + } else { // run vm + if (!_isRerun && Boolean.TRUE.equals(getParameters().getRunAsStateless()) + && getVm().getStatus() != VMStatus.Suspended) { + if (getVm().getDiskList().isEmpty()) { // If there are no snappable disks, there is no meaning for + // running as stateless, log a warning and run normally + warnIfNotAllDisksPermitSnapshots(); runVm(); } + else { + statelessVmTreatment(); + } + } else if (!isInternalExecution() && !_isRerun + && getVm().getStatus() != VMStatus.Suspended + && isStatelessSnapshotExistsForVm() + && !isVMPartOfManualPool()) { + removeVmStatlessImages(); + } else { + runVm(); } - } else { - setActionReturnValue(getVm().getStatus()); } } @@ -564,62 +561,44 @@ } } - protected boolean initVm() { - if (getVm() == null) { - log.warnFormat("ResourceManager::{0}::No such vm (where id = '{1}' )in database", getClass().getName(), - getVmId().toString()); - throw new VdcBLLException(VdcBllErrors.DB_NO_SUCH_VM); - } - if ((getVm().getStatus() == VMStatus.ImageIllegal) || (getVm().getStatus() == VMStatus.ImageLocked)) { - log.warnFormat("ResourceManager::{0}::vm '{1}' has {2}", getClass().getName(), getVmId().toString(), - (getVm().getStatus() == VMStatus.ImageLocked ? "a locked image" : "an illegal image")); - setActionReturnValue(getVm().getStatus()); - return false; - } else if (!getSnapshotsValidator().vmNotDuringSnapshot(getVmId()).isValid()) { - log.warnFormat("ResourceManager::{0}::VM {1} is during snapshot", - getClass().getName(), - getVmId().toString()); - return false; - } else { - VmHandler.updateDisksFromDb(getVm()); - getVm().setKvmEnable(getParameters().getKvmEnable()); - getVm().setRunAndPause(getParameters().getRunAndPause() == null ? getVm().isRunAndPause() : getParameters().getRunAndPause()); - getVm().setAcpiEnable(getParameters().getAcpiEnable()); + protected void initVm() { + VmHandler.updateDisksFromDb(getVm()); + getVm().setKvmEnable(getParameters().getKvmEnable()); + getVm().setRunAndPause(getParameters().getRunAndPause() == null ? getVm().isRunAndPause() : getParameters().getRunAndPause()); + getVm().setAcpiEnable(getParameters().getAcpiEnable()); - // Clear the first user: - getVm().setConsoleUserId(null); - getParameters().setRunAsStateless(getParameters().getRunAsStateless() != null ? getParameters().getRunAsStateless() - : getVm().isStateless()); + // Clear the first user: + getVm().setConsoleUserId(null); + getParameters().setRunAsStateless(getParameters().getRunAsStateless() != null ? getParameters().getRunAsStateless() + : getVm().isStateless()); - getVm().setDisplayType(getParameters().getUseVnc() == null ? - getVm().getDefaultDisplayType() : - // if Use Vnc is not null it means runVM was launch from the run once command, thus - // the VM can run with display type which is different from its default display type + getVm().setDisplayType(getParameters().getUseVnc() == null ? + getVm().getDefaultDisplayType() : + // if Use Vnc is not null it means runVM was launch from the run once command, thus + // the VM can run with display type which is different from its default display type (getParameters().getUseVnc() ? DisplayType.vnc : DisplayType.qxl)); - if (getParameters().getInitializationType() == null) { - // if vm not initialized, use sysprep/cloud-init - if (!getVm().isInitialized()) { - getVm().setInitializationType(osRepository.isWindows(getVm().getVmOsId()) ? - InitializationType.Sysprep : + if (getParameters().getInitializationType() == null) { + // if vm not initialized, use sysprep/cloud-init + if (!getVm().isInitialized()) { + getVm().setInitializationType(osRepository.isWindows(getVm().getVmOsId()) ? + InitializationType.Sysprep : // TODO: we should use cloud init automatically only when cloud init configuration will be available InitializationType.None); - } - } else { - getVm().setInitializationType(getParameters().getInitializationType()); } - - // if we attach floppy we don't need the sysprep - if (!StringUtils.isEmpty(getParameters().getFloppyPath())) { - getVmStaticDAO().update(getVm().getStaticData()); - } - // get what cpu flags should be passed to vdsm according to cluster - // cpu name - getVm().setVdsGroupCpuFlagsData( - CpuFlagsManagerHandler.GetVDSVerbDataByCpuName(getVm().getVdsGroupCpuName(), getVm() - .getVdsGroupCompatibilityVersion())); - return true; + } else { + getVm().setInitializationType(getParameters().getInitializationType()); } + + // if we attach floppy we don't need the sysprep + if (!StringUtils.isEmpty(getParameters().getFloppyPath())) { + getVmStaticDAO().update(getVm().getStaticData()); + } + // get what cpu flags should be passed to vdsm according to cluster + // cpu name + getVm().setVdsGroupCpuFlagsData( + CpuFlagsManagerHandler.GetVDSVerbDataByCpuName(getVm().getVdsGroupCpuName(), getVm() + .getVdsGroupCompatibilityVersion())); } protected boolean getVdsToRunOn() { diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllErrors.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllErrors.java index ecc5953..1955dcb 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllErrors.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllErrors.java @@ -414,7 +414,6 @@ VM_INVALID_SERVER_CLUSTER_ID(5017), VM_TEMPLATE_CANT_LOCATE_DISKS_IN_DB(5018), USER_FAILED_POPULATE_DATA(5019), - DB_NO_SUCH_VM(5020), VDS_FENCE_OPERATION_FAILED(5021), VDS_NETWORK_ERROR(5022), NO_FREE_VM_IN_POOL(5023), -- To view, visit http://gerrit.ovirt.org/22075 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I396b3322c551af3414268f5fe16a1ccaf5ccf1bd 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
