10:30 a.m.
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 <jferlan(a)redhat.com>
---
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 | 3 +-
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 | 5 +---
src/storage/storage_driver.c | 53 +++++++++-------------------------
src/storage/storage_util.c | 4 +--
src/test/test_driver.c | 18 ++++--------
13 files changed, 81 insertions(+), 72 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 69ed66d..cc3464e 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -305,6 +305,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 3b6e395..d27ff57 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -127,6 +127,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 e8b4eda..03777a3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1007,6 +1007,7 @@ virSecretObjSetValueSize;
# conf/virstorageobj.h
+virStoragePoolObjAddVol;
virStoragePoolObjAssignDef;
virStoragePoolObjClearVols;
virStoragePoolObjDecrAsyncjobs;
@@ -1019,6 +1020,7 @@ virStoragePoolObjGetConfigFile;
virStoragePoolObjGetDef;
virStoragePoolObjGetNames;
virStoragePoolObjGetNewDef;
+virStoragePoolObjGetVolumesCount;
virStoragePoolObjIncrAsyncjobs;
virStoragePoolObjIsActive;
virStoragePoolObjIsDuplicate;
@@ -1030,6 +1032,7 @@ virStoragePoolObjLock;
virStoragePoolObjNumOfStoragePools;
virStoragePoolObjNumOfVolumes;
virStoragePoolObjRemove;
+virStoragePoolObjRemoveVol;
virStoragePoolObjSaveDef;
virStoragePoolObjSetActive;
virStoragePoolObjSetAutostart;
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index e8f67bb..0bf5567 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;
}
@@ -595,7 +594,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 93dce40..afd93b7 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -385,8 +385,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 67f70e5..2f2782c 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -356,8 +356,7 @@ 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;
ret = 0;
diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c
index 606b399..fcaaa0e 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 7b8887b..6731677 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 b55d96a..e72dcda 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 c6dae71..c063090 100644
--- a/src/storage/storage_backend_zfs.c
+++ b/src/storage/storage_backend_zfs.c
@@ -161,10 +161,7 @@ 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;
ret = 0;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index c4650cd..fc393dd 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1579,25 +1579,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,
@@ -1629,7 +1610,7 @@ storageVolDeleteInternal(virStorageVolPtr vol,
}
}
- storageVolRemoveFromPool(obj, voldef);
+ virStoragePoolObjRemoveVol(obj, voldef);
ret = 0;
cleanup:
@@ -1768,24 +1749,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;
- volobj = virGetStorageVol(pool->conn, obj->def->name, voldef->name,
- voldef->key, NULL, NULL);
- if (!volobj) {
- obj->volumes.count--;
+ if (!(volobj = 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;
@@ -1820,7 +1796,7 @@ storageVolCreateXML(virStoragePoolPtr pool,
if (buildret < 0) {
/* buildVol handles deleting volume on failure */
- storageVolRemoveFromPool(obj, voldef);
+ virStoragePoolObjRemoveVol(obj, voldef);
voldef = NULL;
goto cleanup;
}
@@ -1971,9 +1947,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. */
@@ -1990,13 +1963,13 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool,
memcpy(shadowvol, voldef, sizeof(*voldef));
- obj->volumes.objs[obj->volumes.count++] = voldef;
- volobj = virGetStorageVol(pool->conn, obj->def->name, voldef->name,
- voldef->key, NULL, NULL);
- if (!volobj) {
- obj->volumes.count--;
+ if (!(volobj = 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 08dca94..8d9dd3f 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3595,7 +3595,7 @@ virStorageBackendRefreshLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
* An error message was raised, but we just continue. */
}
- if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) <
0)
+ if (virStoragePoolObjAddVol(pool, vol) < 0)
goto cleanup;
}
if (direrr < 0)
@@ -3783,7 +3783,7 @@ 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;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d68a18d..6ea3a5b 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1075,7 +1075,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;
@@ -4937,8 +4938,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;
@@ -5005,8 +5005,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;
@@ -5031,7 +5030,6 @@ testStorageVolDelete(virStorageVolPtr vol,
testDriverPtr privconn = vol->conn->privateData;
virStoragePoolObjPtr obj;
virStorageVolDefPtr privvol;
- size_t i;
int ret = -1;
virCheckFlags(0, -1);
@@ -5045,14 +5043,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:
--
2.9.3