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 <aha...@redhat.com> --- 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/61/24161/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 1b28042..b863529 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(); @@ -363,27 +353,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(); setDestinationVdsId(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 e0d461c..df0c959 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 @@ -47,7 +47,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 18168e7..8d10b59 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 @@ -129,13 +129,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() { @@ -154,31 +159,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/24161 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I42cefc4c5f7974730a2b38f63f310ab975f73622 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