api: move disk: fork before locking

using the familiar early+repeated checks pattern from other API calls.
Only intended functional changes are with regard to locking/forking.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 38e08c887c1865d56c473d7e433bf6531225ee99..59e083eb5efa130d4720936fa6580e8fc9818554 100644 (file)
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3396,7 +3396,7 @@ __PACKAGE__->register_method({
 
        my $storecfg = PVE::Storage::config();
 
-       my $move_updatefn =  sub {
+       my $load_and_check_move = sub {
            my $conf = PVE::QemuConfig->load_config($vmid);
            PVE::QemuConfig->check_lock($conf);
 
@@ -3416,8 +3416,8 @@ __PACKAGE__->register_method({
                $oldfmt = $1;
            }
 
-           die "you can't move to the same storage with same format\n" if $oldstoreid eq $storeid &&
-                (!$format || !$oldfmt || $oldfmt eq $format);
+           die "you can't move to the same storage with same format\n"
+               if $oldstoreid eq $storeid && (!$format || !$oldfmt || $oldfmt eq $format);
 
            # this only checks snapshots because $disk is passed!
            my $snapshotted = PVE::QemuServer::Drive::is_volume_in_use(
@@ -3429,6 +3429,13 @@ __PACKAGE__->register_method({
            die "you can't move a disk with snapshots and delete the source\n"
                if $snapshotted && $param->{delete};
 
+           return ($conf, $drive, $oldstoreid, $snapshotted);
+       };
+
+       my $move_updatefn = sub {
+           my ($conf, $drive, $oldstoreid, $snapshotted) = $load_and_check_move->();
+           my $old_volid = $drive->{file};
+
            PVE::Cluster::log_msg(
                'info',
                $authuser,
@@ -3439,83 +3446,79 @@ __PACKAGE__->register_method({
 
            PVE::Storage::activate_volumes($storecfg, [ $drive->{file} ]);
 
-           my $realcmd = sub {
-               my $newvollist = [];
+           my $newvollist = [];
 
-               eval {
-                   local $SIG{INT} =
-                       local $SIG{TERM} =
-                       local $SIG{QUIT} =
-                       local $SIG{HUP} = sub { die "interrupted by signal\n"; };
-
-                   warn "moving disk with snapshots, snapshots will not be moved!\n"
-                       if $snapshotted;
-
-                   my $bwlimit = extract_param($param, 'bwlimit');
-                   my $movelimit = PVE::Storage::get_bandwidth_limit(
-                       'move',
-                       [$oldstoreid, $storeid],
-                       $bwlimit
-                   );
+           eval {
+               local $SIG{INT} =
+                   local $SIG{TERM} =
+                   local $SIG{QUIT} =
+                   local $SIG{HUP} = sub { die "interrupted by signal\n"; };
 
-                   my $newdrive = PVE::QemuServer::clone_disk(
-                       $storecfg,
-                       $vmid,
-                       $running,
-                       $disk,
-                       $drive,
-                       undef,
-                       $vmid,
-                       $storeid,
-                       $format,
-                       1,
-                       $newvollist,
-                       undef,
-                       undef,
-                       undef,
-                       $movelimit,
-                       $conf,
-                   );
-                   $conf->{$disk} = PVE::QemuServer::print_drive($newdrive);
+               warn "moving disk with snapshots, snapshots will not be moved!\n"
+                   if $snapshotted;
 
-                   PVE::QemuConfig->add_unused_volume($conf, $old_volid) if !$param->{delete};
+               my $bwlimit = extract_param($param, 'bwlimit');
+               my $movelimit = PVE::Storage::get_bandwidth_limit(
+                   'move',
+                   [$oldstoreid, $storeid],
+                   $bwlimit
+               );
 
-                   # convert moved disk to base if part of template
-                   PVE::QemuServer::template_create($vmid, $conf, $disk)
-                       if PVE::QemuConfig->is_template($conf);
+               my $newdrive = PVE::QemuServer::clone_disk(
+                   $storecfg,
+                   $vmid,
+                   $running,
+                   $disk,
+                   $drive,
+                   undef,
+                   $vmid,
+                   $storeid,
+                   $format,
+                   1,
+                   $newvollist,
+                   undef,
+                   undef,
+                   undef,
+                   $movelimit,
+                   $conf,
+               );
+               $conf->{$disk} = PVE::QemuServer::print_drive($newdrive);
 
-                   PVE::QemuConfig->write_config($vmid, $conf);
+               PVE::QemuConfig->add_unused_volume($conf, $old_volid) if !$param->{delete};
 
-                   my $do_trim = PVE::QemuServer::get_qga_key($conf, 'fstrim_cloned_disks');
-                   if ($running && $do_trim && PVE::QemuServer::qga_check_running($vmid)) {
-                       eval { mon_cmd($vmid, "guest-fstrim") };
-                   }
+               # convert moved disk to base if part of template
+               PVE::QemuServer::template_create($vmid, $conf, $disk)
+                   if PVE::QemuConfig->is_template($conf);
 
-                   eval {
-                       # try to deactivate volumes - avoid lvm LVs to be active on several nodes
-                       PVE::Storage::deactivate_volumes($storecfg, [ $newdrive->{file} ])
-                           if !$running;
-                   };
-                   warn $@ if $@;
-               };
-               if (my $err = $@) {
-                   foreach my $volid (@$newvollist) {
-                       eval { PVE::Storage::vdisk_free($storecfg, $volid) };
-                       warn $@ if $@;
-                   }
-                   die "storage migration failed: $err";
-                }
+               PVE::QemuConfig->write_config($vmid, $conf);
 
-               if ($param->{delete}) {
-                   eval {
-                       PVE::Storage::deactivate_volumes($storecfg, [$old_volid]);
-                       PVE::Storage::vdisk_free($storecfg, $old_volid);
-                   };
-                   warn $@ if $@;
+               my $do_trim = PVE::QemuServer::get_qga_key($conf, 'fstrim_cloned_disks');
+               if ($running && $do_trim && PVE::QemuServer::qga_check_running($vmid)) {
+                   eval { mon_cmd($vmid, "guest-fstrim") };
                }
+
+               eval {
+                   # try to deactivate volumes - avoid lvm LVs to be active on several nodes
+                   PVE::Storage::deactivate_volumes($storecfg, [ $newdrive->{file} ])
+                       if !$running;
+               };
+               warn $@ if $@;
            };
+           if (my $err = $@) {
+               foreach my $volid (@$newvollist) {
+                   eval { PVE::Storage::vdisk_free($storecfg, $volid) };
+                   warn $@ if $@;
+               }
+               die "storage migration failed: $err";
+           }
 
-            return $rpcenv->fork_worker('qmmove', $vmid, $authuser, $realcmd);
+           if ($param->{delete}) {
+               eval {
+                   PVE::Storage::deactivate_volumes($storecfg, [$old_volid]);
+                   PVE::Storage::vdisk_free($storecfg, $old_volid);
+               };
+               warn $@ if $@;
+           }
        };
 
        my $load_and_check_reassign_configs = sub {
@@ -3695,7 +3698,14 @@ __PACKAGE__->register_method({
 
            die "cannot move disk '$disk', only configured disks can be moved to another storage\n"
                if $disk =~ m/^unused\d+$/;
-           return PVE::QemuConfig->lock_config($vmid, $move_updatefn);
+
+           $load_and_check_move->(); # early checks before forking/locking
+
+           my $realcmd = sub {
+               PVE::QemuConfig->lock_config($vmid, $move_updatefn);
+           };
+
+           return $rpcenv->fork_worker('qmmove', $vmid, $authuser, $realcmd);
        } else {
            my $msg = "both 'storage' and 'target-vmid' missing, either needs to be set";
            raise_param_exc({ 'target-vmid' => $msg, 'storage' => $msg });