[pve-devel] [PATCH v3 storage 14/22] storage_migrate: add volname_for_storage helper

[Fabian Grünbichler]

On March 12, 2020 1:08 pm, Fabian Ebner wrote:
> to guess a valid volname for a targetstorage of a different type.
> This makes it possible to migrate raw volumes between 'dir' and 'lvm'
> storages.
> 
> It is only used when the storage type for the source storage X
> and target storage Y differ and should work as long as Y uses
> the standard naming scheme (VMID/vm-VMID-name.fmt respectively vm-VMID-name).
> If it doesn't, we get an invalid name and fail, which is the old
> behavior (except if X and Y have different types but the same
> non-standard naming-scheme, where the old behavior did work).
> 
> 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 | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index cac3ba7..91b1ec8 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -562,6 +562,25 @@ sub abs_filesystem_path {
>      return $path;
>  }
>  

might benefit from a short comment here, something like:

use by storage_migrate to convert raw volume names between path and 
non-path based storages (e.g., LVM).

at least AFAIU it ;) if the above is true, it might also make sense to limit 
it to format 'raw' for now?

> +my $volname_for_storage = sub {
> +    my ($cfg, $volid, $target_storeid) = @_;
> +
> +    my (undef, $name, $vmid, undef, undef, undef, $format) = parse_volname($cfg, $volid);
> +    my $target_scfg = storage_config($cfg, $target_storeid);
> +
> +    my (undef, $valid_formats) = PVE::Storage::Plugin::default_format($target_scfg);
> +    my $format_is_valid = grep { $_ eq $format } @$valid_formats;
> +    die "unsupported format '$format' for storage type $target_scfg->{type}\n" if !$format_is_valid;
> +
> +    (my $name_without_extension = $name) =~ s/\.$format$//;
> +
> +    if ($target_scfg->{path}) {
> +       return "$vmid/$name_without_extension.$format";
> +    } else {
> +       return "$name_without_extension";
> +    }
> +};
> +
>  sub storage_migrate {
>      my ($cfg, $volid, $target_sshinfo, $target_storeid, $opts, $logfunc) = @_;
>  
> @@ -573,14 +592,21 @@ sub storage_migrate {
>      my $allow_rename = $opts->{allow_rename} ? 1 : 0;
>  
>      my ($storeid, $volname) = parse_volume_id($volid);
> -    my $target_volname = $opts->{target_volname} || $volname;
>  
>      my $scfg = storage_config($cfg, $storeid);
> +    my $tcfg = storage_config($cfg, $target_storeid);
>  
>      # no need to migrate shared content
>      return $volid if $storeid eq $target_storeid && $scfg->{shared};
>  
> -    my $tcfg = storage_config($cfg, $target_storeid);
> +    my $target_volname;
> +    if ($opts->{target_volname}) {
> +	$target_volname = $opts->{target_volname};
> +    } elsif ($scfg->{type} eq $tcfg->{type}) {

  || ($scfg->{path} && $tcfg->{path}) || (!$scfg->{path} && !$tcfg->{path}))

?

> +	$target_volname = $volname;
> +    } else {
> +	$target_volname = $volname_for_storage->($cfg, $volid, $target_storeid);
> +    }
>  
>      my $target_volid = "${target_storeid}:${target_volname}";
>  
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>