storage: Introduce virStoragePoolObjListSearch

Create an API to search through the storage pool objects looking for
a specific truism from a callback API in order to return the specific
storage pool object that is desired.

src/conf/virstorageobj.c

@@ -259,6 +259,38 @@ virStoragePoolObjListForEach(virStoragePoolObjListPtr pools,
 }
 
 
+/**
+ * virStoragePoolObjListSearch
+ * @pools: Pointer to pools object
+ * @search: Callback searcher helper
+ * @opaque: Opaque data to use as argument to helper
+ *
+ * Search through the @pools objects calling the @search helper using
+ * the @opaque data in order to find an object that matches some criteria
+ * and return that object locked.
+ *
+ * Returns a locked object when found and NULL when not found
+ */
+virStoragePoolObjPtr
+virStoragePoolObjListSearch(virStoragePoolObjListPtr pools,
+                            virStoragePoolObjListSearcher searcher,
+                            const void *opaque)
+{
+    size_t i;
+    virStoragePoolObjPtr obj;
+
+    for (i = 0; i < pools->count; i++) {
+        obj = pools->objs[i];
+        virStoragePoolObjLock(obj);
+        if (searcher(obj, opaque))
+            return obj;
+        virStoragePoolObjUnlock(obj);
+    }
+
+    return NULL;
+}
+
+
 void
 virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
                         virStoragePoolObjPtr obj)

src/conf/virstorageobj.h

@@ -235,6 +235,15 @@ virStoragePoolObjListForEach(virStoragePoolObjListPtr pools,
                              virStoragePoolObjListIterator iter,
                              const void *opaque);
 
+typedef bool
+(*virStoragePoolObjListSearcher)(virStoragePoolObjPtr obj,
+                                 const void *opaque);
+
+virStoragePoolObjPtr
+virStoragePoolObjListSearch(virStoragePoolObjListPtr pools,
+                            virStoragePoolObjListSearcher searcher,
+                            const void *opaque);
+
 void
 virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
                         virStoragePoolObjPtr obj);

src/libvirt_private.syms

@@ -1092,6 +1092,7 @@ virStoragePoolObjIsDuplicate;
 virStoragePoolObjListExport;
 virStoragePoolObjListForEach;
 virStoragePoolObjListFree;
+virStoragePoolObjListSearch;
 virStoragePoolObjLoadAllConfigs;
 virStoragePoolObjLoadAllState;
 virStoragePoolObjLock;

src/storage/storage_driver.c

@@ -1489,148 +1489,174 @@ storageVolLookupByName(virStoragePoolPtr pool,
 }
 
 
