Roy Golan has uploaded a new change for review.

Change subject: core: Prevent StopVm to interleave with other VM locked actions
......................................................................

core: Prevent StopVm to interleave with other VM locked actions

Stop and Shutdown VM commands are calling the DestoryVds command which is 
contending on
the VdsManager lock. The lock is implemented as synchronized (object) -
this means that order isn't kept and we're subjected to JVM's scheduling
descisions so other commands, like RunVm can take the lock and result the Vm to 
stop instead of start

Fix: take an exclusive engine lock on Stop and Shutdown commands

This will also prevent a situation where asyncRunningVms has an entry
although the VM is down.

Bug-Url: https://bugzilla.redhat.com/1038351
Change-Id: I6109ef0ea4869b59a93be6fef4b0c3cff50da62e
Signed-off-by: Roy Golan <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ShutdownVmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java
3 files changed, 20 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/76/24376/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ShutdownVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ShutdownVmCommand.java
index 860becb..e44d10e 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ShutdownVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ShutdownVmCommand.java
@@ -1,5 +1,6 @@
 package org.ovirt.engine.core.bll;
 
+import org.ovirt.engine.core.bll.job.ExecutionHandler;
 import org.ovirt.engine.core.common.AuditLogType;
 import org.ovirt.engine.core.common.action.ShutdownVmParameters;
 import org.ovirt.engine.core.common.action.StopVmParameters;
@@ -16,6 +17,7 @@
 import org.ovirt.engine.core.utils.log.LogFactory;
 
 @NonTransactiveCommandAttribute(forceCompensation=true)
+@LockIdNameAttribute
 public class ShutdownVmCommand<T extends ShutdownVmParameters> extends 
StopVmCommandBase<T> {
 
     protected ShutdownVmCommand(Guid commandId) {
@@ -73,7 +75,10 @@
             StopVmParameters stopVmParams = new StopVmParameters(getVmId(), 
StopVmTypeEnum.CANNOT_SHUTDOWN);
             // stopVmParams.ParametersCurrentUser = CurrentUser;
             stopVmParams.setSessionId(getParameters().getSessionId());
-            Backend.getInstance().runInternalAction(VdcActionType.StopVm, 
stopVmParams);
+            Backend.getInstance().runInternalAction(
+                    VdcActionType.StopVm,
+                    stopVmParams,
+                    
ExecutionHandler.createDefaultContexForTasks(getExecutionContext(), getLock()));
         }
 
         setSucceeded(true);
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommand.java
index 821b787..cff1aa3 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommand.java
@@ -5,6 +5,7 @@
 import org.ovirt.engine.core.common.errors.VdcBllMessages;
 import org.ovirt.engine.core.compat.Guid;
 
+@LockIdNameAttribute
 @NonTransactiveCommandAttribute(forceCompensation=true)
 public class StopVmCommand<T extends StopVmParameters> extends 
StopVmCommandBase<T> {
     public StopVmCommand(T stopVmParams) {
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java
index f0a0eb4..a5e7bfd 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java
@@ -1,7 +1,9 @@
 package org.ovirt.engine.core.bll;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 
 import org.apache.commons.lang.StringUtils;
 import org.ovirt.engine.core.bll.job.ExecutionHandler;
@@ -21,6 +23,8 @@
 import org.ovirt.engine.core.common.businessentities.VMStatus;
 import org.ovirt.engine.core.common.businessentities.VmDynamic;
 import org.ovirt.engine.core.common.errors.VdcBllMessages;
+import org.ovirt.engine.core.common.locks.LockingGroup;
+import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.common.vdscommands.DestroyVmVDSCommandParameters;
 import 
org.ovirt.engine.core.common.vdscommands.UpdateVmDynamicDataVDSCommandParameters;
 import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
@@ -243,4 +247,13 @@
     public void addQuotaPermissionSubject(List<PermissionSubject> 
quotaPermissionList) {
         //
     }
+
+    @Override
+    protected Map<String, Pair<String, String>> getExclusiveLocks() {
+        return Collections.singletonMap(
+                getVmId().toString(),
+                LockMessagesMatchUtil.makeLockingPair(
+                        LockingGroup.VM,
+                        VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED));
+    }
 }


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6109ef0ea4869b59a93be6fef4b0c3cff50da62e
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.3
Gerrit-Owner: Roy Golan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to