[libvirt] [PATCH v2 5/8] util: Introduce and use virObjectRWUnlock

John Ferlan jferlan at redhat.com
Tue Aug 1 00:05:05 UTC 2017


Rather than overload virObjectUnlock as commit id '77f4593b' has
done, create a separate virObjectRWUnlock API that will force the
consumers to make the proper decision regarding unlocking the
RWLock's. Similar to the RWLockRead and RWLockWrite, use the
virObjectGetRWLockableObj helper. This restores the virObjectUnlock
code to using the virObjectGetLockableObj.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/conf/virdomainobjlist.c | 36 ++++++++++++++++++------------------
 src/libvirt_private.syms    |  1 +
 src/util/virobject.c        | 45 +++++++++++++++++++++++++++++----------------
 src/util/virobject.h        |  4 ++++
 4 files changed, 52 insertions(+), 34 deletions(-)

diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 573032f..d874133 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -122,7 +122,7 @@ virDomainObjListFindByIDInternal(virDomainObjListPtr doms,
     obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id, NULL);
     if (ref) {
         virObjectRef(obj);
-        virObjectUnlock(doms);
+        virObjectRWUnlock(doms);
     }
     if (obj) {
         virObjectLock(obj);
@@ -134,7 +134,7 @@ virDomainObjListFindByIDInternal(virDomainObjListPtr doms,
         }
     }
     if (!ref)
-        virObjectUnlock(doms);
+        virObjectRWUnlock(doms);
     return obj;
 }
 
@@ -166,7 +166,7 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms,
     obj = virHashLookup(doms->objs, uuidstr);
     if (ref) {
         virObjectRef(obj);
-        virObjectUnlock(doms);
+        virObjectRWUnlock(doms);
     }
     if (obj) {
         virObjectLock(obj);
@@ -178,7 +178,7 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms,
         }
     }
     if (!ref)
-        virObjectUnlock(doms);
+        virObjectRWUnlock(doms);
     return obj;
 }
 
@@ -207,7 +207,7 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
     virObjectRWLockRead(doms);
     obj = virHashLookup(doms->objsName, name);
     virObjectRef(obj);
-    virObjectUnlock(doms);
+    virObjectRWUnlock(doms);
     if (obj) {
         virObjectLock(obj);
         if (obj->removing) {
@@ -329,7 +329,7 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms,
 
     virObjectRWLockWrite(doms);
     ret = virDomainObjListAddLocked(doms, def, xmlopt, flags, oldDef);
-    virObjectUnlock(doms);
+    virObjectRWUnlock(doms);
     return ret;
 }
 
@@ -355,7 +355,7 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
     virHashRemoveEntry(doms->objsName, dom->def->name);
     virObjectUnlock(dom);
     virObjectUnref(dom);
-    virObjectUnlock(doms);
+    virObjectRWUnlock(doms);
 }
 
 
@@ -420,7 +420,7 @@ virDomainObjListRename(virDomainObjListPtr doms,
 
     ret = 0;
  cleanup:
-    virObjectUnlock(doms);
+    virObjectRWUnlock(doms);
     VIR_FREE(old_name);
     return ret;
 }
@@ -609,7 +609,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
     }
 
     VIR_DIR_CLOSE(dir);
-    virObjectUnlock(doms);
+    virObjectRWUnlock(doms);
     return ret;
 }
 
@@ -655,7 +655,7 @@ virDomainObjListNumOfDomains(virDomainObjListPtr doms,
     struct virDomainObjListData data = { filter, conn, active, 0 };
     virObjectRWLockRead(doms);
     virHashForEach(doms->objs, virDomainObjListCount, &data);
-    virObjectUnlock(doms);
+    virObjectRWUnlock(doms);
     return data.count;
 }
 
@@ -699,7 +699,7 @@ virDomainObjListGetActiveIDs(virDomainObjListPtr doms,
                                     0, maxids, ids };
     virObjectRWLockRead(doms);
     virHashForEach(doms->objs, virDomainObjListCopyActiveIDs, &data);
-    virObjectUnlock(doms);
+    virObjectRWUnlock(doms);
     return data.numids;
 }
 
