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 <jferlan@redhat.com>

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index a9fa190c3ca99a46de1c5ffdf3911f67a68fcf5e..912c27a62d90910b03af6fe3ce077cc677224ec8 100644 (file)
--- 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 401c4d5125b2efbc0d836b2a35f3d12d6b93c482..d1a1247adeceecb0343355830d0744ddd8dface7 100644 (file)
--- 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 80b3784558423799f95e9f0467253527a2fc3b54..9432d23af57de8e13cdd379c3db35810c636e1a9 100644 (file)
--- 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 bce3b4e2ab9a68b8895ca3d99533505da96c394d..aad0272d86b2f45ba9c2df10ab6db62cd706eb6a 100644 (file)
--- 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 92038c13159063f35e08c911a065e8988da360b2..90f31b00ed6397c5ddd24df238f0c6ccf7091989 100644 (file)
--- 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 67f70e551729a5aa0270f5d8a3c10fa3daf222e8..7bfe211c2d88b71cecb5435ebde0e83b6e979187 100644 (file)
--- 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 434a477e709ef7118758b16b29e60a3631a3e453..46818ef2cc541c742a78f4a0158ef629f4dad4f0 100644 (file)
--- 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 7b8887b9308200cc6dc49166d93ead9d06c4c726..67316778515b42ea10a61ba9ddf110ca39bd49ca 100644 (file)
--- 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 b55d96a7e2284a72c660bc7b2149edd2819842fd..e72dcda9c8f013d4d2c0e9788a017c471df2ab32 100644 (file)
--- 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 c6dae7183d3420635a0315c7a8a9772bb39ff70b..c2662815a17c6ba9b4584478f4f9bd7a8eb79c14 100644 (file)
--- 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 7cf5943cb68d7aeadea116cf2c4d18a724d6dd95..91dd43ff8c3a9b82f685549b7821dacc7c092c0b 100644 (file)
--- 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 b0a698a7c4edd818d9bf72d0e5eaadefcfcf4fa6..07dba222205e48a147aa4b026a94ed1e67da3456 100644 (file)
--- 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 e6660e4d687a201f2b8bf114c739960826fa7626..7cb5338c6f16072d35ec0e1446f06e3470d22b2c 100644 (file)
--- 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: