[pve-devel] [PATCH v3 storage 3/4] add disk reassign feature

Aaron Lauterer a.lauterer at proxmox.com
Thu Sep 10 16:32:41 CEST 2020


Functionality has been added for the following storage types:

* dir based ones
    * directory
    * NFS
    * CIFS
    * gluster
* ZFS
* (thin) LVM
* Ceph

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

A new intermediate class for directory based storages has been
introduced. This was necessary to maintain compatibility with third
party storage plugins and to avoid duplicate code in the dir based
plugins.

The new `BaseDirPlugin.pm` adds the `reassign` feature flag and
containes the implementation for the reassign functionlity.

In the future all the directory specific code in Plugin.pm should be
moved to the BaseDirPlugin.pm. But this will most likely break
compatibility with third party plugins and should thus be done with
care.

Signed-off-by: Aaron Lauterer <a.lauterer at proxmox.com>
---
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


 PVE/Storage.pm                 | 10 ++++++
 PVE/Storage/BaseDirPlugin.pm   | 61 ++++++++++++++++++++++++++++++++++
 PVE/Storage/CIFSPlugin.pm      |  2 +-
 PVE/Storage/DirPlugin.pm       |  2 +-
 PVE/Storage/GlusterfsPlugin.pm |  2 +-
 PVE/Storage/LVMPlugin.pm       | 24 +++++++++++++
 PVE/Storage/LvmThinPlugin.pm   |  1 +
 PVE/Storage/Makefile           |  1 +
 PVE/Storage/NFSPlugin.pm       |  2 +-
 PVE/Storage/Plugin.pm          | 22 ++++++------
 PVE/Storage/RBDPlugin.pm       | 31 +++++++++++++++++
 PVE/Storage/ZFSPoolPlugin.pm   | 28 ++++++++++++++++
 12 files changed, 172 insertions(+), 14 deletions(-)
 create mode 100644 PVE/Storage/BaseDirPlugin.pm

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/BaseDirPlugin.pm b/PVE/Storage/BaseDirPlugin.pm
new file mode 100644
index 0000000..b7f4ae4
--- /dev/null
+++ b/PVE/Storage/BaseDirPlugin.pm
@@ -0,0 +1,61 @@
+package PVE::Storage::BaseDirPlugin;
+
+use strict;
+use warnings;
+
+use File::Path;
+use Data::Dumper;
+
+use base qw(PVE::Storage::Plugin);
+
+$PVE::Storage::Plugin::features->{reassign} = { current => {
+	    'raw' => 1,
+	    'qcow2'=> 1,
+	    'vmdk' => 1,
+	}
+    };
+
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+
+    my $base;
+    my $base_vmid;
+    my $format;
+    my $source_vmid;
+
+    (undef, $volname, $source_vmid, $base, $base_vmid, undef, $format) = $class->parse_volname($volname);
+
+    # parse_volname strips the directory from volname
+    my $source_volname = "${source_vmid}/${volname}";
+
+    if ($base) {
+	$base = "${base_vmid}/${base}/";
+    } else {
+	$base = '';
+    }
+
+    $class->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub {
+	my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format, 1);
+
+	return $class->rename_volume($scfg, $storeid, $source_volname, $target_volname, $target_vmid, $base);
+    });
+}
+
+sub rename_volume {
+    my ($class, $scfg, $storeid, $source_volname, $target_volname, $vmid, $base) = @_;
+
+    my $basedir = $class->get_subdir($scfg, 'images');
+    my $imagedir = "${basedir}/${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";
+
+    return "${storeid}:${base}${vmid}/${target_volname}";
+}
+
+1;
diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index 6edbc9d..d9714dd 100644
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -9,7 +9,7 @@ use File::Path;
 use PVE::Storage::Plugin;
 use PVE::JSONSchema qw(get_standard_option);
 
-use base qw(PVE::Storage::Plugin);
+use base qw(PVE::Storage::BaseDirPlugin);
 
 # CIFS helper functions
 
diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
index 3c81d24..66b590e 100644
--- a/PVE/Storage/DirPlugin.pm
+++ b/PVE/Storage/DirPlugin.pm
@@ -7,7 +7,7 @@ use File::Path;
 use PVE::Storage::Plugin;
 use PVE::JSONSchema qw(get_standard_option);
 
-use base qw(PVE::Storage::Plugin);
+use base qw(PVE::Storage::BaseDirPlugin);
 
 # Configuration
 
diff --git a/PVE/Storage/GlusterfsPlugin.pm b/PVE/Storage/GlusterfsPlugin.pm
index 2dd414d..14111e6 100644
--- a/PVE/Storage/GlusterfsPlugin.pm
+++ b/PVE/Storage/GlusterfsPlugin.pm
@@ -10,7 +10,7 @@ use PVE::Network;
 use PVE::Storage::Plugin;
 use PVE::JSONSchema qw(get_standard_option);
 
-use base qw(PVE::Storage::Plugin);
+use base qw(PVE::Storage::BaseDirPlugin);
 
 # Glusterfs helper functions
 
diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
index c0740d4..4203eda 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) = @_;
 
@@ -581,6 +588,7 @@ sub volume_has_feature {
 
     my $features = {
 	copy => { base => 1, current => 1},
+	reassign => {current => 1},
     };
 
     my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) =