+struct storageVolLookupData {
+    virConnectPtr conn;
+    const char *key;
+    char *cleanpath;
+    const char *path;
+    virStorageVolDefPtr voldef;
+};
+
+static bool
+storageVolLookupByKeyCallback(virStoragePoolObjPtr obj,
+                              const void *opaque)
+{
+    struct storageVolLookupData *data = (struct storageVolLookupData *) opaque;
+
+    if (virStoragePoolObjIsActive(obj))
+        data->voldef = virStorageVolDefFindByKey(obj, data->key);
+
+    return !!data->voldef;
+}
+
+
 static virStorageVolPtr
 storageVolLookupByKey(virConnectPtr conn,
                       const char *key)
 {
-    size_t i;
+    virStoragePoolObjPtr obj;
+    virStoragePoolDefPtr def;
+    struct storageVolLookupData data = {
+        .conn = conn, .key = key, .voldef = NULL };
     virStorageVolPtr vol = NULL;
 
     storageDriverLock();
-    for (i = 0; i < driver->pools.count && !vol; i++) {
+    if ((obj = virStoragePoolObjListSearch(&driver->pools,
-        virStoragePoolObjPtr obj = driver->pools.objs[i];
+                                           storageVolLookupByKeyCallback,
-        virStoragePoolDefPtr def;
+                                           &data)) && data.voldef) {
-
-        virStoragePoolObjLock(obj);
         def = virStoragePoolObjGetDef(obj);
-        if (virStoragePoolObjIsActive(obj)) {
+        if (virStorageVolLookupByKeyEnsureACL(conn, def, data.voldef) == 0) {
-            virStorageVolDefPtr voldef = virStorageVolDefFindByKey(obj, key);
+            vol = virGetStorageVol(conn, def->name,
-
+                                   data.voldef->name, data.voldef->key,
-            if (voldef) {
+                                   NULL, NULL);
-                if (virStorageVolLookupByKeyEnsureACL(conn, def, voldef) < 0) {
-                    virStoragePoolObjEndAPI(&obj);
-                    goto cleanup;
-                }
-
-                vol = virGetStorageVol(conn, def->name,
-                                       voldef->name, voldef->key,
-                                       NULL, NULL);
-            }
         }
         virStoragePoolObjEndAPI(&obj);
     }
+    storageDriverUnlock();
 
     if (!vol)
         virReportError(VIR_ERR_NO_STORAGE_VOL,
                        _("no storage vol with matching key %s"), key);
 
- cleanup:
-    storageDriverUnlock();
     return vol;
 }
 
+
+static bool
+storageVolLookupByPathCallback(virStoragePoolObjPtr obj,
+                               const void *opaque)
+{
+    struct storageVolLookupData *data = (struct storageVolLookupData *) opaque;
+    virStoragePoolDefPtr def;
+    char *stable_path = NULL;
+
+    if (!virStoragePoolObjIsActive(obj))
+        return false;
+
+    def = virStoragePoolObjGetDef(obj);
+
+    switch ((virStoragePoolType) def->type) {
+        case VIR_STORAGE_POOL_DIR:
+        case VIR_STORAGE_POOL_FS:
+        case VIR_STORAGE_POOL_NETFS:
+        case VIR_STORAGE_POOL_LOGICAL:
+        case VIR_STORAGE_POOL_DISK:
+        case VIR_STORAGE_POOL_ISCSI:
+        case VIR_STORAGE_POOL_SCSI:
+        case VIR_STORAGE_POOL_MPATH:
+        case VIR_STORAGE_POOL_VSTORAGE:
+            stable_path = virStorageBackendStablePath(obj, data->cleanpath,
+                                                      false);
+            break;
+
+        case VIR_STORAGE_POOL_GLUSTER:
+        case VIR_STORAGE_POOL_RBD:
+        case VIR_STORAGE_POOL_SHEEPDOG:
+        case VIR_STORAGE_POOL_ZFS:
+        case VIR_STORAGE_POOL_LAST:
+            ignore_value(VIR_STRDUP(stable_path, data->path));
+            break;
+    }
+
+    /* Don't break the whole lookup process if it fails on
+     * getting the stable path for some of the pools. */
+    if (!stable_path) {
+        VIR_WARN("Failed to get stable path for pool '%s'", def->name);
+        return false;
+    }
+
+    data->voldef = virStorageVolDefFindByPath(obj, stable_path);
+    VIR_FREE(stable_path);
+
+    return !!data->voldef;
+}
+
+
 static virStorageVolPtr
 storageVolLookupByPath(virConnectPtr conn,
                        const char *path)
 {
-    size_t i;
+    virStoragePoolObjPtr obj;
+    virStoragePoolDefPtr def;
+    struct storageVolLookupData data = {
+        .conn = conn, .path = path, .voldef = NULL };
     virStorageVolPtr vol = NULL;
-    char *cleanpath;
 
-    cleanpath = virFileSanitizePath(path);
+    if (!(data.cleanpath = virFileSanitizePath(path)))
-    if (!cleanpath)
         return NULL;
 
     storageDriverLock();
-    for (i = 0; i < driver->pools.count && !vol; i++) {
+    if ((obj = virStoragePoolObjListSearch(&driver->pools,
-        virStoragePoolObjPtr obj = driver->pools.objs[i];
+                                           storageVolLookupByPathCallback,
-        virStoragePoolDefPtr def;
+                                           &data)) && data.voldef) {
-        virStorageVolDefPtr voldef;
-        char *stable_path = NULL;
-
-        virStoragePoolObjLock(obj);
         def = virStoragePoolObjGetDef(obj);
 
-        if (!virStoragePoolObjIsActive(obj)) {
+        if (virStorageVolLookupByPathEnsureACL(conn, def, data.voldef) == 0) {
-           virStoragePoolObjEndAPI(&obj);
-           continue;
-        }
-
-        switch ((virStoragePoolType) def->type) {
-            case VIR_STORAGE_POOL_DIR:
-            case VIR_STORAGE_POOL_FS:
-            case VIR_STORAGE_POOL_NETFS:
-            case VIR_STORAGE_POOL_LOGICAL:
-            case VIR_STORAGE_POOL_DISK:
-            case VIR_STORAGE_POOL_ISCSI:
-            case VIR_STORAGE_POOL_SCSI:
-            case VIR_STORAGE_POOL_MPATH:
-            case VIR_STORAGE_POOL_VSTORAGE:
-                stable_path = virStorageBackendStablePath(obj,
-                                                          cleanpath,
-                                                          false);
-                if (stable_path == NULL) {
-                    /* Don't break the whole lookup process if it fails on
-                     * getting the stable path for some of the pools.
-                     */
-                    VIR_WARN("Failed to get stable path for pool '%s'",
-                             def->name);
-                    virStoragePoolObjEndAPI(&obj);
-                    continue;
-                }
-                break;
-
-            case VIR_STORAGE_POOL_GLUSTER:
-            case VIR_STORAGE_POOL_RBD:
-            case VIR_STORAGE_POOL_SHEEPDOG:
-            case VIR_STORAGE_POOL_ZFS:
-            case VIR_STORAGE_POOL_LAST:
-                if (VIR_STRDUP(stable_path, path) < 0) {
-                     virStoragePoolObjEndAPI(&obj);
-                    goto cleanup;
-                }
-                break;
-        }
-
-        voldef = virStorageVolDefFindByPath(obj, stable_path);
-        VIR_FREE(stable_path);
-
-        if (voldef) {
-            if (virStorageVolLookupByPathEnsureACL(conn, def, voldef) < 0) {
-                virStoragePoolObjEndAPI(&obj);
-                goto cleanup;
-            }
-
             vol = virGetStorageVol(conn, def->name,
-                                   voldef->name, voldef->key,
+                                   data.voldef->name, data.voldef->key,
                                    NULL, NULL);
         }
-
         virStoragePoolObjEndAPI(&obj);
     }
+    storageDriverUnlock();
 
     if (!vol) {
-        if (STREQ(path, cleanpath)) {
+        if (STREQ(path, data.cleanpath)) {
             virReportError(VIR_ERR_NO_STORAGE_VOL,
                            _("no storage vol with matching path '%s'"), path);
         } else {
             virReportError(VIR_ERR_NO_STORAGE_VOL,
                            _("no storage vol with matching path '%s' (%s)"),
-                           path, cleanpath);
+                           path, data.cleanpath);
         }
     }
 
- cleanup:
+    VIR_FREE(data.cleanpath);
-    VIR_FREE(cleanpath);
-    storageDriverUnlock();
     return vol;
 }
 
+
+static bool
+storagePoolLookupByTargetPathCallback(virStoragePoolObjPtr obj,
+                                      const void *opaque)
+{
+    const char *path = opaque;
+    virStoragePoolDefPtr def;
+
+    if (!virStoragePoolObjIsActive(obj))
+        return false;
+
+    def = virStoragePoolObjGetDef(obj);
+    return STREQ(path, def->target.path);
+}
+
+
 virStoragePoolPtr
 storagePoolLookupByTargetPath(virConnectPtr conn,
                               const char *path)
 {
-    size_t i;
+    virStoragePoolObjPtr obj;
+    virStoragePoolDefPtr def;
     virStoragePoolPtr pool = NULL;
     char *cleanpath;
 
@@ -1639,21 +1665,11 @@ storagePoolLookupByTargetPath(virConnectPtr conn,
         return NULL;
 
     storageDriverLock();
-    for (i = 0; i < driver->pools.count && !pool; i++) {
+    if ((obj == virStoragePoolObjListSearch(&driver->pools,
-        virStoragePoolObjPtr obj = driver->pools.objs[i];
+                                            storagePoolLookupByTargetPathCallback,
-        virStoragePoolDefPtr def;
+                                            path))) {
-
-        virStoragePoolObjLock(obj);
         def = virStoragePoolObjGetDef(obj);
-
+        pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
-        if (!virStoragePoolObjIsActive(obj)) {
-            virStoragePoolObjEndAPI(&obj);
-            continue;
-        }
-
-        if (STREQ(path, def->target.path))
-            pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
-
         virStoragePoolObjEndAPI(&obj);
     }
     storageDriverUnlock();

src/test/test_driver.c

@@ -4908,6 +4908,26 @@ testStorageVolLookupByName(virStoragePoolPtr pool,
 }
 
 
+struct storageVolLookupData {
+    virConnectPtr conn;
+    const char *key;
+    const char *path;
+    virStorageVolDefPtr voldef;
+};
+
+static bool
+testStorageVolLookupByKeyCallback(virStoragePoolObjPtr obj,
+                                  const void *opaque)
+{
+    struct storageVolLookupData *data = (struct storageVolLookupData *) opaque;
+
+    if (virStoragePoolObjIsActive(obj))
+        data->voldef = virStorageVolDefFindByKey(obj, data->key);
+
+    return !!data->voldef;
+}
+
+
 static virStorageVolPtr
 testStorageVolLookupByKey(virConnectPtr conn,
                           const char *key)
@@ -4915,34 +4935,40 @@ testStorageVolLookupByKey(virConnectPtr conn,
     testDriverPtr privconn = conn->privateData;
     virStoragePoolObjPtr obj;
     virStoragePoolDefPtr def;
-    size_t i;
+    struct storageVolLookupData data = {
-    virStorageVolPtr ret = NULL;
+        .conn = conn, .key = key, .voldef = NULL };
+    virStorageVolPtr vol = NULL;
 
     testDriverLock(privconn);
-    for (i = 0; i < privconn->pools.count; i++) {
+    if ((obj = virStoragePoolObjListSearch(&privconn->pools,
-        obj = privconn->pools.objs[i];
+                                           testStorageVolLookupByKeyCallback,
-        virStoragePoolObjLock(obj);
+                                           &data)) && data.voldef) {
         def = virStoragePoolObjGetDef(obj);
-        if (virStoragePoolObjIsActive(obj)) {
+        vol = virGetStorageVol(conn, def->name,
-            virStorageVolDefPtr privvol = virStorageVolDefFindByKey(obj, key);
+                               data.voldef->name, data.voldef->key,
-
+                               NULL, NULL);
-            if (privvol) {
-                ret = virGetStorageVol(conn, def->name,
-                                       privvol->name, privvol->key,
-                                       NULL, NULL);
-                virStoragePoolObjEndAPI(&obj);
-                break;
-            }
-        }
         virStoragePoolObjEndAPI(&obj);
     }
     testDriverUnlock(privconn);
 
-    if (!ret)
+    if (!vol)
         virReportError(VIR_ERR_NO_STORAGE_VOL,
                        _("no storage vol with matching key '%s'"), key);
 
-    return ret;
+    return vol;
+}
+
+
+static bool
+testStorageVolLookupByPathCallback(virStoragePoolObjPtr obj,
+                                   const void *opaque)
+{
+    struct storageVolLookupData *data = (struct storageVolLookupData *) opaque;
+
+    if (virStoragePoolObjIsActive(obj))
+        data->voldef = virStorageVolDefFindByPath(obj, data->path);
+
+    return !!data->voldef;
 }
 
 
@@ -4953,34 +4979,27 @@ testStorageVolLookupByPath(virConnectPtr conn,
     testDriverPtr privconn = conn->privateData;
     virStoragePoolObjPtr obj;
     virStoragePoolDefPtr def;
-    size_t i;
+    struct storageVolLookupData data = {
-    virStorageVolPtr ret = NULL;
+        .conn = conn, .path = path, .voldef = NULL };
+    virStorageVolPtr vol = NULL;
 
     testDriverLock(privconn);
-    for (i = 0; i < privconn->pools.count; i++) {
+    if ((obj = virStoragePoolObjListSearch(&privconn->pools,
-        obj = privconn->pools.objs[i];
+                                           testStorageVolLookupByPathCallback,
-        virStoragePoolObjLock(obj);
+                                           &data)) && data.voldef) {
         def = virStoragePoolObjGetDef(obj);
-        if (virStoragePoolObjIsActive(obj)) {
+        vol = virGetStorageVol(conn, def->name,
-            virStorageVolDefPtr privvol = virStorageVolDefFindByPath(obj, path);
+                               data.voldef->name, data.voldef->key,
-
+                               NULL, NULL);
-            if (privvol) {
-                ret = virGetStorageVol(conn, def->name,
-                                       privvol->name, privvol->key,
-                                       NULL, NULL);
-                virStoragePoolObjEndAPI(&obj);
-                break;
-            }
-        }
         virStoragePoolObjEndAPI(&obj);
     }
     testDriverUnlock(privconn);
 
-    if (!ret)
+    if (!vol)
         virReportError(VIR_ERR_NO_STORAGE_VOL,
                        _("no storage vol with matching path '%s'"), path);
 
-    return ret;
+    return vol;
 }