[libvirt] [PATCH 3/3] storage: Don't duplicate efforts of backend driver

John Ferlan jferlan at redhat.com
Fri Mar 27 16:07:31 UTC 2015


https://bugzilla.redhat.com/show_bug.cgi?id=1206521

If the backend driver updates the pool available and/or allocation values,
then the storage_driver VolCreateXML, VolCreateXMLFrom, and VolDelete APIs
should not change the value; otherwise, it will appear as if the values
were "doubled" for each change.  Additionally since unsigned arithmetic will
be used depending on the size and operation, either or both values could be
appear to be much larger than they should be (in the EiB range).

Currently only the disk pool updates the values, but other pools could.
Assume a "fresh" disk pool of 500 MiB using /dev/sde:

$ virsh pool-info disk-pool
...
Capacity:       509.88 MiB
Allocation:     0.00 B
Available:      509.84 MiB

$ virsh vol-create-as disk-pool sde1 --capacity 300M

$ virsh pool-info disk-pool
...
Capacity:       509.88 MiB
Allocation:     600.47 MiB
Available:      16.00 EiB

Following assumes disk backend updated to refresh the disk pool at deletion
of primary partition as well as extended partition:

$ virsh vol-delete --pool disk-pool sde1
Vol sde1 deleted

$ virsh pool-info disk-pool
...
Capacity:       509.88 MiB
Allocation:     9.73 EiB
Available:      6.27 EiB

This patch will check if the backend updated the pool values and honor that
update.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/storage/storage_driver.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 8d91879..47486e2 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1499,6 +1499,7 @@ storageVolDeleteInternal(virStorageVolPtr obj,
 {
     size_t i;
     int ret = -1;
+    unsigned long long orig_pool_available, orig_pool_allocation;
 
     if (!backend->deleteVol) {
         virReportError(VIR_ERR_NO_SUPPORT,
@@ -1507,6 +1508,9 @@ storageVolDeleteInternal(virStorageVolPtr obj,
         goto cleanup;
     }
 
+    orig_pool_available = pool->def->available;
+    orig_pool_allocation = pool->def->allocation;
+
     if (backend->deleteVol(obj->conn, pool, vol, flags) < 0)
         goto cleanup;
 
@@ -1514,8 +1518,10 @@ storageVolDeleteInternal(virStorageVolPtr obj,
      * in this module since the allocation/available weren't adjusted yet
      */
     if (updateMeta) {
-        pool->def->allocation -= vol->target.allocation;
-        pool->def->available += vol->target.allocation;
+        if (orig_pool_allocation == pool->def->allocation)
+            pool->def->allocation -= vol->target.allocation;
+        if (orig_pool_available == pool->def->available)
+            pool->def->available += vol->target.allocation;
     }
 
     for (i = 0; i < pool->volumes.count; i++) {
@@ -1634,6 +1640,7 @@ storageVolCreateXML(virStoragePoolPtr obj,
     virStorageVolDefPtr voldef = NULL;
     virStorageVolPtr ret = NULL, volobj = NULL;
     virStorageVolDefPtr buildvoldef = NULL;
+    unsigned long long orig_pool_available, orig_pool_allocation;
 
     virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);
 
@@ -1681,6 +1688,9 @@ storageVolCreateXML(virStoragePoolPtr obj,
         goto cleanup;
     }
 
+    orig_pool_available = pool->def->available;
+    orig_pool_allocation = pool->def->allocation;
+
     /* Wipe any key the user may have suggested, as volume creation
      * will generate the canonical key.  */
     VIR_FREE(voldef->key);
@@ -1738,8 +1748,10 @@ storageVolCreateXML(virStoragePoolPtr obj,
         goto cleanup;
 
     /* Update pool metadata */
-    pool->def->allocation += buildvoldef->target.allocation;
-    pool->def->available -= buildvoldef->target.allocation;
+    if (orig_pool_allocation == pool->def->allocation)
+        pool->def->allocation += buildvoldef->target.allocation;
+    if (orig_pool_available == pool->def->available)
+        pool->def->available -= buildvoldef->target.allocation;
 
     VIR_INFO("Creating volume '%s' in storage pool '%s'",
              volobj->name, pool->def->name);
@@ -1767,6 +1779,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
     virStorageVolDefPtr origvol = NULL, newvol = NULL;
     virStorageVolPtr ret = NULL, volobj = NULL;
     unsigned long long allocation;
+    unsigned long long orig_pool_available, orig_pool_allocation;
     int buildret;
 
     virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA |
@@ -1869,6 +1882,9 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
                       pool->volumes.count+1) < 0)
         goto cleanup;
 
+    orig_pool_available = pool->def->available;
+    orig_pool_allocation = pool->def->allocation;
+
     /* '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.  */
@@ -1922,8 +1938,10 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
     newvol = NULL;
 
     /* Updating pool metadata */
-    pool->def->allocation += allocation;
-    pool->def->available -= allocation;
+    if (orig_pool_allocation == pool->def->allocation)
+        pool->def->allocation += allocation;
+    if (orig_pool_available == pool->def->available)
+        pool->def->available -= allocation;
 
     VIR_INFO("Creating volume '%s' in storage pool '%s'",
              volobj->name, pool->def->name);
-- 
2.1.0




More information about the libvir-list mailing list