@@ -753,7 +753,7 @@ virDomainObjListGetInactiveNames(virDomainObjListPtr doms,
     size_t i;
     virObjectRWLockRead(doms);
     virHashForEach(doms->objs, virDomainObjListCopyInactiveNames, &data);
-    virObjectUnlock(doms);
+    virObjectRWUnlock(doms);
     if (data.oom) {
         for (i = 0; i < data.numnames; i++)
             VIR_FREE(data.names[i]);
@@ -794,7 +794,7 @@ virDomainObjListForEach(virDomainObjListPtr doms,
     };
     virObjectRWLockRead(doms);
     virHashForEach(doms->objs, virDomainObjListHelper, &data);
-    virObjectUnlock(doms);
+    virObjectRWUnlock(doms);
     return data.ret;
 }
 
@@ -928,12 +928,12 @@ virDomainObjListCollect(virDomainObjListPtr domlist,
     virObjectRWLockRead(domlist);
     sa_assert(domlist->objs);
     if (VIR_ALLOC_N(data.vms, virHashSize(domlist->objs)) < 0) {
-        virObjectUnlock(domlist);
+        virObjectRWUnlock(domlist);
         return -1;
     }
 
     virHashForEach(domlist->objs, virDomainObjListCollectIterator, &data);
-    virObjectUnlock(domlist);
+    virObjectRWUnlock(domlist);
 
     virDomainObjListFilter(&data.vms, &data.nvms, conn, filter, flags);
 
@@ -972,7 +972,7 @@ virDomainObjListConvert(virDomainObjListPtr domlist,
             if (skip_missing)
                 continue;
 
-            virObjectUnlock(domlist);
+            virObjectRWUnlock(domlist);
             virReportError(VIR_ERR_NO_DOMAIN,
                            _("no domain with matching uuid '%s' (%s)"),
                            uuidstr, dom->name);
@@ -982,12 +982,12 @@ virDomainObjListConvert(virDomainObjListPtr domlist,
         virObjectRef(vm);
 
         if (VIR_APPEND_ELEMENT(*vms, *nvms, vm) < 0) {
-            virObjectUnlock(domlist);
+            virObjectRWUnlock(domlist);
             virObjectUnref(vm);
             goto error;
         }
     }
-    virObjectUnlock(domlist);
+    virObjectRWUnlock(domlist);
 
     sa_assert(*vms);
     virDomainObjListFilter(vms, nvms, conn, filter, flags);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7f2f6d6..97aaf37 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2310,6 +2310,7 @@ virObjectRef;
 virObjectRWLockableNew;
 virObjectRWLockRead;
 virObjectRWLockWrite;
+virObjectRWUnlock;
 virObjectUnlock;
 virObjectUnref;
 
diff --git a/src/util/virobject.c b/src/util/virobject.c
index c1e4474..85e5a53 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -428,7 +428,7 @@ virObjectLock(void *anyobj)
  * @anyobj: any instance of virObjectRWLockable
  *
  * Acquire a read lock on @anyobj. The lock must be
- * released by virObjectUnlock.
+ * released by virObjectRWUnlock.
  *
  * The caller is expected to have acquired a reference
  * on the object before locking it (eg virObjectRef).
@@ -457,7 +457,7 @@ virObjectRWLockRead(void *anyobj)
  * @anyobj: any instance of virObjectRWLockable
  *
  * Acquire a write lock on @anyobj. The lock must be
- * released by virObjectUnlock.
+ * released by virObjectRWUnlock.
  *
  * The caller is expected to have acquired a reference
  * on the object before locking it (eg virObjectRef).
@@ -483,26 +483,39 @@ virObjectRWLockWrite(void *anyobj)
 
 /**
  * virObjectUnlock:
- * @anyobj: any instance of virObjectLockable or virObjectRWLockable
+ * @anyobj: any instance of virObjectLockable
  *
  * Release a lock on @anyobj. The lock must have been acquired by
- * virObjectLock, virObjectRWLockRead, or virObjectRWLockWrite.
+ * virObjectLock.
  */
 void
 virObjectUnlock(void *anyobj)
 {
-    if (virObjectIsClass(anyobj, virObjectLockableClass)) {
-        virObjectLockablePtr obj = anyobj;
-        virMutexUnlock(&obj->lock);
-    } else if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
-        virObjectRWLockablePtr obj = anyobj;
-        virRWLockUnlock(&obj->lock);
-    } else {
-        virObjectPtr obj = anyobj;
-        VIR_WARN("Object %p (%s) is not a virObjectLockable "
-                 "nor virObjectRWLockable instance",
-                 anyobj, obj ? obj->klass->name : "(unknown)");
-    }
+    virObjectLockablePtr obj = virObjectGetLockableObj(anyobj);
+
+    if (!obj)
+        return;
+
+    virMutexUnlock(&obj->lock);
+}
+
+
+/**
+ * virObjectRWUnlock:
+ * @anyobj: any instance of virObjectRWLockable
+ *
+ * Release a lock on @anyobj. The lock must have been acquired by
+ * virObjectRWLockRead or virObjectRWLockWrite.
+ */
+void
+virObjectRWUnlock(void *anyobj)
+{
+    virObjectRWLockablePtr obj = virObjectGetRWLockableObj(anyobj);
+
+    if (!obj)
+        return;
+
+    virRWLockUnlock(&obj->lock);
 }
 
 
diff --git a/src/util/virobject.h b/src/util/virobject.h
index 24ee6dd..ac6cf22 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -137,6 +137,10 @@ virObjectUnlock(void *lockableobj)
     ATTRIBUTE_NONNULL(1);
 
 void
+virObjectRWUnlock(void *lockableobj)
+    ATTRIBUTE_NONNULL(1);
+
+void
 virObjectListFree(void *list);
 
 void
-- 
2.9.4




More information about the libvir-list mailing list