Re: [pve-devel] [PATCH v3 storage 1/9] storage: add new find_free_volname

[Fabian Ebner <f.ebner@proxmox.com>]

Am 21.10.21 um 13:30 schrieb Aaron Lauterer:
> This new method exposes the functionality to request a new, not yet
> used, volname for a storage.
> 
> The default implementation will return the result from
> 'find_free_diskname' prefixed with "<VMID>/" if $scfg->{path} exists.
> Otherwise it will only return the result from 'find_free_diskname'.
> 
> If the format suffix is added also depends on the existence of
> $scfg->{path}.
> 
> $scfg->{path} will be present for directory based storage types.
> 
> Should a storage need to return different volname, it needs to override
> the 'find_free_volname' method.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> v2 -> v3:
> * dropped exists() checks
> 
> v1 -> v2:
> * drop exposed 'find_free_diskname' in favor of 'find_free_volname'
> * dropped 'wants_fmt_suffix' as 'find_free_volname' now decides that itself
> 
> rfc -> v1:
> dropped $add_fmt_suffix parameter and added the "wants_fmt_suffix"
> helper method in each plugin.
> 
>   PVE/Storage.pm        |  9 +++++++++
>   PVE/Storage/Plugin.pm | 11 +++++++++++
>   2 files changed, 20 insertions(+)
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 71d6ad7..4fbeaea 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -203,6 +203,15 @@ sub storage_can_replicate {
>       return $plugin->storage_can_replicate($scfg, $storeid, $format);
>   }
>   
> +sub find_free_volname {
> +    my ($cfg, $storeid, $vmid, $fmt) = @_;
> +
> +    my $scfg = storage_config($cfg, $storeid);
> +    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
> +
> +    return $plugin->find_free_volname($storeid, $scfg, $vmid, $fmt);
> +}
> +
>   sub storage_ids {
>       my ($cfg) = @_;
>   
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index aeb4fff..26465fd 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -732,6 +732,17 @@ sub find_free_diskname {
>       return get_next_vm_diskname($disk_list, $storeid, $vmid, $fmt, $scfg, $add_fmt_suffix);
>   }
>   
> +sub find_free_volname {
> +    my ($class, $storeid, $scfg, $vmid, $fmt) = @_;
> +
> +    my $diskname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt, $scfg->{path});
> +
> +    if ($scfg->{path}) {
> +	return "${vmid}/$diskname";
> +    }

Style nit: could be shorter with post-if or use ternary '?'.

> +    return $diskname;
> +}
> +
>   sub clone_image {
>       my ($class, $scfg, $storeid, $volname, $vmid, $snap) = @_;
>   
>