[pve-devel] [PATCH v3 storage 1/2] volume_resize: use KiB instead of bytes

[Thomas Lamprecht]

On 2/17/20 12:41 PM, Fabian Ebner wrote:
> Avoid some problems with 'qemu-img resize', which expects
> that the size is a multiple of 512 bytes for qcow2 images.
> 
> Since vdisk_alloc already uses KiB, this also improves
> consistency a little.
> 
> The tests for ZFS are also adapted to the new interface.
> 
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
> 
> Changes from v2:
>     * align to 1024 instead of 512 bytes
>     * change the interface for volume_resize
> 
> Because of the interface change, dependency bumps are needed.

This effectively changes the storage plugin ABI, so we need to handle that:
* adapting VERSION/AGE (needs a bit of thought to be sure to get it right)
  not to cool as we do not just want to bumo the version/age for some nicer
  $size interface..
* keep existing stuff backward compatible, rather ugly as it is sure way
  to add cruft..

Thoughts?

> 
>  PVE/Storage/DRBDPlugin.pm      |  2 +-
>  PVE/Storage/LVMPlugin.pm       |  2 +-
>  PVE/Storage/Plugin.pm          |  2 +-
>  PVE/Storage/RBDPlugin.pm       |  2 +-
>  PVE/Storage/ZFSPoolPlugin.pm   |  2 +-
>  test/run_test_zfspoolplugin.pl | 26 +++++++++++++-------------
>  6 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/PVE/Storage/DRBDPlugin.pm b/PVE/Storage/DRBDPlugin.pm
> index dbae4d1..353c86d 100644
> --- a/PVE/Storage/DRBDPlugin.pm
> +++ b/PVE/Storage/DRBDPlugin.pm
> @@ -352,7 +352,7 @@ sub deactivate_volume {
>  sub volume_resize {
>      my ($class, $scfg, $storeid, $volname, $size, $running) = @_;
>  
> -    $size = ($size/1024/1024) . "M";
> +    $size = ($size/1024) . "M";
>  
>      my $path = $class->path($scfg, $volname);
>  
> diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
> index f02c110..edc6843 100644
> --- a/PVE/Storage/LVMPlugin.pm
> +++ b/PVE/Storage/LVMPlugin.pm
> @@ -524,7 +524,7 @@ sub deactivate_volume {
>  sub volume_resize {
>      my ($class, $scfg, $storeid, $volname, $size, $running) = @_;
>  
> -    $size = ($size/1024/1024) . "M";
> +    $size = ($size/1024) . "M";
>  
>      my $path = $class->path($scfg, $volname);
>      my $cmd = ['/sbin/lvextend', '-L', $size, $path];
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 0c39cbd..bb3f0a9 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -747,7 +747,7 @@ sub volume_resize {
>  
>      my $format = ($class->parse_volname($volname))[6];
>  
> -    my $cmd = ['/usr/bin/qemu-img', 'resize', '-f', $format, $path , $size];
> +    my $cmd = ['/usr/bin/qemu-img', 'resize', '-f', $format, $path, "${size}K"];
>  
>      run_command($cmd, timeout => 10);
>  
> diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
> index 0a33ec0..dfe14fd 100644
> --- a/PVE/Storage/RBDPlugin.pm
> +++ b/PVE/Storage/RBDPlugin.pm
> @@ -680,7 +680,7 @@ sub volume_resize {
>  
>      my ($vtype, $name, $vmid) = $class->parse_volname($volname);
>  
> -    my $cmd = &$rbd_cmd($scfg, $storeid, 'resize', '--allow-shrink', '--size', ($size/1024/1024), $name);
> +    my $cmd = &$rbd_cmd($scfg, $storeid, 'resize', '--allow-shrink', '--size', ($size/1024), $name);
>      run_rbd_command($cmd, errmsg => "rbd resize '$volname' error");
>      return undef;
>  }
> diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
> index d72ee16..dbe0465 100644
> --- a/PVE/Storage/ZFSPoolPlugin.pm
> +++ b/PVE/Storage/ZFSPoolPlugin.pm
> @@ -645,7 +645,7 @@ sub create_base {
>  sub volume_resize {
>      my ($class, $scfg, $storeid, $volname, $size, $running) = @_;
>  
> -    my $new_size = int($size/1024);
> +    my $new_size = $size;
>  
>      my (undef, $vname, undef, undef, undef, undef, $format) =
>          $class->parse_volname($volname);
> diff --git a/test/run_test_zfspoolplugin.pl b/test/run_test_zfspoolplugin.pl
> index 9c5e841..8a78144 100755
> --- a/test/run_test_zfspoolplugin.pl
> +++ b/test/run_test_zfspoolplugin.pl
> @@ -2415,15 +2415,15 @@ $tests->{3} = $test3;
>  my $test2 = sub {
>  
>      print "\nrun test2 \"volume_resize\"\n";
> -    my $newsize = ($volsize + 1) * 1024 * 1024 * 1024;
> +    my $newsize = ($volsize + 1) * 1024 * 1024;
>  
>      eval {
> -	if (($newsize/1024) !=
> +	if ($newsize !=
>  	    PVE::Storage::volume_resize($cfg, "$storagename:$vmdisk", $newsize, 0)) {
>  	    $count++;
>  	    warn "Test2 a failed";
>  	}
> -	if ($newsize  !=
> +	if (($newsize * 1024) !=
>  	    PVE::Storage::volume_size_info($cfg, "$storagename:$vmdisk")) {
>  	    $count++;
>  	    warn "Test2 a failed";
> @@ -2435,8 +2435,8 @@ my $test2 = sub {
>      }
>  
>      eval {
> -	warn "Test2 b failed" if ($newsize/1024) != PVE::Storage::volume_resize($cfg, "$storagename:$vmbase", $newsize, 0);
> -	warn "Test2 b failed" if $newsize  !=
> +	warn "Test2 b failed" if $newsize != PVE::Storage::volume_resize($cfg, "$storagename:$vmbase", $newsize, 0);
> +	warn "Test2 b failed" if ($newsize * 1024) !=
>  	    PVE::Storage::volume_size_info($cfg, "$storagename:$vmbase");
>      };
>      if ($@) {
> @@ -2445,11 +2445,11 @@ my $test2 = sub {
>      }
>  
>      eval {
> -	if (($newsize/1024) != PVE::Storage::volume_resize($cfg, "$storagename:$vmbase\/$vmlinked", $newsize, 0)) {
> +	if ($newsize != PVE::Storage::volume_resize($cfg, "$storagename:$vmbase\/$vmlinked", $newsize, 0)) {
>  	    $count++;
>  	    warn "Test2 c failed";
>  	}
> -	if ($newsize  !=
> +	if (($newsize * 1024) !=
>  	    PVE::Storage::volume_size_info($cfg, "$storagename:$vmbase\/$vmlinked")) {
>  	    $count++;
>  	    warn "Test2 c failed";
> @@ -2461,11 +2461,11 @@ my $test2 = sub {
>      }
>  
>      eval {
> -	if (($newsize/1024) != PVE::Storage::volume_resize($cfg, "$storagename:$ctdisk", $newsize, 0)) {
> +	if ($newsize != PVE::Storage::volume_resize($cfg, "$storagename:$ctdisk", $newsize, 0)) {
>  	    $count++;
>  	    warn "Test2 d failed";
>  	}
> -	if ($newsize  !=
> +	if (($newsize * 1024) !=
>  	    PVE::Storage::volume_size_info($cfg, "$storagename:$ctdisk")) {
>  	    $count++;
>  	    warn "Test2 d failed"
> @@ -2477,12 +2477,12 @@ my $test2 = sub {
>      }
>  
>      eval {
> -	if (($newsize/1024) !=
> +	if ($newsize !=
>  	    PVE::Storage::volume_resize($cfg, "$storagename:$ctbase", $newsize, 0)) {
>  	    $count++;
>  	    warn "Test2 e failed";
>  	}
> -	if ($newsize  !=
> +	if (($newsize * 1024) !=
>  	    PVE::Storage::volume_size_info($cfg, "$storagename:$ctbase")) {
>  	    $count++;
>  	    warn "Test2 e failed";
> @@ -2494,12 +2494,12 @@ my $test2 = sub {
>      }
>  
>      eval {
> -	if (($newsize/1024) !=
> +	if ($newsize !=
>  	    PVE::Storage::volume_resize($cfg, "$storagename:$ctbase\/$ctlinked", $newsize, 0)) {
>  	    $count++;
>  	    warn "Test2 f failed";
>  	}
> -	if ($newsize  !=
> +	if (($newsize * 1024) !=
>  	    PVE::Storage::volume_size_info($cfg, "$storagename:$ctbase\/$ctlinked")) {
>  	    $count++;
>  	    warn "Test2 f failed";
>