[libvirt] [PATCH 1/3] Simplify allocation check in storageVolResize

[John Ferlan]

On 05/27/2015 11:08 AM, Ján Tomko wrote:
> The volume cannot be shrinked below existing allocation, thus
> a successful resize with VOL_RESIZE_ALLOCATE will never increase
> the pool's available value.

Since shrinking a volume below existing allocation is not allowed, it is
not possible for a successful resize with VOL_RESIZE_ALLOCATE to
increase the pool's available value.

> 
> Even with the SHRINK flag it is possible to extend the current
> allocation or even the capacity. Remove the overflow when
> computing delta with this flag and do the check even if the
> flag was specified.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1073305

Editorial comment:

This bz should go back to POST...

> ---
>  src/storage/storage_driver.c | 21 +++++----------------
>  1 file changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index ac4a74a..fbb8050 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -2292,7 +2292,7 @@ storageVolResize(virStorageVolPtr obj,
>      virStorageBackendPtr backend;
>      virStoragePoolObjPtr pool = NULL;
>      virStorageVolDefPtr vol = NULL;
> -    unsigned long long abs_capacity, delta;
> +    unsigned long long abs_capacity, delta = 0;
>      int ret = -1;
>  
>      virCheckFlags(VIR_STORAGE_VOL_RESIZE_ALLOCATE |
> @@ -2341,18 +2341,14 @@ storageVolResize(virStorageVolPtr obj,
>          goto cleanup;
>      }
>  
> -    if (flags & VIR_STORAGE_VOL_RESIZE_SHRINK)
> -        delta = vol->target.allocation - abs_capacity;
> -    else
> +    if (flags & VIR_STORAGE_VOL_RESIZE_ALLOCATE)
>          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
>       */

The comment won't make sense any more as well.

ACK

I would think safe for freeze

John

> -    if (flags & VIR_STORAGE_VOL_RESIZE_ALLOCATE &&
> -        !(flags & VIR_STORAGE_VOL_RESIZE_SHRINK) &&
> -        delta > pool->def->available) {
> +    if (delta > pool->def->available) {
>          virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>                         _("Not enough space left in storage pool"));
>          goto cleanup;
> @@ -2375,15 +2371,8 @@ storageVolResize(virStorageVolPtr obj,
>       */
>      if (flags & VIR_STORAGE_VOL_RESIZE_ALLOCATE) {
>          vol->target.allocation = abs_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;
> -        }
> +        pool->def->allocation += delta;
> +        pool->def->available -= delta;
>      }
>  
>      ret = 0;
>