[pve-devel] [PATCH v7 storage 1/5] add disk reassign feature

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

Functionality has been added for the following storage types:

* dir based ones
* ZFS
* (thin) LVM
* Ceph

A new feature `reassign` has been introduced to mark which storage
plugin supports the feature.

Version API and AGE have been bumped.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
v6 -> v7:
We now place everything for dis based plugins in the Plugin.pm and check
against the supported API version to avoid running the code on external
plugins that do not yet officially support the reassign feature.

* activate storage before doing anything else
    * checks if storage is enabled as well
* code cleanup
    * change long function calls to multiline
    * base parameter is not passed to rename function anymore but
      handled in the reassign function
    * prefixed vars with source_ / target_ to make them easier to
      distinguish

v5 -> v6:
* refactor dir based feature check to reduce code repetition by
  introducing the file_can_reassign_volume sub that does the actual check

v4 -> v5:
* rebased on master
* bumped api ver and api age
* rephrased "not implemented" message as suggested [0].

v3 -> v4:
* revert intermediate storage plugin for directory based plugins
* add a `die "not supported"` method in Plugin.pm
* dir based plugins now call the file_reassign_volume method in
  Plugin.pm as the generic file/directory based method
* restored old `volume_has_feature` method in Plugin.pm and override it
  in directory based plugins to check against the new `reassign` feature
  (not too happy about the repetition for each plugin)

v2 -> v3:
* added feature flags instead of dummy "not implemented" methods in
  plugins which do not support it as that would break compatibility with
  3rd party plugins.
  Had to make $features available outside the `has_features` method in
  Plugins.pm in order to be able to individually add features in the
  `BaseDirPlugin.pm`.
* added the BaseDirPlugin.pm to maintain compat with 3rd party plugins,
  this is explained in the commit message
* moved the actual renaming from the `reassign_volume` to a dedicated
  `rename_volume` method to make this functionality available to other
  possible uses in the future.
* added support for linked clones ($base)


rfc -> v1 -> v2: nothing changed

[0] https://lists.proxmox.com/pipermail/pve-devel/2020-November/046031.html




 PVE/Storage.pm               | 19 +++++++++++--
 PVE/Storage/LVMPlugin.pm     | 34 +++++++++++++++++++++++
 PVE/Storage/LvmThinPlugin.pm |  1 +
 PVE/Storage/Plugin.pm        | 52 ++++++++++++++++++++++++++++++++++++
 PVE/Storage/RBDPlugin.pm     | 37 +++++++++++++++++++++++++
 PVE/Storage/ZFSPoolPlugin.pm | 38 ++++++++++++++++++++++++++
 6 files changed, 179 insertions(+), 2 deletions(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 122c3e9..ea782cc 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -41,11 +41,11 @@ use PVE::Storage::DRBDPlugin;
 use PVE::Storage::PBSPlugin;
 
 # Storage API version. Increment it on changes in storage API interface.
-use constant APIVER => 8;
+use constant APIVER => 9;
 # 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 => 7;
+use constant APIAGE => 8;
 
 # load standard plugins
 PVE::Storage::DirPlugin->register();
@@ -349,6 +349,7 @@ sub volume_snapshot_needs_fsfreeze {
 #            snapshot - taking a snapshot is possible
 #            sparseinit - volume is sparsely initialized
 #            template - conversion to base image is possible
+#            reassign - reassigning disks to other guest is possible
 # $snap - check if the feature is supported for a given snapshot
 # $running - if the guest owning the volume is running
 # $opts - hash with further options:
@@ -1843,6 +1844,20 @@ sub complete_volume {
     return $res;
 }
 
+sub reassign_volume {
+    my ($cfg, $volid, $target_vmid) = @_;
+
+    my ($storeid, $volname) = parse_volume_id($volid);
+
+    activate_storage($cfg, $storeid);
+
+    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 df49b76..ff169f6 100644
--- a/PVE/Storage/LVMPlugin.pm
+++ b/PVE/Storage/LVMPlugin.pm
@@ -339,6 +339,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) = @_;
 
@@ -584,6 +591,7 @@ sub volume_has_feature {
 
     my $features = {
 	copy => { base => 1, current => 1},
+	reassign => {current => 1},
     };
 
     my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) =
@@ -684,4 +692,30 @@ sub volume_import_write {
 	input => '<&'.fileno($input_fh));
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $source_volname, $target_vmid) = @_;
+
+    $class->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub {
+	my $target_volname = $class->find_free_diskname(
+	    $storeid,
+	    $scfg,
+	    $target_vmid,
+	);
+	$class->rename_volume(
+	    $scfg,
+	    $storeid,
+	    $source_volname,
+	    $target_volname,
+	    $target_vmid,
+	);
+	return "${storeid}:${target_volname}";
+    });
+}
+
+sub rename_volume {
+    my ($class, $scfg, $storeid, $source_volname, $target_volname, $target_vmid) = @_;
+
+    lvrename($scfg->{vgname}, $source_volname, $target_volname);
+}
+
 1;
