From c63315789f91bede6abca133dc2e67d730bc0698 Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Fri, 24 May 2019 16:35:37 +0200
Subject: [PATCH] virStoragePoolObjRemove: Don't unlock pool object upon return
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

The fact that we're removing a pool object from the list of pools
doesn't mean we want to unlock it. It violates locking policy
too as object locking and unlocking is not done on the same
level.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 src/conf/virstorageobj.c     |  5 ++---
 src/storage/storage_driver.c | 17 +++--------------
 src/test/test_driver.c       | 14 +++-----------
 3 files changed, 8 insertions(+), 28 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 0ac128c4bc..0b83c2d2de 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -514,7 +514,6 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
     virObjectLock(obj);
     virHashRemoveEntry(pools->objs, uuidstr);
     virHashRemoveEntry(pools->objsName, obj->def->name);
-    virObjectUnlock(obj);
     virObjectUnref(obj);
     virObjectRWUnlock(pools);
 }
@@ -1594,13 +1593,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 dfa654178b..74a11f4999 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -126,8 +126,7 @@ virStoragePoolUpdateInactive(virStoragePoolObjPtr *objptr)
 
     if (!virStoragePoolObjGetConfigFile(obj)) {
         virStoragePoolObjRemove(driver->pools, obj);
-        virObjectUnref(obj);
-        *objptr = NULL;
+        virStoragePoolObjEndAPI(objptr);
     } else if (virStoragePoolObjGetNewDef(obj)) {
         virStoragePoolObjDefUseNewDef(obj);
     }
@@ -771,12 +770,8 @@ storagePoolCreateXML(virConnectPtr conn,
 
         if (build_flags ||
             (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD)) {
-            if (backend->buildPool(obj, build_flags) < 0) {
-                virStoragePoolObjRemove(driver->pools, obj);
-                virObjectUnref(obj);
-                obj = NULL;
-                goto cleanup;
-            }
+            if (backend->buildPool(obj, build_flags) < 0)
+                goto error;
         }
     }
 
@@ -809,8 +804,6 @@ storagePoolCreateXML(virConnectPtr conn,
 
  error:
     virStoragePoolObjRemove(driver->pools, obj);
-    virObjectUnref(obj);
-    obj = NULL;
     goto cleanup;
 }
 
@@ -846,8 +839,6 @@ storagePoolDefineXML(virConnectPtr conn,
 
     if (virStoragePoolObjSaveDef(driver, obj, newDef ? newDef : def) < 0) {
         virStoragePoolObjRemove(driver->pools, obj);
-        virObjectUnref(obj);
-        obj = NULL;
         newDef = NULL;
         goto cleanup;
     }
@@ -914,8 +905,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 b6b4cb0c0b..87b84e979b 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -6450,16 +6450,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;
     }
 
@@ -6517,8 +6513,6 @@ testStoragePoolDefineXML(virConnectPtr conn,
 
     if (testStoragePoolObjSetDefaults(obj) == -1) {
         virStoragePoolObjRemove(privconn->pools, obj);
-        virObjectUnref(obj);
-        obj = NULL;
         goto cleanup;
     }
 
@@ -6547,7 +6541,7 @@ testStoragePoolUndefine(virStoragePoolPtr pool)
                                             0);
 
     virStoragePoolObjRemove(privconn->pools, obj);
-    virObjectUnref(obj);
+    virStoragePoolObjEndAPI(&obj);
 
     virObjectEventStateQueue(privconn->eventState, event);
     return 0;
@@ -6639,11 +6633,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.47.2