[pve-devel] [PATCH storage] map_volume: fallback to 'path'

[Thomas Lamprecht]

Am 4/26/19 um 3:00 PM schrieb Mira Limbeck:
> Adds a fallback to 'Plugin::path' in the default implementation of
> 'map_volume' to make the additional call to 'path' unnecessary if
> 'map_volume' is not implemented in the plugin used. 'Plugin::path' is now
> always returned if the plugin in question does not override 'map_volume'.
> 
> Signed-off-by: Mira Limbeck <m.limbeck at proxmox.com>
> ---
> This change was discussed with @Wolfgang off-list. It should simplify
> the use-case of calling 'map_volume' followed by 'path' if it is not
> defined as found for example in pve-container/src/API2/LXC.pm:
> my $path = PVE::Storage::map_volume($storage_cfg, $volid);
> $path = PVE::Storage::path($storage_cfg, $volid) if !defined($path);
> 
>  PVE/Storage/Plugin.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 7964441..6b428d7 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -966,7 +966,8 @@ sub deactivate_storage {
>  sub map_volume {
>      my ($class, $storeid, $scfg, $volname, $snapname) = @_;
>  
> -    return undef;
> +    my ($path, $vmid, $vtype) = $class->path($scfg, $volname, $storeid, $snapname);
> +    return wantarray ? ($path, $vmid, $vtype) : $path;

why wantarray and not just returning the $path, like our single actual
implementation of map_volume in RBD does? What's the rationale?

And if there'd be one it still makes sense to have the return
signatures in sync... But as it does not fits the map, which has no
interest in $vmid or $vtype, but only at which path the volume is
accessible (i.e., mapped) I doubt there is one.

also, why not just:

return $class->path($scfg, $volname, $storeid, $snapname);

then?

>  }
>  
>  sub unmap_volume {
>