From 5d5c732d748d644ec14626bce448e84bdc4bd93e Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Sun, 8 Oct 2017 08:44:31 -0400 Subject: [PATCH] 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 | 32 +++++ src/conf/virstorageobj.h | 9 ++ src/libvirt_private.syms | 1 + src/storage/storage_driver.c | 224 +++++++++++++++++++---------------- src/test/test_driver.c | 91 ++++++++------ 5 files changed, 217 insertions(+), 140 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 3cae34d970..eb8a57324b 100644 --- a/src/conf/virstorageobj.c +++ b/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) diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h index c84877694e..7fe4a9f68a 100644 --- a/src/conf/virstorageobj.h +++ b/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); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9fb302b693..58e8cebb3f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1092,6 +1092,7 @@ virStoragePoolObjIsDuplicate; virStoragePoolObjListExport; virStoragePoolObjListForEach; virStoragePoolObjListFree; +virStoragePoolObjListSearch; virStoragePoolObjLoadAllConfigs; virStoragePoolObjLoadAllState; virStoragePoolObjLock; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 454b1b0df0..abb5e40c4a 100644 --- a/src/storage/storage_driver.c +++ b/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++) { - virStoragePoolObjPtr obj = driver->pools.objs[i]; - virStoragePoolDefPtr def; - - virStoragePoolObjLock(obj); + if ((obj = virStoragePoolObjListSearch(&driver->pools, + storageVolLookupByKeyCallback, + &data)) && data.voldef) { def = virStoragePoolObjGetDef(obj); - if (virStoragePoolObjIsActive(obj)) { - virStorageVolDefPtr voldef = virStorageVolDefFindByKey(obj, key); - - if (voldef) { - if (virStorageVolLookupByKeyEnsureACL(conn, def, voldef) < 0) { - virStoragePoolObjEndAPI(&obj); - goto cleanup; - } - - vol = virGetStorageVol(conn, def->name, - voldef->name, voldef->key, - NULL, NULL); - } + if (virStorageVolLookupByKeyEnsureACL(conn, def, data.voldef) == 0) { + vol = virGetStorageVol(conn, def->name, + data.voldef->name, data.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 (!cleanpath) + if (!(data.cleanpath = virFileSanitizePath(path))) return NULL; storageDriverLock(); - for (i = 0; i < driver->pools.count && !vol; i++) { - virStoragePoolObjPtr obj = driver->pools.objs[i]; - virStoragePoolDefPtr def; - virStorageVolDefPtr voldef; - char *stable_path = NULL; - - virStoragePoolObjLock(obj); + if ((obj = virStoragePoolObjListSearch(&driver->pools, + storageVolLookupByPathCallback, + &data)) && data.voldef) { def = virStoragePoolObjGetDef(obj); - if (!virStoragePoolObjIsActive(obj)) { - 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; - } - + if (virStorageVolLookupByPathEnsureACL(conn, def, data.voldef) == 0) { 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(cleanpath); - storageDriverUnlock(); + VIR_FREE(data.cleanpath); 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++) { - virStoragePoolObjPtr obj = driver->pools.objs[i]; - virStoragePoolDefPtr def; - - virStoragePoolObjLock(obj); + if ((obj == virStoragePoolObjListSearch(&driver->pools, + storagePoolLookupByTargetPathCallback, + path))) { def = virStoragePoolObjGetDef(obj); - - if (!virStoragePoolObjIsActive(obj)) { - virStoragePoolObjEndAPI(&obj); - continue; - } - - if (STREQ(path, def->target.path)) - pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); - + pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); virStoragePoolObjEndAPI(&obj); } storageDriverUnlock(); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 72b3c6db5d..25b6592bcd 100644 --- a/src/test/test_driver.c +++ b/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; - virStorageVolPtr ret = NULL; + struct storageVolLookupData data = { + .conn = conn, .key = key, .voldef = NULL }; + virStorageVolPtr vol = NULL; testDriverLock(privconn); - for (i = 0; i < privconn->pools.count; i++) { - obj = privconn->pools.objs[i]; - virStoragePoolObjLock(obj); + if ((obj = virStoragePoolObjListSearch(&privconn->pools, + testStorageVolLookupByKeyCallback, + &data)) && data.voldef) { def = virStoragePoolObjGetDef(obj); - if (virStoragePoolObjIsActive(obj)) { - virStorageVolDefPtr privvol = virStorageVolDefFindByKey(obj, key); - - if (privvol) { - ret = virGetStorageVol(conn, def->name, - privvol->name, privvol->key, - NULL, NULL); - virStoragePoolObjEndAPI(&obj); - break; - } - } + vol = virGetStorageVol(conn, def->name, + data.voldef->name, data.voldef->key, + NULL, NULL); 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; - virStorageVolPtr ret = NULL; + struct storageVolLookupData data = { + .conn = conn, .path = path, .voldef = NULL }; + virStorageVolPtr vol = NULL; testDriverLock(privconn); - for (i = 0; i < privconn->pools.count; i++) { - obj = privconn->pools.objs[i]; - virStoragePoolObjLock(obj); + if ((obj = virStoragePoolObjListSearch(&privconn->pools, + testStorageVolLookupByPathCallback, + &data)) && data.voldef) { def = virStoragePoolObjGetDef(obj); - if (virStoragePoolObjIsActive(obj)) { - virStorageVolDefPtr privvol = virStorageVolDefFindByPath(obj, path); - - if (privvol) { - ret = virGetStorageVol(conn, def->name, - privvol->name, privvol->key, - NULL, NULL); - virStoragePoolObjEndAPI(&obj); - break; - } - } + vol = virGetStorageVol(conn, def->name, + data.voldef->name, data.voldef->key, + NULL, NULL); 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; }