From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 021A674F4C for ; Tue, 20 Apr 2021 18:44:27 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B8BB826B5A for ; Tue, 20 Apr 2021 18:43:56 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id D4DE6269E6 for ; Tue, 20 Apr 2021 18:43:52 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 0AB4345F38 for ; Tue, 20 Apr 2021 18:34:15 +0200 (CEST) From: Aaron Lauterer To: pve-devel@lists.proxmox.com Date: Tue, 20 Apr 2021 18:34:08 +0200 Message-Id: <20210420163412.31465-2-a.lauterer@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210420163412.31465-1-a.lauterer@proxmox.com> References: <20210420163412.31465-1-a.lauterer@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.000 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_SHORT 0.001 Use of a URL Shortener for very short URL SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [gnu.org, lvmplugin.pm, rbdplugin.pm, lvmthinplugin.pm, plugin.pm, plugins.pm, proxmox.com, basedirplugin.pm, storage.pm, zfspoolplugin.pm] Subject: [pve-devel] [PATCH v7 storage 1/5] add disk reassign feature X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Apr 2021 16:44:27 -0000 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 --- 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