@@ -681,4 +689,20 @@ sub volume_import_write {
 	input => '<&'.fileno($input_fh));
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+
+    $class->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub {
+	my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
+	return $class->rename_volume($scfg, $storeid, $volname, $target_volname, $target_vmid);
+    });
+}
+
+sub rename_volume {
+    my ($class, $scfg, $storeid, $source_volname, $target_volname, $vmid) = @_;
+
+    lvrename($scfg->{vgname}, $source_volname, $target_volname);
+    return "${storeid}:${target_volname}";
+}
+
 1;
diff --git a/PVE/Storage/LvmThinPlugin.pm b/PVE/Storage/LvmThinPlugin.pm
index d1c5b1f..adc4ae2 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/Makefile b/PVE/Storage/Makefile
index 5b8f6e8..936fbf2 100644
--- a/PVE/Storage/Makefile
+++ b/PVE/Storage/Makefile
@@ -1,5 +1,6 @@
 SOURCES= \
 	Plugin.pm \
+	BaseDirPlugin.pm \
 	DirPlugin.pm \
 	LVMPlugin.pm \
 	NFSPlugin.pm \
diff --git a/PVE/Storage/NFSPlugin.pm b/PVE/Storage/NFSPlugin.pm
index 6abb24b..f91decc 100644
--- a/PVE/Storage/NFSPlugin.pm
+++ b/PVE/Storage/NFSPlugin.pm
@@ -10,7 +10,7 @@ use PVE::ProcFSTools;
 use PVE::Storage::Plugin;
 use PVE::JSONSchema qw(get_standard_option);
 
-use base qw(PVE::Storage::Plugin);
+use base qw(PVE::Storage::BaseDirPlugin);
 
 # NFS helper functions
 
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index e6cd99c..581c581 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -888,19 +888,20 @@ sub storage_can_replicate {
     return 0;
 }
 
+our $features = {
+    snapshot => { current => { qcow2 => 1}, snap => { qcow2 => 1} },
+    clone => { base => {qcow2 => 1, raw => 1, vmdk => 1} },
+    template => { current => {qcow2 => 1, raw => 1, vmdk => 1, subvol => 1} },
+    copy => { base => {qcow2 => 1, raw => 1, vmdk => 1},
+	      current => {qcow2 => 1, raw => 1, vmdk => 1},
+	      snap => {qcow2 => 1} },
+    sparseinit => { base => {qcow2 => 1, raw => 1, vmdk => 1},
+		    current => {qcow2 => 1, raw => 1, vmdk => 1} },
+};
+
 sub volume_has_feature {
     my ($class, $scfg, $feature, $storeid, $volname, $snapname, $running, $opts) = @_;
 
-    my $features = {
-	snapshot => { current => { qcow2 => 1}, snap => { qcow2 => 1} },
-	clone => { base => {qcow2 => 1, raw => 1, vmdk => 1} },
-	template => { current => {qcow2 => 1, raw => 1, vmdk => 1, subvol => 1} },
-	copy => { base => {qcow2 => 1, raw => 1, vmdk => 1},
-		  current => {qcow2 => 1, raw => 1, vmdk => 1},
-		  snap => {qcow2 => 1} },
-	sparseinit => { base => {qcow2 => 1, raw => 1, vmdk => 1},
-			current => {qcow2 => 1, raw => 1, vmdk => 1} },
-    };
 
     # clone_image creates a qcow2 volume
     return 0 if $feature eq 'clone' &&
@@ -1411,4 +1412,5 @@ sub volume_import_formats {
     return ();
 }
 
+
 1;
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index 38f2b46..325937a 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -703,6 +703,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) =
@@ -719,4 +720,34 @@ sub volume_has_feature {
     return undef;
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+
+    my $base;;
+    (undef, $volname, undef, $base) = $class->parse_volname($volname);
+
+    if ($base) {
+	$base = $base . '/';
+    } else {
+	$base = '';
+    }
+
+    $class->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub {
+	my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
+	return $class->rename_volume($scfg, $storeid, $volname, $target_volname, $target_vmid, $base);
+    });
+}
+
+sub rename_volume {
+    my ($class, $scfg, $storeid, $source_volname, $target_volname, $vmid, $base) = @_;
+
+    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}'",
+    );
+    return "${storeid}:${base}${target_volname}";
+}
+
 1;
diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index 10354b3..9f5f055 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -9,6 +9,8 @@ use PVE::Storage::Plugin;
 use PVE::RPCEnvironment;
 use Net::IP;
 
+use Data::Dumper;
+
 use base qw(PVE::Storage::Plugin);
 
 sub type {
@@ -647,6 +649,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) =
@@ -749,4 +752,29 @@ 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 $base;;
+    (undef, $volname, undef, $base) = $class->parse_volname($volname);
+
+    if ($base) {
+	$base = $base . '/';
+    } else {
+	$base = '';
+    }
+
+    $class->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub {
+	my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
+	return $class->rename_volume($scfg, $storeid, $volname, $target_volname, $target_vmid, $base);
+    });
+}
+
+sub rename_volume {
+    my ($class, $scfg, $storeid, $source_volname, $target_volname, $vmid, $base) = @_;
+
+    $class->zfs_request($scfg, 5, 'rename', "$scfg->{pool}/$source_volname", "$scfg->{pool}/$target_volname");
+    return "${storeid}:${base}${target_volname}";
+}
+
 1;
-- 
2.20.1





More information about the pve-devel mailing list