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 1EFB27BBC0 for ; Tue, 2 Nov 2021 16:03:45 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7A157A95A for ; Tue, 2 Nov 2021 16:03:44 +0100 (CET) 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 097FBA870 for ; Tue, 2 Nov 2021 16:03:38 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id D343646042 for ; Tue, 2 Nov 2021 16:03:37 +0100 (CET) From: Aaron Lauterer To: pve-devel@lists.proxmox.com Date: Tue, 2 Nov 2021 16:03:34 +0100 Message-Id: <20211102150335.2371545-9-a.lauterer@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20211102150335.2371545-1-a.lauterer@proxmox.com> References: <20211102150335.2371545-1-a.lauterer@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.120 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH v4 container 8/9] api: move-volume: add move to another container 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, 02 Nov 2021 15:03:45 -0000 The goal of this is to expand the move-volume API endpoint to make it possible to move a container volume / mountpoint to another container. Currently it works for regular mountpoints though it would be nice to be able to do it for unused mounpoints as well. Signed-off-by: Aaron Lauterer --- This is mostly the code from qemu-server with some adaptions. Mainly error messages and some checks. Previous checks have been moved to '$move_to_storage_checks'. v3 -> v4: * add check to avoid potential undefined warning in $vmlist->{$vmid} * add check if volume is part of a snapshot * removed line bloat v2 -> v3: * mention default volume key in description * use $rpcenv->warn should replication snapshot removal fail * also print the original error * code style things v1 -> v2: * change --target-mp to --target-volume * make --target-volume optional and fallback to source mount point * use parse_volname instead of custom regex * adapt to find_free_volname * print warnings from update_pct_config * move running check back in both lock_config sections * fixed a few style issues rfc -> v1: * add check if target guest is replicated and fail if the moved volume does not support it * check if source volume has a format suffix and pass it to 'find_free_disk' or if the prefix is vm/subvol as those also have their own meaning, see the comment in the code * fixed some style nits src/PVE/API2/LXC.pm | 253 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 232 insertions(+), 21 deletions(-) diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm index 69df366..afc16d7 100644 --- a/src/PVE/API2/LXC.pm +++ b/src/PVE/API2/LXC.pm @@ -1813,10 +1813,12 @@ __PACKAGE__->register_method({ method => 'POST', protected => 1, proxyto => 'node', - description => "Move a rootfs-/mp-volume to a different storage", + description => "Move a rootfs-/mp-volume to a different storage or to a different container.", permissions => { description => "You need 'VM.Config.Disk' permissions on /vms/{vmid}, " . - "and 'Datastore.AllocateSpace' permissions on the storage.", + "and 'Datastore.AllocateSpace' permissions on the storage. To move ". + "a volume to another container, you need the permissions on the ". + "target container as well.", check => [ 'and', ['perm', '/vms/{vmid}', [ 'VM.Config.Disk' ]], @@ -1828,14 +1830,20 @@ __PACKAGE__->register_method({ properties => { node => get_standard_option('pve-node'), vmid => get_standard_option('pve-vmid', { completion => \&PVE::LXC::complete_ctid }), + 'target-vmid' => get_standard_option('pve-vmid', { + completion => \&PVE::LXC::complete_ctid, + optional => 1, + }), volume => { type => 'string', + #TODO: check how to handle unused mount points as the mp parameter is not configured enum => [ PVE::LXC::Config->valid_volume_keys() ], description => "Volume which will be moved.", }, storage => get_standard_option('pve-storage-id', { description => "Target Storage.", completion => \&PVE::Storage::complete_storage_enabled, + optional => 1, }), delete => { type => 'boolean', @@ -1856,6 +1864,21 @@ __PACKAGE__->register_method({ minimum => '0', default => 'clone limit from datacenter or storage config', }, + 'target-volume' => { + type => 'string', + description => "The config key the volume will be moved to. Default is the " . + "source volume key.", + enum => [PVE::LXC::Config->valid_volume_keys()], + optional => 1, + }, + 'target-digest' => { + type => 'string', + description => 'Prevent changes if current configuration file of the target " . + "container has a different SHA1 digest. This can be used to prevent " . + "concurrent modifications.', + maxLength => 40, + optional => 1, + }, }, }, returns => { @@ -1870,32 +1893,48 @@ __PACKAGE__->register_method({ my $vmid = extract_param($param, 'vmid'); + my $target_vmid = extract_param($param, 'target-vmid'); + my $storage = extract_param($param, 'storage'); my $mpkey = extract_param($param, 'volume'); + my $target_mpkey = extract_param($param, 'target-volume') // $mpkey; + + my $digest = extract_param($param, 'digest'); + + my $target_digest = extract_param($param, 'target-digest'); + my $lockname = 'disk'; my ($mpdata, $old_volid); - PVE::LXC::Config->lock_config($vmid, sub { - my $conf = PVE::LXC::Config->load_config($vmid); - PVE::LXC::Config->check_lock($conf); + die "either set storage or target-vmid, but not both\n" + if $storage && $target_vmid; - die "cannot move volumes of a running container\n" if PVE::LXC::check_running($vmid); + my $storecfg = PVE::Storage::config(); + my $source_volid; - $mpdata = PVE::LXC::Config->parse_volume($mpkey, $conf->{$mpkey}); - $old_volid = $mpdata->{volume}; + my $move_to_storage_checks = sub { + PVE::LXC::Config->lock_config($vmid, sub { + my $conf = PVE::LXC::Config->load_config($vmid); + PVE::LXC::Config->check_lock($conf); - die "you can't move a volume with snapshots and delete the source\n" - if $param->{delete} && PVE::LXC::Config->is_volume_in_use_by_snapshots($conf, $old_volid); + die "cannot move volumes of a running container\n" if PVE::LXC::check_running($vmid); - PVE::Tools::assert_if_modified($param->{digest}, $conf->{digest}); + $mpdata = PVE::LXC::Config->parse_volume($mpkey, $conf->{$mpkey}); + $old_volid = $mpdata->{volume}; - PVE::LXC::Config->set_lock($vmid, $lockname); - }); + die "you can't move a volume with snapshots and delete the source\n" + if $param->{delete} && PVE::LXC::Config->is_volume_in_use_by_snapshots($conf, $old_volid); - my $realcmd = sub { + PVE::Tools::assert_if_modified($digest, $conf->{digest}); + + PVE::LXC::Config->set_lock($vmid, $lockname); + }); + }; + + my $storage_realcmd = sub { eval { PVE::Cluster::log_msg('info', $authuser, "move volume CT $vmid: move --volume $mpkey --storage $storage"); @@ -1965,15 +2004,187 @@ __PACKAGE__->register_method({ warn $@ if $@; die $err if $err; }; - my $task = eval { - $rpcenv->fork_worker('move_volume', $vmid, $authuser, $realcmd); + + my $load_and_check_reassign_configs = sub { + my $vmlist = PVE::Cluster::get_vmlist()->{ids}; + + die "could not find CT ${vmid}\n" if !exists($vmlist->{$vmid}); + + die "Both containers need to be on the same node ($vmlist->{$vmid}->{node}) ". + "but target continer is on $vmlist->{$target_vmid}->{node}.\n" + if $vmlist->{$vmid}->{node} ne $vmlist->{$target_vmid}->{node}; + + my $source_conf = PVE::LXC::Config->load_config($vmid); + PVE::LXC::Config->check_lock($source_conf); + my $target_conf = PVE::LXC::Config->load_config($target_vmid); + PVE::LXC::Config->check_lock($target_conf); + + die "Can't move volumes from or to template VMs\n" + if ($source_conf->{template} || $target_conf->{template}); + + if ($digest) { + eval { PVE::Tools::assert_if_modified($digest, $source_conf->{digest}) }; + die "Container ${vmid}: $@" if $@; + } + + if ($target_digest) { + eval { PVE::Tools::assert_if_modified($target_digest, $target_conf->{digest}) }; + die "Container ${target_vmid}: $@" if $@; + } + + die "volume '${mpkey}' for container '$vmid' does not exist\n" + if !defined($source_conf->{$mpkey}); + + die "Target volume key '${target_mpkey}' is already in use for container '$target_vmid'\n" + if exists $target_conf->{$target_mpkey}; + + my $drive = PVE::LXC::Config->parse_volume( + $mpkey, + $source_conf->{$mpkey}, + ); + + $source_volid = $drive->{volume}; + + die "disk '${mpkey}' has no associated volume\n" + if !$source_volid; + die "Cannot move volume used by a snapshot to another container\n" + if PVE::LXC::Config->is_volume_in_use_by_snapshots($source_conf, $source_volid); + die "Storage does not support moving of this disk to another container\n" + if !PVE::Storage::volume_has_feature($storecfg, 'rename', $source_volid); + die "Cannot move a bindmound or device mount to another container\n" + if $drive->{type} ne "volume"; + die "Cannot move volume to another container while the source container is running\n" + if PVE::LXC::check_running($vmid) && $mpkey !~ m/^unused\d+$/; + + my $repl_conf = PVE::ReplicationConfig->new(); + my $is_replicated = $repl_conf->check_for_existing_jobs($target_vmid, 1); + my ($storeid, undef) = PVE::Storage::parse_volume_id($source_volid); + my $format = (PVE::Storage::parse_volname($storecfg, $source_volid))[6]; + if ( + $is_replicated + && !PVE::Storage::storage_can_replicate($storecfg, $storeid, $format) + ) { + die "Cannot move volume to a replicated container. Storage " . + "does not support replication!\n"; + } + return ($source_conf, $target_conf); }; - if (my $err = $@) { - eval { PVE::LXC::Config->remove_lock($vmid, $lockname) }; - warn $@ if $@; - die $err; + + my $logfunc = sub { + my ($msg) = @_; + print STDERR "$msg\n"; + }; + + my $volume_reassignfn = sub { + return PVE::LXC::Config->lock_config($vmid, sub { + return PVE::LXC::Config->lock_config($target_vmid, sub { + my ($source_conf, $target_conf) = &$load_and_check_reassign_configs(); + die "cannot move volumes of a running container\n" + if PVE::LXC::check_running($vmid); + + my $drive_param = PVE::LXC::Config->parse_volume( + $target_mpkey, + $source_conf->{$mpkey}, + ); + + print "moving volume '$mpkey' from container '$vmid' to '$target_vmid'\n"; + my ($storage, $source_volname) = PVE::Storage::parse_volume_id($source_volid); + + my $fmt = (PVE::Storage::parse_volname($storecfg, $source_volid))[6]; + + my $target_volname = PVE::Storage::find_free_volname( + $storecfg, + $storage, + $target_vmid, + $fmt + ); + + my $new_volid = PVE::Storage::rename_volume( + $storecfg, + $source_volid, + $target_volname, + ); + + $drive_param->{volume} = $new_volid; + + delete $source_conf->{$mpkey}; + print "removing volume '${mpkey}' from container '${vmid}' config\n"; + PVE::LXC::Config->write_config($vmid, $source_conf); + + my $drive_string = PVE::LXC::Config->print_volume($target_mpkey, $drive_param); + my $running = PVE::LXC::check_running($target_vmid); + my $param = { $target_mpkey => $drive_string }; + + my $errors = PVE::LXC::Config->update_pct_config( + $target_vmid, + $target_conf, + $running, + $param + ); + + foreach my $key (keys %$errors) { + $rpcenv->warn($errors->{$key}); + } + + PVE::LXC::Config->write_config($target_vmid, $target_conf); + $target_conf = PVE::LXC::Config->load_config($target_vmid); + + PVE::LXC::update_lxc_config($target_vmid, $target_conf); + print "target container '$target_vmid' updated with '$target_mpkey'\n"; + + # remove possible replication snapshots + if (PVE::Storage::volume_has_feature($storecfg,'replicate', $source_volid)) { + eval { + PVE::Replication::prepare( + $storecfg, + [$new_volid], + undef, + 1, + undef, + $logfunc, + ) + }; + if (my $err = $@) { + $rpcenv->warn("Failed to remove replication snapshots on volume ". + "'${target_mpkey}'. Manual cleanup could be necessary. " . + "Error: ${err}\n"); + } + } + }); + }); + }; + + if ($target_vmid) { + $rpcenv->check_vm_perm($authuser, $target_vmid, undef, ['VM.Config.Disk']) + if $authuser ne 'root@pam'; + + die "Moving a volume to the same container is not possible. Did you ". + "mean to move the volume to a different storage?\n" + if $vmid eq $target_vmid; + + &$load_and_check_reassign_configs(); + return $rpcenv->fork_worker( + 'move_volume', + "${vmid}-${mpkey}>${target_vmid}-${target_mpkey}", + $authuser, + $volume_reassignfn + ); + } elsif ($storage) { + &$move_to_storage_checks(); + my $task = eval { + $rpcenv->fork_worker('move_volume', $vmid, $authuser, $storage_realcmd); + }; + if (my $err = $@) { + eval { PVE::LXC::Config->remove_lock($vmid, $lockname) }; + warn $@ if $@; + die $err; + } + return $task; + } else { + die "Provide either a 'storage' to move the mount point to a ". + "different storage or 'target-vmid' and 'target-mp' to move ". + "the mount point to another container\n"; } - return $task; }}); __PACKAGE__->register_method({ -- 2.30.2