From 2ac0e647bdd33d93a374e7ef3eadf2a253c7bf79 Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@redhat.com>
Date: Fri, 27 Mar 2015 11:19:54 -0400
Subject: [PATCH] storage: Don't duplicate efforts of backend driver

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.
---
 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 157690dbc5..306d98e397 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1609,6 +1609,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,
@@ -1617,6 +1618,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;
 
@@ -1624,8 +1628,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++) {
@@ -1744,6 +1750,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);
 
@@ -1791,6 +1798,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);
@@ -1848,8 +1858,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);
@@ -1877,6 +1889,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 |
@@ -1979,6 +1992,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.  */
@@ -2032,8 +2048,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);
-- 
GitLab