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. Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- 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 b95506f..8d91879 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2139,7 +2139,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 | @@ -2184,13 +2184,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; } @@ -2205,12 +2216,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; -- 2.1.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list