[pve-devel] [PATCH v2 storage 27/28] Add volname_for_storage and print_volname helper

[Fabian Grünbichler]

On February 24, 2020 1:44 pm, Fabian Ebner wrote:
> Allows to convert a volume ID from the source storage to
> a valid volume name for the target storage. The original name is
> preserved, except for a possible extension and it's also checked
> whether the format is valid for the target storage.
> Example: mylvm:vm-123-disk-4 <-> mydir:123/vm-123-disk-4.raw
> 
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
>  PVE/Storage.pm        | 17 +++++++++++++++--
>  PVE/Storage/Plugin.pm | 19 +++++++++++++++++++
>  2 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 8969d2e..7a76b2c 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -39,11 +39,11 @@ use PVE::Storage::DRBDPlugin;
>  use PVE::Storage::PBSPlugin;
>  
>  # Storage API version. Icrement it on changes in storage API interface.
> -use constant APIVER => 4;
> +use constant APIVER => 5;
>  # Age is the number of versions we're backward compatible with.
>  # This is like having 'current=APIVER' and age='APIAGE' in libtool,
>  # see https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html
> -use constant APIAGE => 3;
> +use constant APIAGE => 4;
>  
>  # load standard plugins
>  PVE::Storage::DirPlugin->register();
> @@ -392,6 +392,19 @@ sub parse_volname {
>      return $plugin->parse_volname($volname);
>  }
>  
> +# returns a valid volume name for the specified target storage
> +# using an existing volume ID on the source storage
> +sub volname_for_storage {
> +    my ($cfg, $volid, $target_storeid) = @_;
> +
> +    my $target_scfg = storage_config($cfg, $target_storeid);
> +    my $target_plugin = PVE::Storage::Plugin->lookup($target_scfg->{type});
> +
> +    my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $format) = parse_volname($cfg, $volid);
> +
> +    return $target_plugin->print_volname($target_scfg, $vtype, $name, $vmid, $basename, $basevmid, $isBase, $format);
> +}
> +
>  sub parse_volume_id {
>      my ($volid, $noerr) = @_;
>  
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 59660d9..6ce73bb 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -445,6 +445,25 @@ sub parse_volname {
>      die "unable to parse directory volume name '$volname'\n";
>  }

a comment here that this is only used and tested for volname_for_storage 
usage might be a good idea (e.g., it's not working for base images, and 
I am not sure whether all the shared exotic storages like iSCSI do the 
right thing?)

also, this is technically not really sure to work for external plugins 
derived from PVE::Storage::Plugin.pm - they might override 
parse_volname, and now get a print_volname with a default 
implementation. we could double check this by re-parsing before 
returning, if the assumption that parse_volname(print_volname($cfg, @param)) 
should return @param again is true ;)

> +sub print_volname {
> +    my ($class, $scfg, $vtype, $name, $vmid, $basename, $basevmid, $isBase, $format) = @_;
> +
> +    die "print_volname is not implemented for type '$vtype'\n" if $vtype ne 'images' && $vtype ne 'rootdir';
> +    die "print_volname is not implemented for base images or linked clones\n" if defined($basename) || $isBase;
> +
> +    my (undef, $valid_formats) = default_format($scfg);
> +    my $format_is_valid = grep {$_ eq $format } @$valid_formats;
> +    die "print_volname: unsupported format '$format' for storage type $scfg->{type}\n" if !$format_is_valid;
> +
> +    (my $name_without_extension = $name) =~ s/\.$format$//;
> +
> +    if ($scfg->{path}) {
> +	return "$vmid/$name_without_extension.$format";
> +    } else {
> +	return "$name_without_extension";
> +    }
> +}
> +
>  my $vtype_subdirs = {
>      images => 'images',
>      rootdir => 'private',
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>