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

Reply via email to