Re: [libvirt] PATCH: 16/28: Thread safety for storage driver

Sunday, 30 November 2008

This patch makes the storage driver thread safe

 storage_conf.h   |    2 
 storage_driver.c |  240 +++++++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 202 insertions(+), 40 deletions(-)


Daniel

diff --git a/src/storage_conf.h b/src/storage_conf.h
--- a/src/storage_conf.h
+++ b/src/storage_conf.h
@@ -248,6 +248,8 @@ typedef virStorageDriverState *virStorag
 typedef virStorageDriverState *virStorageDriverStatePtr;
 
 struct _virStorageDriverState {
+    PTHREAD_MUTEX_T(lock);
+
     virStoragePoolObjList pools;
 
     char *configDir;
diff --git a/src/storage_driver.c b/src/storage_driver.c
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -47,6 +47,14 @@ static virStorageDriverStatePtr driverSt
 
 static int storageDriverShutdown(void);
 
+static void storageDriverLock(virStorageDriverStatePtr driver)
+{
+    pthread_mutex_lock(&driver->lock);
+}
+static void storageDriverUnlock(virStorageDriverStatePtr driver)
+{
+    pthread_mutex_unlock(&driver->lock);
+}
 
 static void
 storageDriverAutostart(virStorageDriverStatePtr driver) {
@@ -100,6 +108,9 @@ storageDriverStartup(void) {
     if (VIR_ALLOC(driverState) < 0)
         return -1;
 
+    pthread_mutex_init(&driverState->lock, NULL);
+    storageDriverLock(driverState);
+
     if (!uid) {
         if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL)
             goto out_of_memory;
@@ -132,8 +143,7 @@ storageDriverStartup(void) {
                   "%s/storage/autostart", base) == -1)
         goto out_of_memory;
 
-    free(base);
-    base = NULL;
+    VIR_FREE(base);
 
     /*
     if (virStorageLoadDriverConfig(driver, driverConf) < 0) {
@@ -145,19 +155,19 @@ storageDriverStartup(void) {
     if (virStoragePoolLoadAllConfigs(NULL,
                                      &driverState->pools,
                                      driverState->configDir,
-                                     driverState->autostartDir) < 0) {
-        storageDriverShutdown();
-        return -1;
-    }
+                                     driverState->autostartDir) < 0)
+        goto error;
     storageDriverAutostart(driverState);
 
+    storageDriverUnlock(driverState);
     return 0;
 
- out_of_memory:
+out_of_memory:
     storageLog("virStorageStartup: out of memory");
-    free(base);
-    free(driverState);
-    driverState = NULL;
+error:
+    VIR_FREE(base);
+    storageDriverUnlock(driverState);
+    storageDriverShutdown();
     return -1;
 }
 
@@ -172,11 +182,13 @@ storageDriverReload(void) {
     if (!driverState)
         return -1;
 
+    storageDriverLock(driverState);
     virStoragePoolLoadAllConfigs(NULL,
                                  &driverState->pools,
                                  driverState->configDir,
                                  driverState->autostartDir);
     storageDriverAutostart(driverState);
+    storageDriverUnlock(driverState);
 
     return 0;
 }
@@ -191,19 +203,22 @@ static int
 static int
 storageDriverActive(void) {
     unsigned int i;
+    int active = 0;
 
     if (!driverState)
         return 0;
 
-    /* If we've any active networks or guests, then we
-     * mark this driver as active
-     */
-    for (i = 0 ; i < driverState->pools.count ; i++)
+    storageDriverLock(driverState);
+
+    for (i = 0 ; i < driverState->pools.count ; i++) {
+        virStoragePoolObjLock(driverState->pools.objs[i]);
         if (virStoragePoolObjIsActive(driverState->pools.objs[i]))
-            return 1;
+            active = 1;
+        virStoragePoolObjUnlock(driverState->pools.objs[i]);
+    }
 
-    /* Otherwise we're happy to deal with a shutdown */
-    return 0;
+    storageDriverUnlock(driverState);
+    return active;
 }
 
 /**
@@ -218,6 +233,7 @@ storageDriverShutdown(void) {
     if (!driverState)
         return -1;
 
+    storageDriverLock(driverState);
     /* shutdown active pools */
     for (i = 0 ; i < driverState->pools.count ; i++) {
         virStoragePoolObjPtr pool = driverState->pools.objs[i];
@@ -244,6 +260,7 @@ storageDriverShutdown(void) {
 
     VIR_FREE(driverState->configDir);
     VIR_FREE(driverState->autostartDir);
+    storageDriverUnlock(driverState);
     VIR_FREE(driverState);
 
     return 0;
@@ -258,7 +275,10 @@ storagePoolLookupByUUID(virConnectPtr co
     virStoragePoolObjPtr pool;
     virStoragePoolPtr ret = NULL;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(conn, VIR_ERR_NO_STORAGE_POOL,
                               "%s", _("no pool with matching
uuid"));
@@ -268,6 +288,8 @@ storagePoolLookupByUUID(virConnectPtr co
     ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid);
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -278,7 +300,10 @@ storagePoolLookupByName(virConnectPtr co
     virStoragePoolObjPtr pool;
     virStoragePoolPtr ret = NULL;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByName(&driver->pools, name);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(conn, VIR_ERR_NO_STORAGE_POOL,
                               "%s", _("no pool with matching
name"));
@@ -288,6 +313,8 @@ storagePoolLookupByName(virConnectPtr co
     ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid);
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -318,9 +345,14 @@ storageNumPools(virConnectPtr conn) {
     virStorageDriverStatePtr driver = conn->storagePrivateData;
     unsigned int i, nactive = 0;
 
-    for (i = 0 ; i < driver->pools.count ; i++)
+    storageDriverLock(driver);
+    for (i = 0 ; i < driver->pools.count ; i++) {
+        virStoragePoolObjLock(driver->pools.objs[i]);
         if (virStoragePoolObjIsActive(driver->pools.objs[i]))
             nactive++;
+        virStoragePoolObjUnlock(driver->pools.objs[i]);
+    }
+    storageDriverUnlock(driver);
 
     return nactive;
 }
@@ -332,23 +364,27 @@ storageListPools(virConnectPtr conn,
     virStorageDriverStatePtr driver = conn->storagePrivateData;
     int got = 0, i;
 
+    storageDriverLock(driver);
     for (i = 0 ; i < driver->pools.count && got < nnames ; i++) {
+        virStoragePoolObjLock(driver->pools.objs[i]);
         if (virStoragePoolObjIsActive(driver->pools.objs[i])) {
             if (!(names[got] = strdup(driver->pools.objs[i]->def->name))) {
+                virStoragePoolObjUnlock(driver->pools.objs[i]);
                 virStorageReportError(conn, VIR_ERR_NO_MEMORY,
                                       "%s", _("names"));
                 goto cleanup;
             }
             got++;
         }
+        virStoragePoolObjUnlock(driver->pools.objs[i]);
     }
+    storageDriverUnlock(driver);
     return got;
 
  cleanup:
-    for (i = 0 ; i < got ; i++) {
-        free(names[i]);
-        names[i] = NULL;
-    }
+    storageDriverUnlock(driver);
+    for (i = 0 ; i < got ; i++)
+        VIR_FREE(names[i]);
     memset(names, 0, nnames);
     return -1;
 }
@@ -358,9 +394,14 @@ storageNumDefinedPools(virConnectPtr con
     virStorageDriverStatePtr driver = conn->storagePrivateData;
     unsigned int i, nactive = 0;
 
-    for (i = 0 ; i < driver->pools.count ; i++)
+    storageDriverLock(driver);
+    for (i = 0 ; i < driver->pools.count ; i++) {
+        virStoragePoolObjLock(driver->pools.objs[i]);
         if (!virStoragePoolObjIsActive(driver->pools.objs[i]))
             nactive++;
+        virStoragePoolObjUnlock(driver->pools.objs[i]);
+    }
+    storageDriverUnlock(driver);
 
     return nactive;
 }
@@ -372,19 +413,25 @@ storageListDefinedPools(virConnectPtr co
     virStorageDriverStatePtr driver = conn->storagePrivateData;
     int got = 0, i;
 
+    storageDriverLock(driver);
     for (i = 0 ; i < driver->pools.count && got < nnames ; i++) {
+        virStoragePoolObjLock(driver->pools.objs[i]);
         if (!virStoragePoolObjIsActive(driver->pools.objs[i])) {
             if (!(names[got] = strdup(driver->pools.objs[i]->def->name))) {
+                virStoragePoolObjUnlock(driver->pools.objs[i]);
                 virStorageReportError(conn, VIR_ERR_NO_MEMORY,
                                       "%s", _("names"));
                 goto cleanup;
             }
             got++;
         }
+        virStoragePoolObjUnlock(driver->pools.objs[i]);
     }
+    storageDriverUnlock(driver);
     return got;
 
  cleanup:
+    storageDriverUnlock(driver);
     for (i = 0 ; i < got ; i++) {
         free(names[i]);
         names[i] = NULL;
@@ -393,6 +440,8 @@ storageListDefinedPools(virConnectPtr co
     return -1;
 }
 
+/* This method is required to be re-entrant / thread safe, so
+   uses no driver lock */
 static char *
 storageFindPoolSources(virConnectPtr conn,
                        const char *type,
@@ -425,10 +474,11 @@ storagePoolCreate(virConnectPtr conn,
                   unsigned int flags ATTRIBUTE_UNUSED) {
     virStorageDriverStatePtr driver = conn->storagePrivateData;
     virStoragePoolDefPtr def;
-    virStoragePoolObjPtr pool;
+    virStoragePoolObjPtr pool = NULL;
     virStoragePoolPtr ret = NULL;
     virStorageBackendPtr backend;
 
+    storageDriverLock(driver);
     if (!(def = virStoragePoolDefParse(conn, xml, NULL)))
         goto cleanup;
 
@@ -464,6 +514,9 @@ storagePoolCreate(virConnectPtr conn,
 
 cleanup:
     virStoragePoolDefFree(def);
+    if (pool)
+        virStoragePoolObjUnlock(pool);
+    storageDriverUnlock(driver);
     return ret;
 }
 
@@ -473,10 +526,11 @@ storagePoolDefine(virConnectPtr conn,
                   unsigned int flags ATTRIBUTE_UNUSED) {
     virStorageDriverStatePtr driver = conn->storagePrivateData;
     virStoragePoolDefPtr def;
-    virStoragePoolObjPtr pool;
+    virStoragePoolObjPtr pool = NULL;
     virStoragePoolPtr ret = NULL;
     virStorageBackendPtr backend;
 
+    storageDriverLock(driver);
     if (!(def = virStoragePoolDefParse(conn, xml, NULL)))
         goto cleanup;
 
@@ -496,6 +550,9 @@ storagePoolDefine(virConnectPtr conn,
 
 cleanup:
     virStoragePoolDefFree(def);
+    if (pool)
+        virStoragePoolObjUnlock(pool);
+    storageDriverUnlock(driver);
     return ret;
 }
 
@@ -505,6 +562,7 @@ storagePoolUndefine(virStoragePoolPtr ob
     virStoragePoolObjPtr pool;
     int ret = -1;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
@@ -529,9 +587,13 @@ storagePoolUndefine(virStoragePoolPtr ob
     VIR_FREE(pool->autostartLink);
 
     virStoragePoolObjRemove(&driver->pools, pool);
+    pool = NULL;
     ret = 0;
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
+    storageDriverUnlock(driver);
     return ret;
 }
 
@@ -543,7 +605,10 @@ storagePoolStart(virStoragePoolPtr obj,
     virStorageBackendPtr backend;
     int ret = -1;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -572,6 +637,8 @@ storagePoolStart(virStoragePoolPtr obj,
     ret = 0;
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -583,7 +650,10 @@ storagePoolBuild(virStoragePoolPtr obj,
     virStorageBackendPtr backend;
     int ret = -1;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -605,6 +675,8 @@ storagePoolBuild(virStoragePoolPtr obj,
     ret = 0;
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -616,7 +688,9 @@ storagePoolDestroy(virStoragePoolPtr obj
     virStorageBackendPtr backend;
     int ret = -1;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -640,11 +714,16 @@ storagePoolDestroy(virStoragePoolPtr obj
 
     pool->active = 0;
 
-    if (pool->configFile == NULL)
+    if (pool->configFile == NULL) {
         virStoragePoolObjRemove(&driver->pools, pool);
+        pool = NULL;
+    }
     ret = 0;
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
+    storageDriverUnlock(driver);
     return ret;
 }
 
@@ -657,7 +736,10 @@ storagePoolDelete(virStoragePoolPtr obj,
     virStorageBackendPtr backend;
     int ret = -1;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -683,6 +765,8 @@ storagePoolDelete(virStoragePoolPtr obj,
     ret = 0;
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -695,7 +779,10 @@ storagePoolRefresh(virStoragePoolPtr obj
     virStorageBackendPtr backend;
     int ret = -1;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -718,13 +805,17 @@ storagePoolRefresh(virStoragePoolPtr obj
 
         pool->active = 0;
 
-        if (pool->configFile == NULL)
+        if (pool->configFile == NULL) {
             virStoragePoolObjRemove(&driver->pools, pool);
+            pool = NULL;
+        }
         goto cleanup;
     }
     ret = 0;
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -737,7 +828,10 @@ storagePoolGetInfo(virStoragePoolPtr obj
     virStorageBackendPtr backend;
     int ret = -1;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -758,6 +852,8 @@ storagePoolGetInfo(virStoragePoolPtr obj
     ret = 0;
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -768,7 +864,10 @@ storagePoolDumpXML(virStoragePoolPtr obj
     virStoragePoolObjPtr pool;
     char *ret = NULL;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -778,6 +877,8 @@ storagePoolDumpXML(virStoragePoolPtr obj
     ret = virStoragePoolDefFormat(obj->conn, pool->def);
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -788,7 +889,10 @@ storagePoolGetAutostart(virStoragePoolPt
     virStoragePoolObjPtr pool;
     int ret = -1;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no pool with matching
uuid"));
@@ -803,6 +907,8 @@ storagePoolGetAutostart(virStoragePoolPt
     ret = 0;
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -813,7 +919,10 @@ storagePoolSetAutostart(virStoragePoolPt
     virStoragePoolObjPtr pool;
     int ret = -1;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no pool with matching
uuid"));
@@ -861,6 +970,8 @@ storagePoolSetAutostart(virStoragePoolPt
     ret = 0;
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -871,7 +982,10 @@ storagePoolNumVolumes(virStoragePoolPtr 
     virStoragePoolObjPtr pool;
     int ret = -1;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -886,6 +1000,8 @@ storagePoolNumVolumes(virStoragePoolPtr 
     ret = pool->volumes.count;
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -899,7 +1015,10 @@ storagePoolListVolumes(virStoragePoolPtr
 
     memset(names, 0, maxnames);
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -920,9 +1039,12 @@ storagePoolListVolumes(virStoragePoolPtr
         }
     }
 
+    virStoragePoolObjUnlock(pool);
     return n;
 
  cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     for (n = 0 ; n < maxnames ; n++)
         VIR_FREE(names[n]);
 
@@ -939,7 +1061,10 @@ storageVolumeLookupByName(virStoragePool
     virStorageVolDefPtr vol;
     virStorageVolPtr ret = NULL;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -963,6 +1088,8 @@ storageVolumeLookupByName(virStoragePool
     ret = virGetStorageVol(obj->conn, pool->def->name, vol->name,
vol->key);
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -974,20 +1101,22 @@ storageVolumeLookupByKey(virConnectPtr c
     unsigned int i;
     virStorageVolPtr ret = NULL;
 
-    for (i = 0 ; i < driver->pools.count ; i++) {
+    storageDriverLock(driver);
+    for (i = 0 ; i < driver->pools.count && !ret ; i++) {
+        virStoragePoolObjLock(driver->pools.objs[i]);
         if (virStoragePoolObjIsActive(driver->pools.objs[i])) {
             virStorageVolDefPtr vol =
                 virStorageVolDefFindByKey(driver->pools.objs[i], key);
 
-            if (vol) {
+            if (vol)
                 ret = virGetStorageVol(conn,
-                                        driver->pools.objs[i]->def->name,
-                                        vol->name,
-                                        vol->key);
-                break;
-            }
+                                       driver->pools.objs[i]->def->name,
+                                       vol->name,
+                                       vol->key);
         }
+        virStoragePoolObjUnlock(driver->pools.objs[i]);
     }
+    storageDriverUnlock(driver);
 
     if (!ret)
         virStorageReportError(conn, VIR_ERR_INVALID_STORAGE_VOL,
@@ -1003,7 +1132,9 @@ storageVolumeLookupByPath(virConnectPtr 
     unsigned int i;
     virStorageVolPtr ret = NULL;
 
-    for (i = 0 ; i < driver->pools.count ; i++) {
+    storageDriverLock(driver);
+    for (i = 0 ; i < driver->pools.count && !ret ; i++) {
+        virStoragePoolObjLock(driver->pools.objs[i]);
         if (virStoragePoolObjIsActive(driver->pools.objs[i])) {
             virStorageVolDefPtr vol;
             const char *stable_path;
@@ -1016,21 +1147,22 @@ storageVolumeLookupByPath(virConnectPtr 
              * virStorageReportError if it fails; we just need to keep
              * propagating the return code
              */
-            if (stable_path == NULL)
+            if (stable_path == NULL) {
+                virStoragePoolObjUnlock(driver->pools.objs[i]);
                 goto cleanup;
+            }
 
             vol = virStorageVolDefFindByPath(driver->pools.objs[i],
                                              stable_path);
             VIR_FREE(stable_path);
 
-            if (vol) {
+            if (vol)
                 ret = virGetStorageVol(conn,
                                        driver->pools.objs[i]->def->name,
                                        vol->name,
                                        vol->key);
-                break;
-            }
         }
+        virStoragePoolObjUnlock(driver->pools.objs[i]);
     }
 
     if (!ret)
@@ -1038,6 +1170,7 @@ storageVolumeLookupByPath(virConnectPtr 
                               "%s", _("no storage vol with matching
path"));
 
 cleanup:
+    storageDriverUnlock(driver);
     return ret;
 }
 
@@ -1051,7 +1184,10 @@ storageVolumeCreateXML(virStoragePoolPtr
     virStorageVolDefPtr vol = NULL;
     virStorageVolPtr ret = NULL;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -1089,6 +1225,7 @@ storageVolumeCreateXML(virStoragePoolPtr
         goto cleanup;
     }
 
+    /* XXX ought to drop pool lock while creating instance */
     if (backend->createVol(obj->conn, pool, vol) < 0) {
         goto cleanup;
     }
@@ -1100,6 +1237,8 @@ storageVolumeCreateXML(virStoragePoolPtr
 
 cleanup:
     virStorageVolDefFree(vol);
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -1113,7 +1252,10 @@ storageVolumeDelete(virStorageVolPtr obj
     unsigned int i;
     int ret = -1;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByName(&driver->pools, obj->pool);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -1168,6 +1310,8 @@ storageVolumeDelete(virStorageVolPtr obj
 
 cleanup:
     virStorageVolDefFree(vol);
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -1180,7 +1324,10 @@ storageVolumeGetInfo(virStorageVolPtr ob
     virStorageVolDefPtr vol;
     int ret = -1;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByName(&driver->pools, obj->pool);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -1215,6 +1362,8 @@ storageVolumeGetInfo(virStorageVolPtr ob
     ret = 0;
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
@@ -1227,7 +1376,10 @@ storageVolumeGetXMLDesc(virStorageVolPtr
     virStorageVolDefPtr vol;
     char *ret = NULL;
 
+    storageDriverLock(driver);
     pool = virStoragePoolObjFindByName(&driver->pools, obj->pool);
+    storageDriverUnlock(driver);
+
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
                               "%s", _("no storage pool with matching
uuid"));
@@ -1254,15 +1406,21 @@ storageVolumeGetXMLDesc(virStorageVolPtr
     ret = virStorageVolDefFormat(obj->conn, pool->def, vol);
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 
 static char *
 storageVolumeGetPath(virStorageVolPtr obj) {
     virStorageDriverStatePtr driver = obj->conn->storagePrivateData;
-    virStoragePoolObjPtr pool = virStoragePoolObjFindByName(&driver->pools,
obj->pool);
+    virStoragePoolObjPtr pool;
     virStorageVolDefPtr vol;
     char *ret = NULL;
+
+    storageDriverLock(driver);
+    pool = virStoragePoolObjFindByName(&driver->pools, obj->pool);
+    storageDriverUnlock(driver);
 
     if (!pool) {
         virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
@@ -1289,6 +1447,8 @@ storageVolumeGetPath(virStorageVolPtr ob
         virStorageReportError(obj->conn, VIR_ERR_NO_MEMORY, "%s",
_("path"));
 
 cleanup:
+    if (pool)
+        virStoragePoolObjUnlock(pool);
     return ret;
 }
 

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

Re: [libvirt] PATCH: 16/28: Thread safety for storage driver