From 40630a8e45db6858bfc2cd1ecda6a61d7c02ca1c Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Tue, 9 May 2017 08:05:16 -0400 Subject: [PATCH] storage: Introduce storage volume add, delete, count APIs Create/use virStoragePoolObjAddVol in order to add volumes onto list. Create/use virStoragePoolObjRemoveVol in order to remove volumes from list. Create/use virStoragePoolObjGetVolumesCount to get count of volumes on list. For the storage driver, the logic alters when the volumes.obj list grows to after we've fetched the volobj. This is an optimization of sorts, but also doesn't "needlessly" grow the volumes.objs list and then just decr the count if the virGetStorageVol fails. Signed-off-by: John Ferlan --- src/conf/virstorageobj.c | 37 ++++++++++++++++++ src/conf/virstorageobj.h | 11 ++++++ src/libvirt_private.syms | 3 ++ src/storage/storage_backend_disk.c | 5 +-- src/storage/storage_backend_gluster.c | 3 +- src/storage/storage_backend_logical.c | 4 +- src/storage/storage_backend_mpath.c | 3 +- src/storage/storage_backend_rbd.c | 4 +- src/storage/storage_backend_sheepdog.c | 4 +- src/storage/storage_backend_zfs.c | 6 +-- src/storage/storage_driver.c | 53 +++++++------------------- src/storage/storage_util.c | 8 ++-- src/test/test_driver.c | 18 +++------ 13 files changed, 85 insertions(+), 74 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index a9fa190c3c..912c27a62d 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -282,6 +282,43 @@ virStoragePoolObjClearVols(virStoragePoolObjPtr obj) } +int +virStoragePoolObjAddVol(virStoragePoolObjPtr obj, + virStorageVolDefPtr voldef) +{ + if (VIR_APPEND_ELEMENT(obj->volumes.objs, obj->volumes.count, voldef) < 0) + return -1; + return 0; +} + + +void +virStoragePoolObjRemoveVol(virStoragePoolObjPtr obj, + virStorageVolDefPtr voldef) +{ + virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj); + size_t i; + + for (i = 0; i < obj->volumes.count; i++) { + if (obj->volumes.objs[i] == voldef) { + VIR_INFO("Deleting volume '%s' from storage pool '%s'", + voldef->name, def->name); + virStorageVolDefFree(voldef); + + VIR_DELETE_ELEMENT(obj->volumes.objs, i, obj->volumes.count); + return; + } + } +} + + +size_t +virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj) +{ + return obj->volumes.count; +} + + virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr obj, const char *key) diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h index 401c4d5125..d1a1247ade 100644 --- a/src/conf/virstorageobj.h +++ b/src/conf/virstorageobj.h @@ -136,6 +136,17 @@ virStoragePoolObjPtr virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, const char *name); +int +virStoragePoolObjAddVol(virStoragePoolObjPtr obj, + virStorageVolDefPtr voldef); + +void +virStoragePoolObjRemoveVol(virStoragePoolObjPtr obj, + virStorageVolDefPtr voldef); + +size_t +virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj); + virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr obj, const char *key); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 80b3784558..9432d23af5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1053,6 +1053,7 @@ virSecretObjSetValueSize; # conf/virstorageobj.h +virStoragePoolObjAddVol; virStoragePoolObjAssignDef; virStoragePoolObjClearVols; virStoragePoolObjDecrAsyncjobs; @@ -1066,6 +1067,7 @@ virStoragePoolObjGetConfigFile; virStoragePoolObjGetDef; virStoragePoolObjGetNames; virStoragePoolObjGetNewDef; +virStoragePoolObjGetVolumesCount; virStoragePoolObjIncrAsyncjobs; virStoragePoolObjIsActive; virStoragePoolObjIsAutostart; @@ -1079,6 +1081,7 @@ virStoragePoolObjNew; virStoragePoolObjNumOfStoragePools; virStoragePoolObjNumOfVolumes; virStoragePoolObjRemove; +virStoragePoolObjRemoveVol; virStoragePoolObjSaveDef; virStoragePoolObjSetActive; virStoragePoolObjSetAutostart; diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index bce3b4e2ab..aad0272d86 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -65,8 +65,7 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, if (VIR_ALLOC(vol) < 0) return -1; if (VIR_STRDUP(vol->name, partname) < 0 || - VIR_APPEND_ELEMENT_COPY(pool->volumes.objs, - pool->volumes.count, vol) < 0) { + virStoragePoolObjAddVol(pool, vol) < 0) { virStorageVolDefFree(vol); return -1; } @@ -597,7 +596,7 @@ virStorageBackendDiskPartFormat(virStoragePoolObjPtr pool, break; } } - if (i == pool->volumes.count) { + if (i == virStoragePoolObjGetVolumesCount(pool)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no extended partition found and no primary partition available")); return -1; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 92038c1315..90f31b00ed 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -386,8 +386,7 @@ virStorageBackendGlusterRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, if (okay < 0) goto cleanup; - if (vol && VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, - vol) < 0) + if (vol && virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; } if (errno) { diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 67f70e5517..7bfe211c2d 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -356,9 +356,9 @@ virStorageBackendLogicalMakeVol(char **const groups, if (virStorageBackendLogicalParseVolExtents(vol, groups) < 0) goto cleanup; - if (is_new_vol && - VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) + if (is_new_vol && virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; + vol = NULL; ret = 0; diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 434a477e70..46818ef2cc 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -71,8 +71,9 @@ virStorageBackendMpathNewVol(virStoragePoolObjPtr pool, if (VIR_STRDUP(vol->key, vol->target.path) < 0) goto cleanup; - if (VIR_APPEND_ELEMENT_COPY(pool->volumes.objs, pool->volumes.count, vol) < 0) + if (virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; + pool->def->capacity += vol->target.capacity; pool->def->allocation += vol->target.allocation; ret = 0; diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 7b8887b930..6731677851 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -506,7 +506,7 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn, goto cleanup; } - if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) { + if (virStoragePoolObjAddVol(pool, vol) < 0) { virStorageVolDefFree(vol); virStoragePoolObjClearVols(pool); goto cleanup; @@ -514,7 +514,7 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn, } VIR_DEBUG("Found %zu images in RBD pool %s", - pool->volumes.count, pool->def->source.name); + virStoragePoolObjGetVolumesCount(pool), pool->def->source.name); ret = 0; diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index b55d96a7e2..e72dcda9c8 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -130,11 +130,9 @@ virStorageBackendSheepdogAddVolume(virConnectPtr conn ATTRIBUTE_UNUSED, if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) goto error; - if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0) + if (virStoragePoolObjAddVol(pool, vol) < 0) goto error; - pool->volumes.objs[pool->volumes.count - 1] = vol; - return 0; error: diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c index c6dae7183d..c2662815a1 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -161,11 +161,9 @@ virStorageBackendZFSParseVol(virStoragePoolObjPtr pool, if (volume->target.allocation < volume->target.capacity) volume->target.sparse = true; - if (is_new_vol && - VIR_APPEND_ELEMENT(pool->volumes.objs, - pool->volumes.count, - volume) < 0) + if (is_new_vol && virStoragePoolObjAddVol(pool, volume) < 0) goto cleanup; + volume = NULL; ret = 0; cleanup: diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 7cf5943cb6..91dd43ff8c 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1626,25 +1626,6 @@ storagePoolLookupByTargetPath(virConnectPtr conn, } -static void -storageVolRemoveFromPool(virStoragePoolObjPtr obj, - virStorageVolDefPtr voldef) -{ - size_t i; - - for (i = 0; i < obj->volumes.count; i++) { - if (obj->volumes.objs[i] == voldef) { - VIR_INFO("Deleting volume '%s' from storage pool '%s'", - voldef->name, obj->def->name); - virStorageVolDefFree(voldef); - - VIR_DELETE_ELEMENT(obj->volumes.objs, i, obj->volumes.count); - break; - } - } -} - - static int storageVolDeleteInternal(virStorageVolPtr vol, virStorageBackendPtr backend, @@ -1676,7 +1657,7 @@ storageVolDeleteInternal(virStorageVolPtr vol, } } - storageVolRemoveFromPool(obj, voldef); + virStoragePoolObjRemoveVol(obj, voldef); ret = 0; cleanup: @@ -1815,24 +1796,19 @@ storageVolCreateXML(virStoragePoolPtr pool, goto cleanup; } - if (VIR_REALLOC_N(obj->volumes.objs, - obj->volumes.count + 1) < 0) - goto cleanup; - /* Wipe any key the user may have suggested, as volume creation * will generate the canonical key. */ VIR_FREE(voldef->key); if (backend->createVol(pool->conn, obj, voldef) < 0) goto cleanup; - obj->volumes.objs[obj->volumes.count++] = voldef; - newvol = virGetStorageVol(pool->conn, obj->def->name, voldef->name, - voldef->key, NULL, NULL); - if (!newvol) { - obj->volumes.count--; + if (!(newvol = virGetStorageVol(pool->conn, obj->def->name, voldef->name, + voldef->key, NULL, NULL))) goto cleanup; - } + /* NB: Upon success voldef "owned" by storage pool for deletion purposes */ + if (virStoragePoolObjAddVol(obj, voldef) < 0) + goto cleanup; if (backend->buildVol) { int buildret; @@ -1867,7 +1843,7 @@ storageVolCreateXML(virStoragePoolPtr pool, if (buildret < 0) { /* buildVol handles deleting volume on failure */ - storageVolRemoveFromPool(obj, voldef); + virStoragePoolObjRemoveVol(obj, voldef); voldef = NULL; goto cleanup; } @@ -2018,9 +1994,6 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, backend->refreshVol(pool->conn, obj, voldefsrc) < 0) goto cleanup; - if (VIR_REALLOC_N(obj->volumes.objs, obj->volumes.count + 1) < 0) - goto cleanup; - /* 'Define' the new volume so we get async progress reporting. * Wipe any key the user may have suggested, as volume creation * will generate the canonical key. */ @@ -2037,13 +2010,13 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, memcpy(shadowvol, voldef, sizeof(*voldef)); - obj->volumes.objs[obj->volumes.count++] = voldef; - newvol = virGetStorageVol(pool->conn, obj->def->name, voldef->name, - voldef->key, NULL, NULL); - if (!newvol) { - obj->volumes.count--; + if (!(newvol = virGetStorageVol(pool->conn, obj->def->name, voldef->name, + voldef->key, NULL, NULL))) + goto cleanup; + + /* NB: Upon success voldef "owned" by storage pool for deletion purposes */ + if (virStoragePoolObjAddVol(obj, voldef) < 0) goto cleanup; - } /* Drop the pool lock during volume allocation */ obj->asyncjobs++; diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index b0a698a7c4..07dba22220 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3623,13 +3623,13 @@ virStorageBackendRefreshLocal(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } - if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) + if (virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; + vol = NULL; } if (direrr < 0) goto cleanup; VIR_DIR_CLOSE(dir); - vol = NULL; if (VIR_ALLOC(target)) goto cleanup; @@ -3811,10 +3811,10 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, pool->def->capacity += vol->target.capacity; pool->def->allocation += vol->target.allocation; - if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) + if (virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; - vol = NULL; + retval = 0; cleanup: diff --git a/src/test/test_driver.c b/src/test/test_driver.c index e6660e4d68..7cb5338c6f 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1079,7 +1079,8 @@ testOpenVolumesForPool(const char *file, if (!def->key && VIR_STRDUP(def->key, def->target.path) < 0) goto error; - if (VIR_APPEND_ELEMENT_COPY(obj->volumes.objs, obj->volumes.count, def) < 0) + + if (virStoragePoolObjAddVol(obj, def) < 0) goto error; obj->def->allocation += def->target.allocation; @@ -5011,8 +5012,7 @@ testStorageVolCreateXML(virStoragePoolPtr pool, goto cleanup; if (VIR_STRDUP(privvol->key, privvol->target.path) < 0 || - VIR_APPEND_ELEMENT_COPY(obj->volumes.objs, - obj->volumes.count, privvol) < 0) + virStoragePoolObjAddVol(obj, privvol) < 0) goto cleanup; obj->def->allocation += privvol->target.allocation; @@ -5079,8 +5079,7 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, goto cleanup; if (VIR_STRDUP(privvol->key, privvol->target.path) < 0 || - VIR_APPEND_ELEMENT_COPY(obj->volumes.objs, - obj->volumes.count, privvol) < 0) + virStoragePoolObjAddVol(obj, privvol) < 0) goto cleanup; obj->def->allocation += privvol->target.allocation; @@ -5105,7 +5104,6 @@ testStorageVolDelete(virStorageVolPtr vol, testDriverPtr privconn = vol->conn->privateData; virStoragePoolObjPtr obj; virStorageVolDefPtr privvol; - size_t i; int ret = -1; virCheckFlags(0, -1); @@ -5119,14 +5117,8 @@ testStorageVolDelete(virStorageVolPtr vol, obj->def->allocation -= privvol->target.allocation; obj->def->available = (obj->def->capacity - obj->def->allocation); - for (i = 0; i < obj->volumes.count; i++) { - if (obj->volumes.objs[i] == privvol) { - virStorageVolDefFree(privvol); + virStoragePoolObjRemoveVol(obj, privvol); - VIR_DELETE_ELEMENT(obj->volumes.objs, i, obj->volumes.count); - break; - } - } ret = 0; cleanup: