Change in vdsm[master]: resourceManager: Avoid uneeded recursive locking

nsoffer at redhat.com nsoffer at redhat.com
Fri Jul 10 18:15:59 UTC 2015


Nir Soffer has uploaded a new change for review.

Change subject: resourceManager: Avoid uneeded recursive locking
......................................................................

resourceManager: Avoid uneeded recursive locking

In registerNamesapce, when force registering an existing namespace, we
first try to remove the existing one using unregisterNamespace. Since
both public methods acquire an exclusive lock, we must use a recursive
lock to avoid deadlock.

This patch removes the need for recursive lock by extracting the
unregisterNamespace logic into private _unregisterNamespaceLocked
method, and using it from the public method.

Change-Id: I6658645bba178fa5f3821a56e3453f62d0c9db4f
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
M vdsm/storage/resourceManager.py
1 file changed, 16 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/43424/1

diff --git a/vdsm/storage/resourceManager.py b/vdsm/storage/resourceManager.py
index b1b0dc7..424aa92 100644
--- a/vdsm/storage/resourceManager.py
+++ b/vdsm/storage/resourceManager.py
@@ -413,7 +413,7 @@
         with self._syncRoot.exclusive:
             if (namespace in self._namespaces):
                 if force:
-                    self.unregisterNamespace(namespace)
+                    self._unregisterNamespaceLocked(namespace)
                 else:
                     raise KeyError("Namespace '%s' already exists." %
                                    namespace)
@@ -427,15 +427,22 @@
             if namespace not in self._namespaces:
                 raise KeyError("Namespace '%s' doesn't exist" % namespace)
 
-            self._log.debug("Unregistering namespace '%s'", namespace)
-            namespaceObj = self._namespaces[namespace]
-            with namespaceObj.lock:
-                if len(namespaceObj.resources) > 0:
-                    raise ResourceManagerError("Cannot unregister Resource "
-                                               "Factory '%s'. It has active "
-                                               "resources." % (namespace))
+            self._unregisterNamespaceLocked(namespace)
 
-                del self._namespaces[namespace]
+    def _unregisterNamespaceLocked(self, namespace):
+        """
+        Must be called when holding self._syncRoot.exclusive, and namespace
+        exists in self._namespaces.
+        """
+        self._log.debug("Unregistering namespace '%s'", namespace)
+        namespaceObj = self._namespaces[namespace]
+        with namespaceObj.lock:
+            if len(namespaceObj.resources) > 0:
+                raise ResourceManagerError("Cannot unregister Resource "
+                                           "Factory '%s'. It has active "
+                                           "resources." % (namespace))
+
+            del self._namespaces[namespace]
 
     def getResourceStatus(self, namespace, name):
         if not self._resourceNameValidator.match(name):


-- 
To view, visit https://gerrit.ovirt.org/43424
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6658645bba178fa5f3821a56e3453f62d0c9db4f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list