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