[pve-devel] [PATCH qemu-server v4 3/4] refactor vm_stop locked code

[Dominik Csapak]

we want to reuse most of the code in the locked context of vm_stop
for vm_reboot (since it really is just a vm_stop with a
create_reboot_request in there) so we factor that out into
_do_vm_stop
and note that it has to be called in a locked context

Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
new in v4 (split from other commit)
fix typo in comment while touching those lines
 PVE/QemuServer.pm | 148 ++++++++++++++++++++++++----------------------
 1 file changed, 77 insertions(+), 71 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b420ffb..8a1168f 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5768,85 +5768,42 @@ sub vm_stop_cleanup {
     warn $@ if $@; # avoid errors - just warn
 }
 
-# Note: use $nockeck to skip tests if VM configuration file exists.
-# We need that when migration VMs to other nodes (files already moved)
-# Note: we set $keepActive in vzdump stop mode - volumes need to stay active
-sub vm_stop {
-    my ($storecfg, $vmid, $skiplock, $nocheck, $timeout, $shutdown, $force, $keepActive, $migratedfrom) = @_;
-
-    $force = 1 if !defined($force) && !$shutdown;
-
-    if ($migratedfrom){
-	my $pid = check_running($vmid, $nocheck, $migratedfrom);
-	kill 15, $pid if $pid;
-	my $conf = PVE::QemuConfig->load_config($vmid, $migratedfrom);
-	vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 0);
-	return;
-    }
-
-    PVE::QemuConfig->lock_config($vmid, sub {
+# call only in locked context
+sub _do_vm_stop {
+    my ($storecfg, $vmid, $skiplock, $nocheck, $timeout, $shutdown, $force, $keepActive) = @_;
 
-	my $pid = check_running($vmid, $nocheck);
-	return if !$pid;
+    my $pid = check_running($vmid, $nocheck);
+    return if !$pid;
 
-	my $conf;
-	if (!$nocheck) {
-	    $conf = PVE::QemuConfig->load_config($vmid);
-	    PVE::QemuConfig->check_lock($conf) if !$skiplock;
-	    if (!defined($timeout) && $shutdown && $conf->{startup}) {
-		my $opts = PVE::JSONSchema::pve_parse_startup_order($conf->{startup});
-		$timeout = $opts->{down} if $opts->{down};
-	    }
-	    PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'pre-stop');
+    my $conf;
+    if (!$nocheck) {
+	$conf = PVE::QemuConfig->load_config($vmid);
+	PVE::QemuConfig->check_lock($conf) if !$skiplock;
+	if (!defined($timeout) && $shutdown && $conf->{startup}) {
+	    my $opts = PVE::JSONSchema::pve_parse_startup_order($conf->{startup});
+	    $timeout = $opts->{down} if $opts->{down};
 	}
+	PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'pre-stop');
+    }
 
-	eval {
-	    if ($shutdown) {
-		if (defined($conf) && parse_guest_agent($conf)->{enabled}) {
-		    vm_qmp_command($vmid, {
+    eval {
+	if ($shutdown) {
+	    if (defined($conf) && parse_guest_agent($conf)->{enabled}) {
+		vm_qmp_command($vmid, {
 			execute => "guest-shutdown",
 			arguments => { timeout => $timeout }
 		    }, $nocheck);
-		} else {
-		    vm_qmp_command($vmid, { execute => "system_powerdown" }, $nocheck);
-		}
-	    } else {
-		vm_qmp_command($vmid, { execute => "quit" }, $nocheck);
-	    }
-	};
-	my $err = $@;
-
-	if (!$err) {
-	    $timeout = 60 if !defined($timeout);
-
-	    my $count = 0;
-	    while (($count < $timeout) && check_running($vmid, $nocheck)) {
-		$count++;
-		sleep 1;
-	    }
-
-	    if ($count >= $timeout) {
-		if ($force) {
-		    warn "VM still running - terminating now with SIGTERM\n";
-		    kill 15, $pid;
-		} else {
-		    die "VM quit/powerdown failed - got timeout\n";
-		}
 	    } else {
-		vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1) if $conf;
-		return;
+		vm_qmp_command($vmid, { execute => "system_powerdown" }, $nocheck);
 	    }
 	} else {
-	    if ($force) {
-		warn "VM quit/powerdown failed - terminating now with SIGTERM\n";
-		kill 15, $pid;
-	    } else {
-		die "VM quit/powerdown failed\n";
-	    }
+	    vm_qmp_command($vmid, { execute => "quit" }, $nocheck);
 	}
+    };
+    my $err = $@;
 
-	# wait again
-	$timeout = 10;
+    if (!$err) {
+	$timeout = 60 if !defined($timeout);
 
 	my $count = 0;
 	while (($count < $timeout) && check_running($vmid, $nocheck)) {
@@ -5855,12 +5812,61 @@ sub vm_stop {
 	}
 
 	if ($count >= $timeout) {
-	    warn "VM still running - terminating now with SIGKILL\n";
-	    kill 9, $pid;
-	    sleep 1;
+	    if ($force) {
+		warn "VM still running - terminating now with SIGTERM\n";
+		kill 15, $pid;
+	    } else {
+		die "VM quit/powerdown failed - got timeout\n";
+	    }
+	} else {
+	    vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1) if $conf;
+	    return;
 	}
+    } else {
+	if ($force) {
+	    warn "VM quit/powerdown failed - terminating now with SIGTERM\n";
+	    kill 15, $pid;
+	} else {
+	    die "VM quit/powerdown failed\n";
+	}
+    }
+
+    # wait again
+    $timeout = 10;
+
+    my $count = 0;
+    while (($count < $timeout) && check_running($vmid, $nocheck)) {
+	$count++;
+	sleep 1;
+    }
+
+    if ($count >= $timeout) {
+	warn "VM still running - terminating now with SIGKILL\n";
+	kill 9, $pid;
+	sleep 1;
+    }
+
+    vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1) if $conf;
+}
+
+# Note: use $nocheck to skip tests if VM configuration file exists.
+# We need that when migration VMs to other nodes (files already moved)
+# Note: we set $keepActive in vzdump stop mode - volumes need to stay active
+sub vm_stop {
+    my ($storecfg, $vmid, $skiplock, $nocheck, $timeout, $shutdown, $force, $keepActive, $migratedfrom) = @_;
+
+    $force = 1 if !defined($force) && !$shutdown;
+
+    if ($migratedfrom){
+	my $pid = check_running($vmid, $nocheck, $migratedfrom);
+	kill 15, $pid if $pid;
+	my $conf = PVE::QemuConfig->load_config($vmid, $migratedfrom);
+	vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 0);
+	return;
+    }
 
-	vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1) if $conf;
+    PVE::QemuConfig->lock_config($vmid, sub {
+	_do_vm_stop($storecfg, $vmid, $skiplock, $nocheck, $timeout, $shutdown, $force, $keepActive);
    });
 }
 
-- 
2.20.1