From 1095230dee8e1bf184248b087695f45f867a4143 Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@redhat.com>
Date: Fri, 27 Mar 2015 09:48:59 -0400
Subject: [PATCH] storage: Fix issues in storageVolResize

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

When creating a volume in a pool, the creation allows the 'capacity'
value to be larger than the available space in the pool. As long as
the 'allocation' value will fit in the space, the volume will be created.

However, resizing the volume checks were made with the new absolute
capacity value against existing capacity + the available space without
regard for whether the new absolute capacity was actually allocating
space or not.  For example, a pool with 75G of available space creates
a volume of 10G using a capacity of 100G and allocation of 10G will succeed;
however, if the allocation used a capacity of 10G instead and then tried
to resize the allocation to 100G the code would fail to allow the backend
to try the resize.

Furthermore, when updating the pool "available" and "allocation" values,
the resize code would just "blindly" adjust them regardless of whether
space was "allocated" or just "capacity" was being adjusted.  This left
a scenario whereby a resize to 100G would fail; however, a resize to 50G
followed by one to 100G would both succeed.  Again, neither was adjusting
the allocation value, just the "capacity" value.

This patch adds more logic to the resize code to understand whether the
new capacity value is actually "allocating" space as well and whether it
shrinking or expanding. Since unsigned arithmatic is involved, the possibility
that we adjust the pool size values incorrectly is probable.

This patch also ensures that updates to the pool values only occur if we
actually performed the allocation.

NB: The storageVolDelete, storageVolCreateXML, and storageVolCreateXMLFrom
each only updates the pool allocation/availability values by the target
volume allocation value.
---
 src/storage/storage_driver.c | 37 ++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 551a0ca122..157690dbc5 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2249,7 +2249,7 @@ storageVolResize(virStorageVolPtr obj,
     virStorageBackendPtr backend;
     virStoragePoolObjPtr pool = NULL;
     virStorageVolDefPtr vol = NULL;
-    unsigned long long abs_capacity;
+    unsigned long long abs_capacity, delta;
     int ret = -1;
 
     virCheckFlags(VIR_STORAGE_VOL_RESIZE_ALLOCATE |
@@ -2294,13 +2294,24 @@ storageVolResize(virStorageVolPtr obj,
         !(flags & VIR_STORAGE_VOL_RESIZE_SHRINK)) {
         virReportError(VIR_ERR_INVALID_ARG, "%s",
                        _("Can't shrink capacity below current "
-                         "capacity with shrink flag explicitly specified"));
+                         "capacity unless shrink flag explicitly specified"));
         goto cleanup;
     }
 
-    if (abs_capacity > vol->target.capacity + pool->def->available) {
+    if (flags & VIR_STORAGE_VOL_RESIZE_SHRINK)
+        delta = vol->target.allocation - abs_capacity;
+    else
+        delta = abs_capacity - vol->target.allocation;
+
+    /* If the operation is going to increase the allocation value and not
+     * just the capacity value, then let's make sure there's enough space
+     * in the pool in order to perform that operation
+     */
+    if (flags & VIR_STORAGE_VOL_RESIZE_ALLOCATE &&
+        !(flags & VIR_STORAGE_VOL_RESIZE_SHRINK) &&
+        delta > pool->def->available) {
         virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-                       _("Not enough space left on storage pool"));
+                       _("Not enough space left in storage pool"));
         goto cleanup;
     }
 
@@ -2315,12 +2326,22 @@ storageVolResize(virStorageVolPtr obj,
         goto cleanup;
 
     vol->target.capacity = abs_capacity;
-    if (flags & VIR_STORAGE_VOL_RESIZE_ALLOCATE)
+    /* Only update the allocation and pool values if we actually did the
+     * allocation; otherwise, this is akin to a create operation with a
+     * capacity value different and potentially much larger than available
+     */
+    if (flags & VIR_STORAGE_VOL_RESIZE_ALLOCATE) {
         vol->target.allocation = abs_capacity;
 
-    /* Update pool metadata */
-    pool->def->allocation += (abs_capacity - vol->target.capacity);
-    pool->def->available -= (abs_capacity - vol->target.capacity);
+        /* Update pool metadata */
+        if (flags & VIR_STORAGE_VOL_RESIZE_SHRINK) {
+           pool->def->allocation -= delta;
+           pool->def->available += delta;
+        } else {
+           pool->def->allocation += delta;
+           pool->def->available -= delta;
+        }
+    }
 
     ret = 0;
 
-- 
GitLab