virStoragePoolObjListAdd: Transfer definition ownership

Upon successful return from virStoragePoolObjListAdd() the
virStoragePoolObj is the owner of secret definition. To make this
ownership transfer even more visible, lets pass the definition as
a double pointer and use g_steal_pointer().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index f906c5b141cc617996ea0b485d576908d914b951..2c29339b6aec310766e732688f20071a2be43a5d 100644 (file)
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -1513,20 +1513,20 @@ virStoragePoolObjSourceFindDuplicate(virStoragePoolObjList *pools,
 
 static void
 virStoragePoolObjAssignDef(virStoragePoolObj *obj,
-                           virStoragePoolDef *def,
+                           virStoragePoolDef **def,
                            unsigned int flags)
 {
     if (virStoragePoolObjIsActive(obj) ||
         virStoragePoolObjIsStarting(obj)) {
         virStoragePoolDefFree(obj->newDef);
-        obj->newDef = def;
+        obj->newDef = g_steal_pointer(def);
     } else {
         if (!obj->newDef &&
             flags & VIR_STORAGE_POOL_OBJ_LIST_ADD_LIVE)
             obj->newDef = g_steal_pointer(&obj->def);
 
         virStoragePoolDefFree(obj->def);
-        obj->def = def;
+        obj->def = g_steal_pointer(def);
     }
 }
 
@@ -1548,11 +1548,16 @@ virStoragePoolObjAssignDef(virStoragePoolObj *obj,
  * If VIR_STORAGE_POOL_OBJ_LIST_ADD_CHECK_LIVE is set in @flags
  * then this will fail if the pool exists and is active.
  *
+ * Upon successful return the virStoragePool object is the owner
+ * of @def and callers should use virStoragePoolObjGetDef() or
+ * virStoragePoolObjGetNewDef() if they need to access the
+ * definition as @def is set to NULL.
+ *
  * Returns locked and reffed object pointer or NULL on error
  */
 virStoragePoolObj *
 virStoragePoolObjListAdd(virStoragePoolObjList *pools,
-                         virStoragePoolDef *def,
+                         virStoragePoolDef **def,
                          unsigned int flags)
 {
     virStoragePoolObj *obj = NULL;
@@ -1561,10 +1566,10 @@ virStoragePoolObjListAdd(virStoragePoolObjList *pools,
 
     virObjectRWLockWrite(pools);
 
-    if (virStoragePoolObjSourceFindDuplicate(pools, def) < 0)
+    if (virStoragePoolObjSourceFindDuplicate(pools, *def) < 0)
         goto error;
 
-    rc = virStoragePoolObjIsDuplicate(pools, def,
+    rc = virStoragePoolObjIsDuplicate(pools, *def,
                                       !!(flags & VIR_STORAGE_POOL_OBJ_LIST_ADD_CHECK_LIVE),
                                       &obj);
 
@@ -1579,17 +1584,17 @@ virStoragePoolObjListAdd(virStoragePoolObjList *pools,
     if (!(obj = virStoragePoolObjNew()))
         goto error;
 
-    virUUIDFormat(def->uuid, uuidstr);
+    virUUIDFormat((*def)->uuid, uuidstr);
     if (virHashAddEntry(pools->objs, uuidstr, obj) < 0)
         goto error;
     virObjectRef(obj);
 
-    if (virHashAddEntry(pools->objsName, def->name, obj) < 0) {
+    if (virHashAddEntry(pools->objsName, (*def)->name, obj) < 0) {
         virHashRemoveEntry(pools->objs, uuidstr);
         goto error;
     }
     virObjectRef(obj);
-    obj->def = def;
+    obj->def = g_steal_pointer(def);
     virObjectRWUnlock(pools);
     return obj;
 
@@ -1620,9 +1625,8 @@ virStoragePoolObjLoad(virStoragePoolObjList *pools,
         return NULL;
     }
 
-    if (!(obj = virStoragePoolObjListAdd(pools, def, 0)))
+    if (!(obj = virStoragePoolObjListAdd(pools, &def, 0)))
         return NULL;
-    def = NULL;
 
     VIR_FREE(obj->configFile);  /* for driver reload */
     obj->configFile = g_strdup(path);
@@ -1673,10 +1677,9 @@ virStoragePoolObjLoadState(virStoragePoolObjList *pools,
     }
 
     /* create the object */
-    if (!(obj = virStoragePoolObjListAdd(pools, def,
+    if (!(obj = virStoragePoolObjListAdd(pools, &def,
                                          VIR_STORAGE_POOL_OBJ_LIST_ADD_CHECK_LIVE)))
         return NULL;
-    def = NULL;
 
     /* XXX: future handling of some additional useful status data,
      * for now, if a status file for a pool exists, the pool will be marked

diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 65f2ae817532bd59aaaedf74f88417ce358ce56a..523bdec24409f090647837b08b2fc9ed72993c6e 100644 (file)
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -203,7 +203,7 @@ typedef enum {
 
 virStoragePoolObj *
 virStoragePoolObjListAdd(virStoragePoolObjList *pools,
-                         virStoragePoolDef *def,
+                         virStoragePoolDef **def,
                          unsigned int flags);
 
 int

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 51daf6a05dbdf9190eb6c4b341b164f26933756a..4df2c75a2bb7b3dcb960d5f8c2bfb2b1f7146688 100644 (file)
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -746,11 +746,10 @@ storagePoolCreateXML(virConnectPtr conn,
     if ((backend = virStorageBackendForType(newDef->type)) == NULL)
         goto cleanup;
 
-    if (!(obj = virStoragePoolObjListAdd(driver->pools, newDef,
+    if (!(obj = virStoragePoolObjListAdd(driver->pools, &newDef,
                                          VIR_STORAGE_POOL_OBJ_LIST_ADD_LIVE |
                                          VIR_STORAGE_POOL_OBJ_LIST_ADD_CHECK_LIVE)))
         goto cleanup;
-    newDef = NULL;
     def = virStoragePoolObjGetDef(obj);
 
     virStoragePoolObjSetStarting(obj, true);
@@ -830,7 +829,7 @@ storagePoolDefineXML(virConnectPtr conn,
     if (virStorageBackendForType(newDef->type) == NULL)
         goto cleanup;
 
-    if (!(obj = virStoragePoolObjListAdd(driver->pools, newDef, 0)))
+    if (!(obj = virStoragePoolObjListAdd(driver->pools, &newDef, 0)))
         goto cleanup;
     newDef = virStoragePoolObjGetNewDef(obj);
     def = virStoragePoolObjGetDef(obj);

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 92c0462170e928e38a837538dbe9b961e88a5725..369edacf29a83f03185d4abdaccb6a3258490c56 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1226,7 +1226,7 @@ testParseStorage(testDriver *privconn,
         if (!def)
             return -1;
 
-        if (!(obj = virStoragePoolObjListAdd(privconn->pools, def, 0))) {
+        if (!(obj = virStoragePoolObjListAdd(privconn->pools, &def, 0))) {
             virStoragePoolDefFree(def);
             return -1;
         }
@@ -6675,10 +6675,9 @@ testStoragePoolCreateXML(virConnectPtr conn,
     if (!(newDef = virStoragePoolDefParseString(xml, 0)))
         goto cleanup;
 
-    if (!(obj = virStoragePoolObjListAdd(privconn->pools, newDef,
+    if (!(obj = virStoragePoolObjListAdd(privconn->pools, &newDef,
                                          VIR_STORAGE_POOL_OBJ_LIST_ADD_CHECK_LIVE)))
         goto cleanup;
-    newDef = NULL;
     def = virStoragePoolObjGetDef(obj);
 
     if (def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) {
@@ -6742,9 +6741,8 @@ testStoragePoolDefineXML(virConnectPtr conn,
     newDef->allocation = defaultPoolAlloc;
     newDef->available = defaultPoolCap - defaultPoolAlloc;
 
-    if (!(obj = virStoragePoolObjListAdd(privconn->pools, newDef, 0)))
+    if (!(obj = virStoragePoolObjListAdd(privconn->pools, &newDef, 0)))
         goto cleanup;
-    newDef = NULL;
     def = virStoragePoolObjGetDef(obj);
 
     event = virStoragePoolEventLifecycleNew(def->name, def->uuid,