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 <a.laute...@proxmox.com>
---
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'.

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 | 270 ++++++++++++++++++++++++++++++++++++++++----
 src/PVE/CLI/pct.pm  |   2 +-
 2 files changed, 250 insertions(+), 22 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index b929481..0af22c1 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -27,6 +27,8 @@ use PVE::API2::LXC::Snapshot;
 use PVE::JSONSchema qw(get_standard_option);
 use base qw(PVE::RESTHandler);
 
+use Data::Dumper;
+
 BEGIN {
     if (!$ENV{PVE_GENERATING_DOCS}) {
        require PVE::HA::Env::PVE2;
@@ -1784,10 +1786,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' ]],
@@ -1799,14 +1803,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',
@@ -1827,6 +1837,20 @@ __PACKAGE__->register_method({
                minimum => '0',
                default => 'clone limit from datacenter or storage config',
            },
+           'target-mp' => {
+               type => 'string',
+               description => "The config key the mp will be moved to.",
+               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 => {
@@ -1841,32 +1865,49 @@ __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_mp = extract_param($param, 'target-mp');
+
+       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);
+       die "cannot move volumes of a running container\n" if 
PVE::LXC::check_running($vmid);
 
-           $mpdata = PVE::LXC::Config->parse_volume($mpkey, $conf->{$mpkey});
-           $old_volid = $mpdata->{volume};
+       my $storecfg = PVE::Storage::config();
+       my $source_volid;
 
-           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 $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);
 
-           PVE::Tools::assert_if_modified($param->{digest}, $conf->{digest});
 
-           PVE::LXC::Config->set_lock($vmid, $lockname);
-       });
+               $mpdata = PVE::LXC::Config->parse_volume($mpkey, 
$conf->{$mpkey});
+               $old_volid = $mpdata->{volume};
 
-       my $realcmd = sub {
+               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);
+
+               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");
 
@@ -1936,15 +1977,202 @@ __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 "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}) };
+               if (my $err = $@) {
+                   die "Container ${vmid}: ${err}";
+               }
+           }
+
+           if ($target_digest) {
+               eval { PVE::Tools::assert_if_modified($target_digest, 
$target_conf->{digest}) };
+               if (my $err = $@) {
+                   die "Container ${target_vmid}: ${err}";
+               }
+           }
+
+           die "volume '${mpkey}' for container '$vmid' does not exist\n"
+               if !defined($source_conf->{$mpkey});
+
+           die "Target volume key '${target_mp}' is already in use for 
container '$target_vmid'\n"
+               if exists $target_conf->{$target_mp};
+
+           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 "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 PVE::LXC::Config->classify_mountpoint($source_volid) 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);
+       };
+
+       my $logfunc = sub {
+           my ($msg) = @_;
+           print STDERR "$msg\n";
        };
-       if (my $err = $@) {
-           eval { PVE::LXC::Config->remove_lock($vmid, $lockname) };
-           warn $@ if $@;
-           die $err;
+
+       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();
+
+                   my $drive_param = PVE::LXC::Config->parse_volume(
+                       $target_mp,
+                       $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);
+
+                   # The format in find_free_diskname handles two cases for 
containers.
+                   # If it is set to 'subvol', the prefix will be set to it 
instead of 'vm',
+                   # this is needed for example for ZFS based storages.
+                   # The other thing to check for, in case of directory/file 
based storages,
+                   # is .raw files as we need to pass this format in that case.
+                   my $fmt = undef;
+                   if ($source_volname =~ m/(subvol|vm)-\d+-disk-\S+$/) {
+                       $fmt = $1 if $1 eq "subvol";
+                   } else {
+                       die "could not detect source volname prefix\n";
+                   }
+                   if ($source_volname =~ m/vm-\d+-disk-\S+\.(raw)/) {
+                       $fmt = $1;
+                   }
+
+                   my $target_volname = PVE::Storage::find_free_diskname(
+                       $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_mp, $drive_param);
+                   my $running = PVE::LXC::check_running($target_vmid);
+                   my $param = { $target_mp => $drive_string };
+
+                   my $err = Dumper(PVE::LXC::Config->update_pct_config(
+                           $target_vmid,
+                           $target_conf,
+                           $running,
+                           $param
+                       ));
+
+                   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_mp'\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 = $@) {
+                           print "Failed to remove replication snapshots on 
volume ".
+                               "'$target_mp'. Manual cleanup could be 
necessary.\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_mp}",
+               $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({
diff --git a/src/PVE/CLI/pct.pm b/src/PVE/CLI/pct.pm
index 7ac5a55..5f2bf04 100755
--- a/src/PVE/CLI/pct.pm
+++ b/src/PVE/CLI/pct.pm
@@ -849,7 +849,7 @@ our $cmddef = {
 
     clone => [ "PVE::API2::LXC", 'clone_vm', ['vmid', 'newid'], { node => 
$nodename }, $upid_exit ],
     migrate => [ "PVE::API2::LXC", 'migrate_vm', ['vmid', 'target'], { node => 
$nodename }, $upid_exit],
-    'move-volume' => [ "PVE::API2::LXC", 'move_volume', ['vmid', 'volume', 
'storage'], { node => $nodename }, $upid_exit ],
+    'move-volume' => [ "PVE::API2::LXC", 'move_volume', ['vmid', 'volume', 
'storage', 'target-vmid', 'target-mp'], { node => $nodename }, $upid_exit ],
     move_volume => { alias => 'move-disk' },
 
     snapshot => [ "PVE::API2::LXC::Snapshot", 'snapshot', ['vmid', 
'snapname'], { node => $nodename } , $upid_exit ],
-- 
2.30.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to