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

[Mira Limbeck]

On 4/26/19 4:30 PM, Thomas Lamprecht wrote:
> 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?
To match the path call in PVE/Storage.pm, returning just the path should 
be enough, yes.
>
> 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);
This returned $vtype in my tests, not $path on ZFS. Works on lvm-thin 
and directory storages. The implementation in ZFSPoolPlugin.pm always 
returns an array containing all 3.
>
> then?
>
>>   }
>>   
>>   sub unmap_volume {
>>