[pve-devel] [PATCH container] Fix #2109: resize rbd volume for container failed

[Thomas Lamprecht]

On 3/5/19 11:25 AM, Alwin Antreich wrote:
> The returned path of the volume was not a mapped device. This patch uses
> un-/map_volume for rbd storage to get a device mapped and its path.
> 
> Signed-off-by: Alwin Antreich <a.antreich at proxmox.com>
> ---
>  src/PVE/API2/LXC.pm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 27d26d5..f06046f 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -1582,6 +1582,9 @@ __PACKAGE__->register_method({
>  
>  	    PVE::Storage::activate_volumes($storage_cfg, [$volid]);
>  
> +	    my $store_type = $storage_cfg->{ids}->{$storeid}->{type};
> +	    my $path = PVE::Storage::map_volume($storage_cfg, $volid, undef) if ($store_type eq 'rbd');
> +

feels like a PITA. I remember some discussion with Wolfgang B that it really
would make sense to return the path as default implementation for map_volume
too, then one wouldn't need to differ here. (Or just map it in path?)

Also, if we decide to not do that it's still not correct to check for 'rbd'
here, just adds coupling between container and specific storage types, we want
to go away from such specific hard coded lists, not add more of them...

Passing undef explicitly as last parameter to map_volume is also unnecessary.

With the current implementation of map_volume one could just do something
like:

my $path = PVE::Storage::map_volume($storage_cfg, $volid) // PVE::Storage::path($storage_cfg, $volid);

that's at least a bit less coupling, and a more future proof way...


>  	    my $size = PVE::Storage::volume_size_info($storage_cfg, $volid, 5);
>  	    $newsize += $size if $ext;
>  	    $newsize = int($newsize);
> @@ -1602,7 +1605,7 @@ __PACKAGE__->register_method({
>  		PVE::LXC::Config->write_config($vmid, $conf);
>  
>  		if ($format eq 'raw') {
> -		    my $path = PVE::Storage::path($storage_cfg, $volid, undef);
> +		    $path = PVE::Storage::path($storage_cfg, $volid, undef) if ($store_type ne 'rbd');
>  		    if ($running) {
>  
>  			$mp->{mp} = '/';
> @@ -1630,6 +1633,8 @@ __PACKAGE__->register_method({
>  			    PVE::Tools::run_command(['resize2fs', $path]);
>  			};
>  			warn "Failed to update the container's filesystem: $@\n" if $@;
> +
> +			PVE::Storage::unmap_volume($storage_cfg, $volid, undef) if ($store_type eq 'rbd');

that's a NOP for storage types not needing it, so unconditional makes sense here too.

>  		    }
>  		}
>  	    };
>