Re: [pve-devel] [RFC storage 1/7] storage: expose find_free_diskname

[Aaron Lauterer <a.lauterer@proxmox.com>]

On 6/2/21 10:29 AM, Fabian Ebner wrote:
> Am 01.06.21 um 18:10 schrieb Aaron Lauterer:
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>>   PVE/Storage.pm | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
>> index aa36bad..93d09ce 100755
>> --- a/PVE/Storage.pm
>> +++ b/PVE/Storage.pm
>> @@ -201,6 +201,14 @@ sub storage_can_replicate {
>>       return $plugin->storage_can_replicate($scfg, $storeid, $format);
>>   }
>> +sub find_free_diskname {
>> +    my ($cfg, $storeid, $vmid, $fmt, $add_fmt_suffix) = @_;
> 
> Nit: Ideally, the $add_fmt_suffix should be decided by the plugin, as an outside caller cannot know what a plugin wants/expects. Don't know if that's easy to do the way things are though.

Yeah, I am not sure how to handle that... most of our storage plugins don't use it at all (ZFS, RBD, LVM). That leaves the Plugin.pm (storage based) and potential 3rd party plugins that have their own implementation.
Depending on where you would use the now public `find_free_diskname`, it could be possible to know the format and if the suffix should already be added. For the move-disk to other guest (disk reassign) stuff I cannot do that and leave it undefined.

In the Plugin.pm implementation of `rename_volume` I added a check if a suffix is present, and if not, take the one from the source volume.
Thinking about it, I might even strip any potential suffixes and always use the one from the source as that should be kept since the file format probably should not change with a rename.

> 
>> +
>> +    my $scfg = storage_config($cfg, $storeid);
>> +    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>> +    return $plugin->find_free_diskname($storeid, $scfg, $vmid, $fmt, $add_fmt_suffix);
>> +}
>> +
>>   sub storage_ids {
>>       my ($cfg) = @_;
>>