diff --git a/PVE/Storage/LvmThinPlugin.pm b/PVE/Storage/LvmThinPlugin.pm
index c9e127c..895af8b 100644
--- a/PVE/Storage/LvmThinPlugin.pm
+++ b/PVE/Storage/LvmThinPlugin.pm
@@ -355,6 +355,7 @@ sub volume_has_feature {
 	template => { current => 1},
 	copy => { base => 1, current => 1, snap => 1},
 	sparseinit => { base => 1, current => 1},
+	reassign => {current => 1},
     };
 
     my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) =
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index d7136a1..f14279e 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -939,6 +939,7 @@ sub volume_has_feature {
 		  snap => {qcow2 => 1} },
 	sparseinit => { base => {qcow2 => 1, raw => 1, vmdk => 1},
 			current => {qcow2 => 1, raw => 1, vmdk => 1} },
+	reassign => { current =>{qcow2 => 1, raw => 1, vmdk => 1} },
     };
 
     # clone_image creates a qcow2 volume
@@ -946,6 +947,10 @@ sub volume_has_feature {
 		defined($opts->{valid_target_formats}) &&
 		!(grep { $_ eq 'qcow2' } @{$opts->{valid_target_formats}});
 
+    return 0 if $feature eq 'reassign'
+	&& $class->can('api')
+	&& $class->api() < 9;
+
     my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $format) =
 	$class->parse_volname($volname);
 
@@ -1463,4 +1468,51 @@ sub volume_import_formats {
     return ();
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $source_volname, $target_vmid) = @_;
+    die "not implemented in storage plugin '$class'\n" if $class->can('api') && $class->api() < 9;
+
+    my $base;
+    my $format;
+    my $source_vmid;
+
+    (undef, undef, $source_vmid, $base, undef, undef, $format) = $class->parse_volname($source_volname);
+
+    $base = $base ? "${base}/" : '';
+
+    $class->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub {
+	my $target_volname = $class->find_free_diskname(
+	    $storeid,
+	    $scfg,
+	    $target_vmid,
+	    $format,
+	    1,
+	);
+	$class->rename_volume(
+	    $scfg,
+	    $storeid,
+	    $source_volname,
+	    $target_volname,
+	    $target_vmid,
+	    $base,
+	);
+	return "${storeid}:${base}${target_vmid}/${target_volname}";
+    });
+}
+
+sub rename_volume {
+    my ($class, $scfg, $storeid, $source_volname, $target_volname, $target_vmid) = @_;
+
+    my $basedir = $class->get_subdir($scfg, 'images');
+    my $imagedir = "${basedir}/${target_vmid}";
+
+    mkpath $imagedir;
+
+    my $old_path = "${basedir}/${source_volname}";
+    my $new_path = "${imagedir}/${target_volname}";
+
+    rename($old_path, $new_path) ||
+	die "rename '$old_path' to '$new_path' failed - $!\n";
+}
+
 1;
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index 42641e2..39db2cf 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -728,6 +728,7 @@ sub volume_has_feature {
 	template => { current => 1},
 	copy => { base => 1, current => 1, snap => 1},
 	sparseinit => { base => 1, current => 1},
+	reassign => {current => 1},
     };
 
     my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) = $class->parse_volname($volname);
@@ -743,4 +744,40 @@ sub volume_has_feature {
     return undef;
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $source_volname, $target_vmid) = @_;
+
+    my $base;
+    (undef, $source_volname, undef, $base) = $class->parse_volname($source_volname);
+
+    $base = $base ? "${base}/" : '';
+
+    $class->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub {
+	my $target_volname = $class->find_free_diskname(
+	    $storeid,
+	    $scfg,
+	    $target_vmid,
+	);
+	$class->rename_volume(
+	    $scfg,
+	    $storeid,
+	    $source_volname,
+	    $target_volname,
+	    $target_vmid,
+	);
+	return "${storeid}:${base}${target_volname}";
+    });
+}
+
+sub rename_volume {
+    my ($class, $scfg, $storeid, $source_volname, $target_volname, $target_vmid) = @_;
+
+    my $cmd = $rbd_cmd->($scfg, $storeid, 'rename', $source_volname, $target_volname);
+
+    run_rbd_command(
+	$cmd,
+	errmsg => "could not rename image '${source_volname}' to '${target_volname}'",
+    );
+}
+
 1;
diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index 2e2abe3..755e117 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -687,6 +687,7 @@ sub volume_has_feature {
 	copy => { base => 1, current => 1},
 	sparseinit => { base => 1, current => 1},
 	replicate => { base => 1, current => 1},
+	reassign => {current => 1},
     };
 
     my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) =
@@ -789,4 +790,41 @@ sub volume_import_formats {
     return $class->volume_export_formats($scfg, $storeid, $volname, undef, $base_snapshot, $with_snapshots);
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $source_volname, $target_vmid) = @_;
+
+    my $base;
+    (undef, $source_volname, undef, $base) = $class->parse_volname($source_volname);
+
+    $base = $base ? "${base}/" : '';
+
+    $class->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub {
+	my $target_volname = $class->find_free_diskname(
+	    $storeid,
+	    $scfg,
+	    $target_vmid,
+	);
+	$class->rename_volume(
+	    $scfg,
+	    $storeid,
+	    $source_volname,
+	    $target_volname,
+	    $target_vmid,
+	);
+	return "${storeid}:${base}${target_volname}";
+    });
+}
+
+sub rename_volume {
+    my ($class, $scfg, $storeid, $source_volname, $target_volname, $target_vmid) = @_;
+
+    $class->zfs_request(
+	$scfg,
+	5,
+	'rename',
+	"$scfg->{pool}/$source_volname",
+	"$scfg->{pool}/$target_volname",
+    );
+}
+
 1;
-- 
2.20.1