Re: [libvirt] [PATCH 5/7] storage: Introduce virStoragePoolObjGetNames

Mostly code motion to move storageConnectList[Defined]StoragePools and similar test driver code into virstorageobj.c and rename to virStoragePoolObjGetNames. Also includes a couple of variable name adjustments to keep code consistent with other drivers. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virstorageobj.c | 37 ++++++++++++++++++++++++++++ src/conf/virstorageobj.h | 8 ++++++ src/libvirt_private.syms | 1 + src/storage/storage_driver.c | 58 +++++++++++--------------------------------- src/test/test_driver.c | 48 +++++++++--------------------------- 5 files changed, 72 insertions(+), 80 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index f0201aa..a9367a2 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -580,6 +580,43 @@ virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools, } +int +virStoragePoolObjGetNames(virStoragePoolObjListPtr pools, + virConnectPtr conn, + bool wantActive, + virStoragePoolObjListACLFilter aclfilter, + char **const names,

+ int maxnames) +{ + int nnames = 0; + size_t i; + + for (i = 0; i < pools->count && nnames < maxnames; i++) { + virStoragePoolObjPtr obj = pools->objs[i]; + virStoragePoolObjLock(obj); + if (!aclfilter || aclfilter(conn, obj->def)) { + if ((wantActive && virStoragePoolObjIsActive(obj)) || + (!wantActive && !virStoragePoolObjIsActive(obj))) {

+ if (VIR_STRDUP(names[nnames++], obj->def->name) < 0) {

+ virStoragePoolObjUnlock(obj); + goto failure; + } + } + } + virStoragePoolObjUnlock(obj); + } + + return nnames; + + failure: + while (--nnames >= 0) + VIR_FREE(names[nnames]); + memset(names, 0, maxnames * sizeof(*names));

+ + return -1; +} + + /* * virStoragePoolObjIsDuplicate: * @doms : virStoragePoolObjListPtr to search diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h index 5eeda1e..b651865 100644 --- a/src/conf/virstorageobj.h +++ b/src/conf/virstorageobj.h @@ -155,6 +155,14 @@ virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools, bool wantActive, virStoragePoolObjListACLFilter aclfilter); +int +virStoragePoolObjGetNames(virStoragePoolObjListPtr pools, + virConnectPtr conn, + bool wantActive, + virStoragePoolObjListACLFilter aclfilter, + char **const names, + int maxnames); + void virStoragePoolObjFree(virStoragePoolObjPtr pool); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7bc3bc4..233110a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -996,6 +996,7 @@ virStoragePoolObjClearVols; virStoragePoolObjDeleteDef; virStoragePoolObjFindByName; virStoragePoolObjFindByUUID; +virStoragePoolObjGetNames; virStoragePoolObjIsDuplicate; virStoragePoolObjListExport; virStoragePoolObjListFree; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 065380b..9493da8 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -490,40 +490,25 @@ storageConnectNumOfStoragePools(virConnectPtr conn) return nactive; } + static int storageConnectListStoragePools(virConnectPtr conn, char **const names, - int nnames) + int maxnames) { int got = 0; - size_t i; + + memset(names, 0, maxnames * sizeof(*names));

if (virConnectListStoragePoolsEnsureACL(conn) < 0) return -1; storageDriverLock(); - for (i = 0; i < driver->pools.count && got < nnames; i++) { - virStoragePoolObjPtr obj = driver->pools.objs[i]; - virStoragePoolObjLock(obj); - if (virConnectListStoragePoolsCheckACL(conn, obj->def) && - virStoragePoolObjIsActive(obj)) { - if (VIR_STRDUP(names[got], obj->def->name) < 0) { - virStoragePoolObjUnlock(obj); - goto cleanup; - } - got++; - } - virStoragePoolObjUnlock(obj); - } + got = virStoragePoolObjGetNames(&driver->pools, conn, true, + virConnectListStoragePoolsCheckACL, + names, maxnames); storageDriverUnlock(); return got; - - cleanup: - storageDriverUnlock(); - for (i = 0; i < got; i++) - VIR_FREE(names[i]); - memset(names, 0, nnames * sizeof(*names)); - return -1; } static int @@ -542,40 +527,25 @@ storageConnectNumOfDefinedStoragePools(virConnectPtr conn) return nactive; } + static int storageConnectListDefinedStoragePools(virConnectPtr conn, char **const names, - int nnames) + int maxnames) { int got = 0; - size_t i; + + memset(names, 0, maxnames * sizeof(*names));

if (virConnectListDefinedStoragePoolsEnsureACL(conn) < 0) return -1; storageDriverLock(); - for (i = 0; i < driver->pools.count && got < nnames; i++) { - virStoragePoolObjPtr obj = driver->pools.objs[i]; - virStoragePoolObjLock(obj); - if (virConnectListDefinedStoragePoolsCheckACL(conn, obj->def) && - !virStoragePoolObjIsActive(obj)) { - if (VIR_STRDUP(names[got], obj->def->name) < 0) { - virStoragePoolObjUnlock(obj); - goto cleanup; - } - got++; - } - virStoragePoolObjUnlock(obj); - } + got = virStoragePoolObjGetNames(&driver->pools, conn, false, + virConnectListDefinedStoragePoolsCheckACL, + names, maxnames); storageDriverUnlock(); return got; - - cleanup: - storageDriverUnlock(); - for (i = 0; i < got; i++) - VIR_FREE(names[i]); - memset(names, 0, nnames * sizeof(*names)); - return -1; } /* This method is required to be re-entrant / thread safe, so diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 4fb9915..1a6601e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4130,35 +4130,23 @@ testConnectNumOfStoragePools(virConnectPtr conn) return numActive; } + static int testConnectListStoragePools(virConnectPtr conn, char **const names, - int nnames) + int maxnames) { testDriverPtr privconn = conn->privateData; int n = 0; - size_t i; + + memset(names, 0, maxnames * sizeof(*names));

testDriverLock(privconn); - memset(names, 0, sizeof(*names)*nnames); - for (i = 0; i < privconn->pools.count && n < nnames; i++) { - virStoragePoolObjLock(privconn->pools.objs[i]); - if (virStoragePoolObjIsActive(privconn->pools.objs[i]) && - VIR_STRDUP(names[n++], privconn->pools.objs[i]->def->name) < 0) { - virStoragePoolObjUnlock(privconn->pools.objs[i]); - goto error; - } - virStoragePoolObjUnlock(privconn->pools.objs[i]); - } + n = virStoragePoolObjGetNames(&privconn->pools, conn, true, NULL, + names, maxnames); testDriverUnlock(privconn); return n; - - error: - for (n = 0; n < nnames; n++) - VIR_FREE(names[n]); - testDriverUnlock(privconn); - return -1; } @@ -4176,35 +4164,23 @@ testConnectNumOfDefinedStoragePools(virConnectPtr conn) return numInactive; } + static int testConnectListDefinedStoragePools(virConnectPtr conn, char **const names, - int nnames) + int maxnames) { testDriverPtr privconn = conn->privateData; int n = 0; - size_t i; + + memset(names, 0, maxnames * sizeof(*names));

testDriverLock(privconn); - memset(names, 0, sizeof(*names)*nnames); - for (i = 0; i < privconn->pools.count && n < nnames; i++) { - virStoragePoolObjLock(privconn->pools.objs[i]); - if (!virStoragePoolObjIsActive(privconn->pools.objs[i]) && - VIR_STRDUP(names[n++], privconn->pools.objs[i]->def->name) < 0) { - virStoragePoolObjUnlock(privconn->pools.objs[i]); - goto error; - } - virStoragePoolObjUnlock(privconn->pools.objs[i]); - } + n = virStoragePoolObjGetNames(&privconn->pools, conn, false, NULL, + names, maxnames); testDriverUnlock(privconn); return n; - - error: - for (n = 0; n < nnames; n++) - VIR_FREE(names[n]); - testDriverUnlock(privconn); - return -1; } static int