[PATCH 1/7] util: Alter virObjectLockRead to return status

[John Ferlan <jferlan@xxxxxxxxxx>]

Rather than ignore errors, let's have virObjectLockRead check for
the correct usage and issue an error when not properly used so
so that we don't run into situations where the resource we think
we're locking really isn't locked because the void input parameter
wasn't valid.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
 src/conf/virdomainobjlist.c | 27 ++++++++++++++++++---------
 src/util/virobject.c        |  6 +++++-
 src/util/virobject.h        |  2 +-
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 1d027a4..fed4029 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -118,7 +118,8 @@ virDomainObjListFindByIDInternal(virDomainObjListPtr doms,
                                  bool ref)
 {
     virDomainObjPtr obj;
-    virObjectLockRead(doms);
+    if (virObjectLockRead(doms) < 0)
+        return NULL;
     obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id, NULL);
     if (ref) {
         virObjectRef(obj);
@@ -160,7 +161,8 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms,
     char uuidstr[VIR_UUID_STRING_BUFLEN];
     virDomainObjPtr obj;
 
-    virObjectLockRead(doms);
+    if (virObjectLockRead(doms) < 0)
+        return NULL;
     virUUIDFormat(uuid, uuidstr);
 
     obj = virHashLookup(doms->objs, uuidstr);
@@ -204,7 +206,8 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
 {
     virDomainObjPtr obj;
 
-    virObjectLockRead(doms);
+    if (virObjectLockRead(doms) < 0)
+        return NULL;
     obj = virHashLookup(doms->objsName, name);
     virObjectRef(obj);
     virObjectUnlock(doms);
@@ -653,7 +656,8 @@ virDomainObjListNumOfDomains(virDomainObjListPtr doms,
                              virConnectPtr conn)
 {
     struct virDomainObjListData data = { filter, conn, active, 0 };
-    virObjectLockRead(doms);
+    if (virObjectLockRead(doms) < 0)
+        return -1;
     virHashForEach(doms->objs, virDomainObjListCount, &data);
     virObjectUnlock(doms);
     return data.count;
@@ -697,7 +701,8 @@ virDomainObjListGetActiveIDs(virDomainObjListPtr doms,
 {
     struct virDomainIDData data = { filter, conn,
                                     0, maxids, ids };
-    virObjectLockRead(doms);
+    if (virObjectLockRead(doms) < 0)
+        return -1;
     virHashForEach(doms->objs, virDomainObjListCopyActiveIDs, &data);
     virObjectUnlock(doms);
     return data.numids;
@@ -751,7 +756,8 @@ virDomainObjListGetInactiveNames(virDomainObjListPtr doms,
     struct virDomainNameData data = { filter, conn,
                                       0, 0, maxnames, names };
     size_t i;
-    virObjectLockRead(doms);
+    if (virObjectLockRead(doms) < 0)
+        return -1;
     virHashForEach(doms->objs, virDomainObjListCopyInactiveNames, &data);
     virObjectUnlock(doms);
     if (data.oom) {
@@ -792,7 +798,8 @@ virDomainObjListForEach(virDomainObjListPtr doms,
     struct virDomainListIterData data = {
         callback, opaque, 0,
     };
-    virObjectLockRead(doms);
+    if (virObjectLockRead(doms) < 0)
+        return -1;
     virHashForEach(doms->objs, virDomainObjListHelper, &data);
     virObjectUnlock(doms);
     return data.ret;
@@ -925,7 +932,8 @@ virDomainObjListCollect(virDomainObjListPtr domlist,
 {
     struct virDomainListData data = { NULL, 0 };
 
-    virObjectLockRead(domlist);
+    if (virObjectLockRead(domlist) < 0)
+        return -1;
     sa_assert(domlist->objs);
     if (VIR_ALLOC_N(data.vms, virHashSize(domlist->objs)) < 0) {
         virObjectUnlock(domlist);
@@ -962,7 +970,8 @@ virDomainObjListConvert(virDomainObjListPtr domlist,
     *nvms = 0;
     *vms = NULL;
 
-    virObjectLockRead(domlist);
+    if (virObjectLockRead(domlist) < 0)
+        return -1;
     for (i = 0; i < ndoms; i++) {
         virDomainPtr dom = doms[i];
 
diff --git a/src/util/virobject.c b/src/util/virobject.c
index b1bb378..73de4d3 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -410,17 +410,21 @@ virObjectLock(void *anyobj)
  * The object must be unlocked before releasing this
  * reference.
  */
-void
+int
 virObjectLockRead(void *anyobj)
 {
     if (virObjectIsClass(anyobj, virObjectRWLockableClass)) {
         virObjectRWLockablePtr obj = anyobj;
         virRWLockRead(&obj->lock);
+        return 0;
     } else {
         virObjectPtr obj = anyobj;
         VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance",
                  anyobj, obj ? obj->klass->name : "(unknown)");
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("unable to obtain rwlock for object=%p"), anyobj);
     }
+    return -1;
 }
 
 
diff --git a/src/util/virobject.h b/src/util/virobject.h
index 5985fad..99910e0 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -124,7 +124,7 @@ void
 virObjectLock(void *lockableobj)
     ATTRIBUTE_NONNULL(1);
 
-void
+int
 virObjectLockRead(void *lockableobj)
     ATTRIBUTE_NONNULL(1);
 
-- 
2.9.4

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list