Arik Hadas has uploaded a new change for review. Change subject: core: cleanup in RunVmCommand ......................................................................
core: cleanup in RunVmCommand - Extract code sections that initialize command context for child commands to separate methods - Extract code sections that initialize parameters for child commands to separate methods - Change logs to more standard way - Add static final String member that contains stateless snapshot description instead of having explicit string inside the code - This patch also contains trivial refactoring in VmPoolMonitor Change-Id: I95c423151b2cdb36d5bd50c635439bea2cec816c 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/VmPoolMonitor.java 2 files changed, 73 insertions(+), 68 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/91/22391/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 fdfc147..49c2b70 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 @@ -95,6 +95,7 @@ private boolean memoryFromSnapshotIrrelevant; public static final String ISO_PREFIX = "iso://"; + public static final String STATELESS_SNAPSHOT_DESCRIPTION = "stateless snapshot"; protected RunVmCommand(Guid commandId) { super(commandId); @@ -339,28 +340,18 @@ if (isStatelessSnapshotExistsForVm()) { log.errorFormat( - "RunVmAsStateless - {0} - found stateless snapshots for this vm - skipped creating snapshots.", - getVm().getName()); + "Found stateless snapshots for this vm {0} ({1}), skipped creating snapshots", + getVm().getName(), getVm().getId()); removeVmStatlessImages(); } else { - log.infoFormat("VdcBll.RunVmCommand.RunVmAsStateless - Creating snapshot for stateless vm {0} - {1}", + log.infoFormat("Creating snapshot for stateless vm {0} ({1})", getVm().getName(), getVm().getId()); CreateAllSnapshotsFromVmParameters createAllSnapshotsFromVmParameters = buildCreateSnapshotParameters(); - Map<String, String> values = getVmValuesForMsgResolving(); - - // Creating snapshots as sub step of run stateless - Step createSnapshotsStep = addSubStep(StepEnum.EXECUTING, - StepEnum.CREATING_SNAPSHOTS, values); - - // Add the step as the first step of the new context - ExecutionContext createSnapshotsCtx = new ExecutionContext(); - createSnapshotsCtx.setMonitored(true); - createSnapshotsCtx.setStep(createSnapshotsStep); VdcReturnValueBase vdcReturnValue = getBackend().runInternalAction(VdcActionType.CreateAllSnapshotsFromVm, createAllSnapshotsFromVmParameters, - new CommandContext(createSnapshotsCtx, getCompensationContext(), getLock())); + createContextForStatelessSnapshotCreation()); // setting lock to null in order not to release lock twice setLock(null); @@ -377,20 +368,39 @@ throw new VdcBLLException(VdcBllErrors.IRS_IMAGE_STATUS_ILLEGAL); } getReturnValue().setFault(vdcReturnValue.getFault()); - log.errorFormat("RunVmAsStateless - {0} - failed to create snapshots", getVm().getName()); + log.errorFormat("Failed to create stateless snapshots for vm {0} ({1})", getVm().getName()); } } } + private CommandContext createContextForStatelessSnapshotCreation() { + Map<String, String> values = getVmValuesForMsgResolving(); + // Creating snapshots as sub step of run stateless + Step createSnapshotsStep = addSubStep(StepEnum.EXECUTING, + StepEnum.CREATING_SNAPSHOTS, values); + + // Add the step as the first step of the new context + ExecutionContext createSnapshotsCtx = new ExecutionContext(); + createSnapshotsCtx.setMonitored(true); + createSnapshotsCtx.setStep(createSnapshotsStep); + return new CommandContext(createSnapshotsCtx, getCompensationContext(), getLock()); + } + + private CreateAllSnapshotsFromVmParameters buildCreateSnapshotParametersForEndAction() { + CreateAllSnapshotsFromVmParameters parameters = buildCreateSnapshotParameters(); + parameters.setImagesParameters(getParameters().getImagesParameters()); + return parameters; + } + private CreateAllSnapshotsFromVmParameters buildCreateSnapshotParameters() { - CreateAllSnapshotsFromVmParameters createAllSnapshotsFromVmParameters = - new CreateAllSnapshotsFromVmParameters(getVm().getId(), "stateless snapshot"); - createAllSnapshotsFromVmParameters.setShouldBeLogged(false); - createAllSnapshotsFromVmParameters.setParentCommand(getActionType()); - createAllSnapshotsFromVmParameters.setParentParameters(getParameters()); - createAllSnapshotsFromVmParameters.setEntityInfo(getParameters().getEntityInfo()); - createAllSnapshotsFromVmParameters.setSnapshotType(SnapshotType.STATELESS); - return createAllSnapshotsFromVmParameters; + CreateAllSnapshotsFromVmParameters parameters = + new CreateAllSnapshotsFromVmParameters(getVm().getId(), STATELESS_SNAPSHOT_DESCRIPTION); + parameters.setShouldBeLogged(false); + parameters.setParentCommand(getActionType()); + parameters.setParentParameters(getParameters()); + parameters.setEntityInfo(getParameters().getEntityInfo()); + parameters.setSnapshotType(SnapshotType.STATELESS); + return parameters; } private boolean areDisksLocked(VdcReturnValueBase vdcReturnValue) { @@ -804,41 +814,17 @@ @Override protected void endSuccessfully() { if (isVmRunningStateless()) { - CreateAllSnapshotsFromVmParameters createSnapshotParameters = buildCreateSnapshotParameters(); - createSnapshotParameters.setImagesParameters(getParameters().getImagesParameters()); - getBackend().endAction(VdcActionType.CreateAllSnapshotsFromVm, createSnapshotParameters); + getBackend().endAction(VdcActionType.CreateAllSnapshotsFromVm, buildCreateSnapshotParametersForEndAction()); getParameters().setShouldBeLogged(false); getParameters().setRunAsStateless(false); - ExecutionContext runStatelessVmCtx = new ExecutionContext(); - Step step = getExecutionContext().getStep(); - // Retrieve the job object and its steps as this the endSuccessfully stage of command execution - - // at this is a new instance of the command is used - // (comparing with the execution state) so all information on the job and steps should be retrieved. - Job job = JobRepositoryFactory.getJobRepository().getJobWithSteps(step.getJobId()); - Step executingStep = job.getDirectStep(StepEnum.EXECUTING); - // We would like to to set the run stateless step as substep of executing step - setInternalExecution(true); - // The internal command should be monitored for tasks - runStatelessVmCtx.setMonitored(true); - Step runStatelessStep = - ExecutionHandler.addSubStep(getExecutionContext(), - executingStep, - StepEnum.RUN_STATELESS_VM, - ExecutionMessageDirector.resolveStepMessage(StepEnum.RUN_STATELESS_VM, - getVmValuesForMsgResolving())); - // This is needed in order to end the job upon exextuion of the steps of the child command - runStatelessVmCtx.setShouldEndJob(true); - runStatelessVmCtx.setJob(job); - // Since run stateless step involves invocation of command, we should set the run stateless vm step as - // the "beginning step" of the child command. - runStatelessVmCtx.setStep(runStatelessStep); - setSucceeded(getBackend() - .runInternalAction(getActionType(), getParameters(), new CommandContext(runStatelessVmCtx)) - .getSucceeded()); + + setSucceeded(getBackend().runInternalAction( + getActionType(), getParameters(), createContextForRunStatelessVm()).getSucceeded()); if (!getSucceeded()) { // could not run the vm don't try to run the end action again - log.warnFormat("Could not run the vm {0} on RunVm.EndSuccessfully", getVm().getName()); + log.warnFormat("Could not run vm {0} ({1}) in stateless mode", + getVm().getName(), getVm().getId()); getReturnValue().setEndActionTryAgain(false); } } @@ -849,13 +835,39 @@ } } + private CommandContext createContextForRunStatelessVm() { + Step step = getExecutionContext().getStep(); + // Retrieve the job object and its steps as this the endSuccessfully stage of command execution - + // at this is a new instance of the command is used + // (comparing with the execution state) so all information on the job and steps should be retrieved. + Job job = JobRepositoryFactory.getJobRepository().getJobWithSteps(step.getJobId()); + Step executingStep = job.getDirectStep(StepEnum.EXECUTING); + // We would like to to set the run stateless step as substep of executing step + setInternalExecution(true); + + ExecutionContext runStatelessVmCtx = new ExecutionContext(); + // The internal command should be monitored for tasks + runStatelessVmCtx.setMonitored(true); + Step runStatelessStep = + ExecutionHandler.addSubStep(getExecutionContext(), + executingStep, + StepEnum.RUN_STATELESS_VM, + ExecutionMessageDirector.resolveStepMessage(StepEnum.RUN_STATELESS_VM, + getVmValuesForMsgResolving())); + // This is needed in order to end the job upon execution of the steps of the child command + runStatelessVmCtx.setShouldEndJob(true); + runStatelessVmCtx.setJob(job); + // Since run stateless step involves invocation of command, we should set the run stateless vm step as + // the "beginning step" of the child command. + runStatelessVmCtx.setStep(runStatelessStep); + return new CommandContext(runStatelessVmCtx); + } + @Override protected void endWithFailure() { if (isVmRunningStateless()) { - CreateAllSnapshotsFromVmParameters createSnapshotParameters = buildCreateSnapshotParameters(); - createSnapshotParameters.setImagesParameters(getParameters().getImagesParameters()); VdcReturnValueBase vdcReturnValue = getBackend().endAction(VdcActionType.CreateAllSnapshotsFromVm, - createSnapshotParameters, new CommandContext(getCompensationContext())); + buildCreateSnapshotParametersForEndAction(), new CommandContext(getCompensationContext())); setSucceeded(vdcReturnValue.getSucceeded()); // we are not running the VM, of course, diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolMonitor.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolMonitor.java index a1688a3..cfcb804 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolMonitor.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolMonitor.java @@ -112,14 +112,11 @@ * @return whether or not succeeded to prestart the Vm */ private boolean prestartVm(Guid vmGuid) { - boolean prestartVmSucceeded = false; if (VmPoolCommandBase.canAttachNonPrestartedVmToUser(vmGuid)) { VM vmToPrestart = DbFacade.getInstance().getVmDao().get(vmGuid); - if (runVmAsStateless(vmToPrestart)) { - prestartVmSucceeded = true; - } + return runVmAsStateless(vmToPrestart); } - return prestartVmSucceeded; + return false; } /** @@ -129,18 +126,14 @@ */ private boolean runVmAsStateless(VM vmToRunAsStateless) { log.infoFormat("Running Vm {0} as stateless", vmToRunAsStateless); - boolean prestartingVmSucceeded = false; RunVmParams runVmParams = new RunVmParams(vmToRunAsStateless.getId()); runVmParams.setEntityInfo(new EntityInfo(VdcObjectType.VM, vmToRunAsStateless.getId())); runVmParams.setRunAsStateless(true); VdcReturnValueBase vdcReturnValue = Backend.getInstance().runInternalAction(VdcActionType.RunVm, runVmParams, ExecutionHandler.createInternalJobContext()); - prestartingVmSucceeded = vdcReturnValue.getSucceeded(); - if (prestartingVmSucceeded) { - log.infoFormat("Running Vm {0} as stateless succeeded", vmToRunAsStateless); - } else { - log.infoFormat("Running Vm {0} as stateless failed", vmToRunAsStateless); - } + boolean prestartingVmSucceeded = vdcReturnValue.getSucceeded(); + log.infoFormat("Running Vm {0} as stateless {1}", + vmToRunAsStateless, prestartingVmSucceeded ? "succeeded" : "failed"); return prestartingVmSucceeded; } -- To view, visit http://gerrit.ovirt.org/22391 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I95c423151b2cdb36d5bd50c635439bea2cec816c 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
