[pve-devel] [PATCH v2 storage 3/5] add disk reassign feature

[Fabian Grünbichler]

some small nits inline

On September 1, 2020 2:44 pm, Aaron Lauterer wrote:
> Functionality has been added for the following storage types:
> 
> * dir based ones
>     * directory
>     * NFS
>     * CIFS
> * ZFS
> * (thin) LVM
> * Ceph
> 
> Signed-off-by: Aaron Lauterer <a.lauterer at proxmox.com>
> ---
> rfc -> v1 -> v2: nothing changed
> 
>  PVE/Storage.pm               | 10 ++++++++++
>  PVE/Storage/LVMPlugin.pm     | 15 +++++++++++++++
>  PVE/Storage/Plugin.pm        | 21 +++++++++++++++++++++
>  PVE/Storage/RBDPlugin.pm     | 13 +++++++++++++
>  PVE/Storage/ZFSPoolPlugin.pm |  9 +++++++++
>  5 files changed, 68 insertions(+)
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 4a60615..919c606 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -1759,6 +1759,16 @@ sub complete_volume {
>      return $res;
>  }
>  
> +sub reassign_volume {
> +    my ($cfg, $volid, $target_vmid) = @_;
> +
> +    my ($storeid, $volname) = parse_volume_id($volid);
> +    my $scfg = storage_config($cfg, $storeid);
> +    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
> +
> +    return $plugin->reassign_volume($scfg, $storeid, $volname, $target_vmid);
> +}
> +
>  # Various io-heavy operations require io/bandwidth limits which can be
>  # configured on multiple levels: The global defaults in datacenter.cfg, and
>  # per-storage overrides. When we want to do a restore from storage A to storage
> diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
> index c0740d4..28ff6c8 100644
> --- a/PVE/Storage/LVMPlugin.pm
> +++ b/PVE/Storage/LVMPlugin.pm
> @@ -337,6 +337,13 @@ sub lvcreate {
>      run_command($cmd, errmsg => "lvcreate '$vg/$name' error");
>  }
>  
> +sub lvrename {
> +    my ($vg, $oldname, $newname) = @_;
> +
> +    my $cmd = ['/sbin/lvrename', $vg, $oldname, $newname];
> +    run_command($cmd, errmsg => "lvrename '${vg}/${oldname}' to '${newname}' error");
> +}
> +
>  sub alloc_image {
>      my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
>  
> @@ -681,4 +688,12 @@ sub volume_import_write {
>  	input => '<&'.fileno($input_fh));
>  }
>  
> +sub reassign_volume {
> +    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
> +
> +    my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
> +    lvrename($scfg->{vgname}, $volname, $target_volname);
> +    return "${storeid}:${target_volname}";
> +}
> +
>  1;
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 8a58ff4..770a482 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -1411,4 +1411,25 @@ sub volume_import_formats {
>      return ();
>  }
>  
> +sub reassign_volume {
> +    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
> +
> +    my $basedir = $class->get_subdir($scfg, 'images');
> +    my $imagedir = "${basedir}/${target_vmid}";
> +
> +    mkpath $imagedir;
> +
> +    my @parsed_volname = $class->parse_volname($volname);
> +    my $format = $parsed_volname[6];
> +    my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format, 1);
> +
$volname vs $target_volname, but one includes the vmid the other does 
not? rename 'target_volname' to diskname (like the sub you call), then 
you can do 

my $target_volname = "$target_vmid/$diskname";

> +    my $old_path = "${basedir}/${volname}";
> +    my $new_path = "${imagedir}/${target_volname}";

and construct both paths in the same fashion

> +
> +    rename($old_path, $new_path) ||
> +	die "rename '$old_path' to '$new_path' failed - $!\n";
> +
> +    return "${storeid}:${target_vmid}/${target_volname}";

and return $storeid:$target_volname here

> +}
> +
>  1;
> diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
> index 38f2b46..c0bd74c 100644
> --- a/PVE/Storage/RBDPlugin.pm
> +++ b/PVE/Storage/RBDPlugin.pm
> @@ -719,4 +719,17 @@ sub volume_has_feature {
>      return undef;
>  }
>  
> +sub reassign_volume {
> +    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
> +
> +    my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
> +    my $cmd = $rbd_cmd->($scfg, $storeid, 'rename', $volname, $target_volname);
> +
> +    run_rbd_command(
> +	$cmd,
> +	errmsg => "could not rename image '$volname' to '$target_volname'",
> +    );
> +    return "${storeid}:${target_volname}";
> +}
> +
>  1;
> diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
> index 10354b3..5baa5c2 100644
> --- a/PVE/Storage/ZFSPoolPlugin.pm
> +++ b/PVE/Storage/ZFSPoolPlugin.pm
> @@ -749,4 +749,13 @@ sub volume_import_formats {
>      return $class->volume_export_formats($scfg, $storeid, $volname, undef, $base_snapshot, $with_snapshots);
>  }
>  
> +sub reassign_volume {
> +    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
> +
> +    my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
> +    $class->zfs_request($scfg, 5, 'rename', "$scfg->{pool}/$volname", "$scfg->{pool}/$target_volname");
> +
> +    return "${storeid}:${target_volname}";
> +}
> +
>  1;
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>