Shireesh Anjal has uploaded a new change for review.
Change subject: gluster: Fixed deadlock in nested command execution
......................................................................
gluster: Fixed deadlock in nested command execution
The CreateGlusterVolume command internally calls SetGlusterVolumeOption.
Both these try to acquire exclusive lock on the cluster, resulting in a
deadlock. To fix this,
Introduced new parameters base class GlusterParametersBase, which will
be extended by all gluster related parameter classes. This class
provides a flag "nested" that indicates whether the action is being
called from another gluster related action.
Modified GlusterCommandBase to acquire a lock on the cluster only if it
is not a nested execution.
Change-Id: Id080ad6ac3587ab9c3443cb83969783955dfb082
Signed-off-by: Shireesh Anjal <san...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterCommandBase.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/CreateGlusterVolumeParameters.java
A
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/GlusterParametersBase.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/GlusterVolumeParameters.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/RemoveGlusterServerParameters.java
5 files changed, 36 insertions(+), 11 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/48/10448/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterCommandBase.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterCommandBase.java
index 9c5a1ff..53468c2 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterCommandBase.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterCommandBase.java
@@ -12,9 +12,9 @@
import org.ovirt.engine.core.bll.utils.PermissionSubject;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.VdcObjectType;
-import org.ovirt.engine.core.common.action.VdcActionParametersBase;
import org.ovirt.engine.core.common.action.VdcActionType;
import org.ovirt.engine.core.common.action.VdcReturnValueBase;
+import org.ovirt.engine.core.common.action.gluster.GlusterParametersBase;
import org.ovirt.engine.core.common.businessentities.VDS;
import org.ovirt.engine.core.common.businessentities.VdsStatic;
import
org.ovirt.engine.core.common.businessentities.gluster.GlusterBrickEntity;
@@ -25,7 +25,7 @@
/**
* Base class for all Gluster commands
*/
-public abstract class GlusterCommandBase<T extends VdcActionParametersBase>
extends CommandBase<T> {
+public abstract class GlusterCommandBase<T extends GlusterParametersBase>
extends CommandBase<T> {
private static final long serialVersionUID = -7394070330293300587L;
protected AuditLogType errorType;
protected VDS upServer;
@@ -36,7 +36,10 @@
@Override
protected Map<String, String> getExclusiveLocks() {
- return Collections.singletonMap(getVdsGroupId().toString(),
LockingGroup.GLUSTER.name());
+ if (!getParameters().isNested()) {
+ return Collections.singletonMap(getVdsGroupId().toString(),
LockingGroup.GLUSTER.name());
+ }
+ return super.getExclusiveLocks();
}
/*
@@ -82,13 +85,16 @@
}
/**
- * Executes given BLL action, updates the success flag based on the
result, and returns the result
+ * Executes given BLL action, updates the success flag based on the
result, and returns the result <br>
+ * Also sets the "nested" flag on the parameters object to true, so that
the action being invoked doesn't try to
+ * acquire an exclusive lock on the cluster, as it has already been
acquired by the currently executing action.
*
* @param actionType
* @param params
* @return
*/
- protected VdcReturnValueBase runBllAction(VdcActionType actionType,
VdcActionParametersBase params) {
+ protected VdcReturnValueBase runBllAction(VdcActionType actionType,
GlusterParametersBase params) {
+ params.setNested(true);
VdcReturnValueBase returnValue =
Backend.getInstance().runInternalAction(actionType, params);
setSucceeded(returnValue.getSucceeded());
return returnValue;
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/CreateGlusterVolumeParameters.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/CreateGlusterVolumeParameters.java
index e953ea9..f260915 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/CreateGlusterVolumeParameters.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/CreateGlusterVolumeParameters.java
@@ -3,13 +3,12 @@
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
-import org.ovirt.engine.core.common.action.VdcActionParametersBase;
import
org.ovirt.engine.core.common.businessentities.gluster.GlusterVolumeEntity;
/**
* Command parameters for the "Create Volume" action
*/
-public class CreateGlusterVolumeParameters extends VdcActionParametersBase {
+public class CreateGlusterVolumeParameters extends GlusterParametersBase {
private static final long serialVersionUID = 2015321730118872954L;
@NotNull(message = "VALIDATION.GLUSTER.VOLUME.NOT_NULL")
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/GlusterParametersBase.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/GlusterParametersBase.java
new file mode 100644
index 0000000..c437d50
--- /dev/null
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/GlusterParametersBase.java
@@ -0,0 +1,22 @@
+package org.ovirt.engine.core.common.action.gluster;
+
+import org.ovirt.engine.core.common.action.VdcActionParametersBase;
+
+/**
+ * Base Parameter class for all Gluster related actions. <br>
+ * Only use of this class, as of now, is to indicate whether the action is
being called from another gluster action. If
+ * true, GlusterCommandBase will not try to acquire lock on the cluster, as it
would have been already acquired by the
+ * calling action.
+ */
+public class GlusterParametersBase extends VdcActionParametersBase {
+ private static final long serialVersionUID = -5468798854917831630L;
+ private boolean nested = false;
+
+ public boolean isNested() {
+ return nested;
+ }
+
+ public void setNested(boolean nested) {
+ this.nested = nested;
+ }
+}
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/GlusterVolumeParameters.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/GlusterVolumeParameters.java
index 6ed1587..839101b 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/GlusterVolumeParameters.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/GlusterVolumeParameters.java
@@ -2,7 +2,6 @@
import javax.validation.constraints.NotNull;
-import org.ovirt.engine.core.common.action.VdcActionParametersBase;
import org.ovirt.engine.core.compat.Guid;
/**
@@ -10,7 +9,7 @@
* This will be used directly by some commands (e.g. start volume), <br>
* and inherited by others (e.g. set volume option).
*/
-public class GlusterVolumeParameters extends VdcActionParametersBase {
+public class GlusterVolumeParameters extends GlusterParametersBase {
private static final long serialVersionUID = -5148741622108406754L;
@NotNull(message = "VALIDATION.GLUSTER.VOLUME.ID.NOT_NULL")
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/RemoveGlusterServerParameters.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/RemoveGlusterServerParameters.java
index a02279a..cc4a0b8 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/RemoveGlusterServerParameters.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/RemoveGlusterServerParameters.java
@@ -1,13 +1,12 @@
package org.ovirt.engine.core.common.action.gluster;
-import org.ovirt.engine.core.common.action.VdcActionParametersBase;
import org.ovirt.engine.core.compat.Guid;
/**
* Parameter class with hostnameOrIp and forceAction as parameters. <br>
* This will be used by remove gluster server command. <br>
*/
-public class RemoveGlusterServerParameters extends VdcActionParametersBase {
+public class RemoveGlusterServerParameters extends GlusterParametersBase {
private static final long serialVersionUID = -1224829720081853632L;
private Guid clusterId;
--
To view, visit http://gerrit.ovirt.org/10448
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id080ad6ac3587ab9c3443cb83969783955dfb082
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shireesh Anjal <san...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
