Re: [libvirt] [PATCH 1/3] Simplify allocation check in storageVolResize
On Wed, May 27, 2015 at 03:03:04PM -0400, John Ferlan wrote: > > > 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. > Thanks, that's much clearer. > > > > 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 removed the commend and pushed the first two patches. The third one does not fix a bug and can wait for the next release. Jan > I would think safe for freeze > > John signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] Simplify allocation check in storageVolResize
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;
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] Simplify allocation check in storageVolResize
The volume cannot be shrinked below existing allocation, thus
a successful resize with VOL_RESIZE_ALLOCATE will never 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
---
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
*/
-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;
--
2.3.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
