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

2015-05-28 Thread Ján Tomko 
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

2015-05-27 Thread 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;
 

--
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

2015-05-27 Thread Ján Tomko 
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