[libvirt] [PATCH 4/4] storagepool: Return with locked obj from virStoragePoolObjRemove

John Ferlan jferlan at redhat.com
Wed Mar 21 15:53:35 UTC 2018


Rather than unlock the object that was expected to be locked on
input, let the caller perform the unlock or more succinctly a
virStoragePoolObjEndAPI on the object which will perform the Unref
and Unlock and clear the @obj.

Also add comments for virStoragePoolObjRemove.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/conf/virstorageobj.c     | 17 ++++++++++++++---
 src/storage/storage_driver.c | 12 +-----------
 src/test/test_driver.c       | 14 +++-----------
 3 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 799b8c9fa3..c057c6b3ed 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -517,6 +517,18 @@ virStoragePoolObjListSearch(virStoragePoolObjListPtr pools,
 }
 
 
+/*
+ * virStoragePoolObjRemove:
+ * @pools: list of storage pool objects
+ * @obj: a storage pool object
+ *
+ * Remove @obj from the storage pool obj list hash tables. The caller must
+ * hold the lock on @obj to ensure no one else is either waiting for @obj or
+ * still using it.
+ *
+ * Upon return the @obj remains locked with at least 1 reference and
+ * the caller is expected to use virStoragePoolObjEndAPI on it.
+ */
 void
 virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
                         virStoragePoolObjPtr obj)
@@ -530,7 +542,6 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
     virObjectLock(obj);
     virHashRemoveEntry(pools->objs, uuidstr);
     virHashRemoveEntry(pools->objsName, obj->def->name);
-    virObjectUnlock(obj);
     virObjectUnref(obj);
     virObjectRWUnlock(pools);
 }
@@ -1138,13 +1149,13 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
     VIR_FREE(obj->configFile);  /* for driver reload */
     if (VIR_STRDUP(obj->configFile, path) < 0) {
         virStoragePoolObjRemove(pools, obj);
-        virObjectUnref(obj);
+        virStoragePoolObjEndAPI(&obj);
         return NULL;
     }
     VIR_FREE(obj->autostartLink); /* for driver reload */
     if (VIR_STRDUP(obj->autostartLink, autostartLink) < 0) {
         virStoragePoolObjRemove(pools, obj);
-        virObjectUnref(obj);
+        virStoragePoolObjEndAPI(&obj);
         return NULL;
     }
 
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index d5e38af5aa..b2946d0509 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -94,7 +94,7 @@ virStoragePoolUpdateInactive(virStoragePoolObjPtr *objptr)
 
     if (!virStoragePoolObjGetConfigFile(obj)) {
         virStoragePoolObjRemove(driver->pools, obj);
-        virObjectUnref(obj);
+        virStoragePoolObjEndAPI(&obj);
         *objptr = NULL;
     } else if (virStoragePoolObjGetNewDef(obj)) {
         virStoragePoolObjDefUseNewDef(obj);
@@ -746,8 +746,6 @@ storagePoolCreateXML(virConnectPtr conn,
             (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD)) {
             if (backend->buildPool(obj, build_flags) < 0) {
                 virStoragePoolObjRemove(driver->pools, obj);
-                virObjectUnref(obj);
-                obj = NULL;
                 goto cleanup;
             }
         }
@@ -756,8 +754,6 @@ storagePoolCreateXML(virConnectPtr conn,
     if (backend->startPool &&
         backend->startPool(obj) < 0) {
         virStoragePoolObjRemove(driver->pools, obj);
-        virObjectUnref(obj);
-        obj = NULL;
         goto cleanup;
     }
 
@@ -771,8 +767,6 @@ storagePoolCreateXML(virConnectPtr conn,
         if (backend->stopPool)
             backend->stopPool(obj);
         virStoragePoolObjRemove(driver->pools, obj);
-        virObjectUnref(obj);
-        obj = NULL;
         goto cleanup;
     }
 
@@ -833,8 +827,6 @@ storagePoolDefineXML(virConnectPtr conn,
 
     if (virStoragePoolObjSaveDef(driver, obj, def) < 0) {
         virStoragePoolObjRemove(driver->pools, obj);
-        virObjectUnref(obj);
-        obj = NULL;
         goto cleanup;
     }
 
@@ -901,8 +893,6 @@ storagePoolUndefine(virStoragePoolPtr pool)
 
     VIR_INFO("Undefining storage pool '%s'", def->name);
     virStoragePoolObjRemove(driver->pools, obj);
-    virObjectUnref(obj);
-    obj = NULL;
     ret = 0;
 
  cleanup:
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index a7aaabe09b..4b7f8c4244 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4556,16 +4556,12 @@ testStoragePoolCreateXML(virConnectPtr conn,
                             def->source.adapter.data.fchost.wwnn,
                             def->source.adapter.data.fchost.wwpn) < 0) {
             virStoragePoolObjRemove(privconn->pools, obj);
-            virObjectUnref(obj);
-            obj = NULL;
             goto cleanup;
         }
     }
 
     if (testStoragePoolObjSetDefaults(obj) == -1) {
         virStoragePoolObjRemove(privconn->pools, obj);
-        virObjectUnref(obj);
-        obj = NULL;
         goto cleanup;
     }
 
@@ -4627,8 +4623,6 @@ testStoragePoolDefineXML(virConnectPtr conn,
 
     if (testStoragePoolObjSetDefaults(obj) == -1) {
         virStoragePoolObjRemove(privconn->pools, obj);
-        virObjectUnref(obj);
-        obj = NULL;
         goto cleanup;
     }
 
@@ -4658,7 +4652,7 @@ testStoragePoolUndefine(virStoragePoolPtr pool)
                                             0);
 
     virStoragePoolObjRemove(privconn->pools, obj);
-    virObjectUnref(obj);
+    virStoragePoolObjEndAPI(&obj);
 
     testObjectEventQueue(privconn, event);
     return 0;
@@ -4750,11 +4744,9 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
                                             VIR_STORAGE_POOL_EVENT_STOPPED,
                                             0);
 
-    if (!(virStoragePoolObjGetConfigFile(obj))) {
+    if (!(virStoragePoolObjGetConfigFile(obj)))
         virStoragePoolObjRemove(privconn->pools, obj);
-        virObjectUnref(obj);
-        obj = NULL;
-    }
+
     ret = 0;
 
  cleanup:
-- 
2.13.6




More information about the libvir-list mailing list