Arik Hadas has uploaded a new change for review. Change subject: core: refactor failure to run vm flow - part 4 ......................................................................
core: refactor failure to run vm flow - part 4 Remove MigrateVmCommand#failedToMigrate which was confusing now that we have MigrateVmCommand#runningFailed method. In addition MigrateVmCommand#runningSucceeded is removed. For the changes above, the calls to CommandBase#freeLock was moved to the relevant places in RunVmCommandBase, which are better places for them as they are relevant to all command that extend RunVmCommandBase. Note that the freeLock method handle the case where the lock is null, so it is ok in case the command release the lock at the end of the execute method as well. Change-Id: I42cefc4c5f7974730a2b38f63f310ab975f73622 Signed-off-by: Arik Hadas <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java 3 files changed, 47 insertions(+), 53 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/19/28119/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java index 1811313..59b5401 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java @@ -87,16 +87,6 @@ } } - protected void failedToMigrate() { - try { - determineMigrationFailureForAuditLog(); - runningFailed(); - } - finally { - freeLock(); - } - } - protected void initVdss() { setVdsIdRef(getVm().getRunOnVds()); VDS destVds = getDestinationVds(); @@ -367,27 +357,18 @@ // make Vm property to null in order to refresh it from db setVm(null); + determineMigrationFailureForAuditLog(); + // if vm is up and rerun is called then it got up on the source, try to rerun if (getVm() != null && getVm().getStatus() == VMStatus.Up) { - determineMigrationFailureForAuditLog(); setVdsDestinationId(null); super.rerun(); } else { // vm went down on the destination and source, migration failed. - failedToMigrate(); + runningFailed(); // signal the caller that a rerun was made so that it won't log // the failure message again _isRerun = true; - } - } - - @Override - public void runningSucceded() { - try { - super.runningSucceded(); - } - finally { - freeLock(); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java index fc68e3e..eff6960 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java @@ -49,7 +49,10 @@ */ @Override public void rerun() { - failedToMigrate(); + // make VM property to null in order to refresh it from db + setVm(null); + determineMigrationFailureForAuditLog(); + runningFailed(); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java index 27e10ab..d2f8f26 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java @@ -130,13 +130,18 @@ } protected void runningFailed() { - Backend.getInstance().getResourceManager().RemoveAsyncRunningCommand(getVmId()); - _isRerun = false; - setSucceeded(false); - log(); - processVmPoolOnStopVm(); - ExecutionHandler.setAsyncJob(getExecutionContext(), false); - ExecutionHandler.endJob(getExecutionContext(), false); + try { + Backend.getInstance().getResourceManager().RemoveAsyncRunningCommand(getVmId()); + _isRerun = false; + setSucceeded(false); + log(); + processVmPoolOnStopVm(); + ExecutionHandler.setAsyncJob(getExecutionContext(), false); + ExecutionHandler.endJob(getExecutionContext(), false); + } + finally { + freeLock(); + } } protected void processVmPoolOnStopVm() { @@ -155,31 +160,36 @@ */ @Override public void runningSucceded() { - setSucceeded(true); - setActionReturnValue(VMStatus.Up); - log(); - ExecutionHandler.setAsyncJob(getExecutionContext(), false); - ExecutionHandler.endJob(getExecutionContext(), true); - notifyHostsVmFailed(); + try { + setSucceeded(true); + setActionReturnValue(VMStatus.Up); + log(); + ExecutionHandler.setAsyncJob(getExecutionContext(), false); + ExecutionHandler.endJob(getExecutionContext(), true); + notifyHostsVmFailed(); - if (getVm().getLastVdsRunOn() == null || !getVm().getLastVdsRunOn().equals(getCurrentVdsId())) { - getVm().setLastVdsRunOn(getCurrentVdsId()); + if (getVm().getLastVdsRunOn() == null || !getVm().getLastVdsRunOn().equals(getCurrentVdsId())) { + getVm().setLastVdsRunOn(getCurrentVdsId()); + } + + if (StringUtils.isNotEmpty(getVm().getHibernationVolHandle())) { + removeVmHibernationVolumes(); + + // In order to prevent a race where VdsUpdateRuntimeInfo saves the Vm Dynamic as UP prior to execution of + // this method (which is a part of the cached VM command, + // so the state this method is aware to is RESTORING, in case of RunVmCommand after the VM got suspended. + // In addition, as the boolean return value of HandleHIbernateVm is ignored here, it is safe to set the + // status to up. + getVm().setStatus(VMStatus.Up); + getVm().setHibernationVolHandle(null); + Backend.getInstance() + .getResourceManager() + .RunVdsCommand(VDSCommandType.UpdateVmDynamicData, + new UpdateVmDynamicDataVDSCommandParameters(getCurrentVdsId(), getVm().getDynamicData())); + } } - - if (StringUtils.isNotEmpty(getVm().getHibernationVolHandle())) { - removeVmHibernationVolumes(); - - // In order to prevent a race where VdsUpdateRuntimeInfo saves the Vm Dynamic as UP prior to execution of - // this method (which is a part of the cached VM command, - // so the state this method is aware to is RESTORING, in case of RunVmCommand after the VM got suspended. - // In addition, as the boolean return value of HandleHIbernateVm is ignored here, it is safe to set the - // status to up. - getVm().setStatus(VMStatus.Up); - getVm().setHibernationVolHandle(null); - Backend.getInstance() - .getResourceManager() - .RunVdsCommand(VDSCommandType.UpdateVmDynamicData, - new UpdateVmDynamicDataVDSCommandParameters(getCurrentVdsId(), getVm().getDynamicData())); + finally { + freeLock(); } } -- To view, visit http://gerrit.ovirt.org/28119 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I42cefc4c5f7974730a2b38f63f310ab975f73622 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Arik Hadas <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
