Arik Hadas has uploaded a new change for review.

Change subject: core: make ClearNonResponsiveVdsVms non-transactive
......................................................................

core: make ClearNonResponsiveVdsVms non-transactive

ClearNonResponsiveVdsVmsCommand basically to the following things:
1. Change all the VMs that ran on the host to DOWN
2. Update the VDS attributes as if no VM is running on the host
3. Run HA VMs that ran on the host

Since this command used to be transactive, the DOWN state on the VMs was
not committed at stage #3. The RunVmCommands triggered in stage #3 still
worked because we were doing the canDoAction checks in the same
transaction (in the multiple action runner). If on the other hand we were
just executing the command one-by-one it would not work because
RunVmCommand is non-transactive so in the canDoAction checks we were
thinking the VMs are in UNKNOWN state - and this is wrong.

There is no real reason for this command to be transactive since after
the user confirmed that the host has been booted, we assume the VMs are
DOWN anyway so the right thing to do is changed their status to DOWN.

The only problematic case of changing the status of the VMs outside of a
transaction scope will be if the host was powered off and we didn't
reach stage #3 (run the HA VMs) after doing stage #1 (this is very
unlikely to happen though) - The VMs will be DOWN and their runningOnVds
field will be black so VURTI won't try to rerun them when the host will
go up. The good side is that the user will be able to run them manually
if the host is not going to restart (as opposed to keep them in UNKNOWN
state).

So for a conclusion: this patch is not ideal, it solved an issue and
creates another minor issue which is very not likely to happen, and
should probably be fixed by another mechanism that will rerun HA VMs
which are DOWN because of an error and AutoStartVmsRunner didn't try
to rerun.

Change-Id: Ia8728e56f21ff6dcf6e103b1d450eecfaa36e809
Signed-off-by: Arik Hadas <aha...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ClearNonResponsiveVdsVmsCommand.java
1 file changed, 10 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/53/26153/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ClearNonResponsiveVdsVmsCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ClearNonResponsiveVdsVmsCommand.java
index 31126f0..a5692c1 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ClearNonResponsiveVdsVmsCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ClearNonResponsiveVdsVmsCommand.java
@@ -20,8 +20,8 @@
 import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
 import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
 import org.ovirt.engine.core.compat.Guid;
-import org.ovirt.engine.core.dal.dbbroker.DbFacade;
 
+@NonTransactiveCommandAttribute
 public class ClearNonResponsiveVdsVmsCommand<T extends VdsActionParameters> 
extends VdsCommand<T> {
 
     /**
@@ -44,7 +44,7 @@
 
     @Override
     protected void executeCommand() {
-        List<VM> vms = 
DbFacade.getInstance().getVmDao().getAllRunningForVds(getVdsId());
+        List<VM> vms = getVmDAO().getAllRunningForVds(getVdsId());
         Collections.sort(vms, Collections.reverseOrder(new VmsComparer()));
         ArrayList<VdcActionParametersBase> runVmParamsList = new 
ArrayList<VdcActionParametersBase>();
         for (VM vm : vms) {
@@ -77,20 +77,19 @@
 
     @Override
     protected boolean canDoAction() {
-        boolean returnValue = true;
         if (getVds() == null) {
-            addCanDoActionMessage(VdcBllMessages.VDS_INVALID_SERVER_ID);
-            returnValue = false;
+            return failCanDoAction(VdcBllMessages.VDS_INVALID_SERVER_ID);
 
-        } else if (hasVMs() && getVds().getStatus() != VDSStatus.NonResponsive 
&& getVds().getStatus() != VDSStatus.Reboot) {
-            
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_CLEAR_VMS_WRONG_STATUS);
-            returnValue = false;
         }
-        return returnValue;
+
+        if (hasVMs() && getVds().getStatus() != VDSStatus.NonResponsive && 
getVds().getStatus() != VDSStatus.Reboot) {
+            return 
failCanDoAction(VdcBllMessages.VDS_CANNOT_CLEAR_VMS_WRONG_STATUS);
+        }
+
+        return true;
     }
 
     private boolean hasVMs() {
-        List<VM> vms = 
DbFacade.getInstance().getVmDao().getAllRunningForVds(getVdsId());
-        return (vms.size() > 0);
+        return !getVmDAO().getAllRunningForVds(getVdsId()).isEmpty();
     }
 }


-- 
To view, visit http://gerrit.ovirt.org/26153
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia8728e56f21ff6dcf6e103b1d450eecfaa36e809
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