[pve-devel] [PATCH container v2 2/6] api: status: move config locking from API handler into worker

[Friedrich Weber]

Previously, container start/stop/shutdown/suspend would try to acquire
the config lock in the API handler prior to forking a worker. If the
lock was currently held elsewhere, this would block the API handler
and thus the pvedaemon worker thread until the 10s timeout expired (or
the lock could be acquired).

To avoid blocking the API handler, immediately fork off a worker
process and try to acquire the config lock in that worker.

Patch best viewed with `git show -w`.

Suggested-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
Signed-off-by: Friedrich Weber <f.weber at proxmox.com>
---

Notes:
    The diff is somewhat messy without `-w` -- couldn't come up with a
    better way.
    
    new in v2

 src/PVE/API2/LXC/Status.pm | 91 ++++++++++++++++++--------------------
 1 file changed, 42 insertions(+), 49 deletions(-)

diff --git a/src/PVE/API2/LXC/Status.pm b/src/PVE/API2/LXC/Status.pm
index f7e3128..7741374 100644
--- a/src/PVE/API2/LXC/Status.pm
+++ b/src/PVE/API2/LXC/Status.pm
@@ -176,32 +176,31 @@ __PACKAGE__->register_method({
 	    my $realcmd = sub {
 		my $upid = shift;
 
-		syslog('info', "starting CT $vmid: $upid\n");
+		PVE::LXC::Config->lock_config($vmid, sub {
+		    syslog('info', "starting CT $vmid: $upid\n");
 
-		my $conf = PVE::LXC::Config->load_config($vmid);
-
-		die "you can't start a CT if it's a template\n"
-		    if PVE::LXC::Config->is_template($conf);
+		    my $conf = PVE::LXC::Config->load_config($vmid);
 
-		if (!$skiplock && !PVE::LXC::Config->has_lock($conf, 'mounted')) {
-		    PVE::LXC::Config->check_lock($conf);
-		}
+		    die "you can't start a CT if it's a template\n"
+			if PVE::LXC::Config->is_template($conf);
 
-		if ($conf->{unprivileged}) {
-		    PVE::LXC::Config->foreach_volume($conf, sub {
-			my ($ms, $mountpoint) = @_;
-			die "Quotas are not supported by unprivileged containers.\n" if $mountpoint->{quota};
-		    });
-		}
+		    if (!$skiplock && !PVE::LXC::Config->has_lock($conf, 'mounted')) {
+			PVE::LXC::Config->check_lock($conf);
+		    }
 
-		PVE::LXC::vm_start($vmid, $conf, $skiplock, $param->{debug});
-	    };
+		    if ($conf->{unprivileged}) {
+			PVE::LXC::Config->foreach_volume($conf, sub {
+			    my ($ms, $mountpoint) = @_;
+			    die "Quotas are not supported by unprivileged containers.\n"
+				if $mountpoint->{quota};
+			});
+		    }
 
-	    my $lockcmd = sub {
-		return $rpcenv->fork_worker('vzstart', $vmid, $authuser, $realcmd);
+		    PVE::LXC::vm_start($vmid, $conf, $skiplock, $param->{debug});
+		});
 	    };
 
-	    return PVE::LXC::Config->lock_config($vmid, $lockcmd);
+	    return $rpcenv->fork_worker('vzstart', $vmid, $authuser, $realcmd);
 	}
     }});
 
@@ -258,21 +257,19 @@ __PACKAGE__->register_method({
 	    my $realcmd = sub {
 		my $upid = shift;
 
-		syslog('info', "stopping CT $vmid: $upid\n");
+		PVE::LXC::Config->lock_config($vmid, sub {
+		    syslog('info', "stopping CT $vmid: $upid\n");
 
-		my $conf = PVE::LXC::Config->load_config($vmid);
-		if (!$skiplock && !PVE::LXC::Config->has_lock($conf, 'mounted')) {
-		    PVE::LXC::Config->check_lock($conf);
-		}
+		    my $conf = PVE::LXC::Config->load_config($vmid);
+		    if (!$skiplock && !PVE::LXC::Config->has_lock($conf, 'mounted')) {
+			PVE::LXC::Config->check_lock($conf);
+		    }
 
-		PVE::LXC::vm_stop($vmid, 1);
+		    PVE::LXC::vm_stop($vmid, 1);
+		});
 	    };
 
-	    my $lockcmd = sub {
-		return $rpcenv->fork_worker('vzstop', $vmid, $authuser, $realcmd);
-	    };
-
-	    return PVE::LXC::Config->lock_config($vmid, $lockcmd);
+	    return $rpcenv->fork_worker('vzstop', $vmid, $authuser, $realcmd);
 	}
     }});
 
@@ -339,19 +336,17 @@ __PACKAGE__->register_method({
 	my $realcmd = sub {
 	    my $upid = shift;
 
-	    syslog('info', "shutdown CT $vmid: $upid\n");
-
-	    my $conf = PVE::LXC::Config->load_config($vmid);
-	    PVE::LXC::Config->check_lock($conf);
+	    PVE::LXC::Config->lock_config($vmid, sub {
+		syslog('info', "shutdown CT $vmid: $upid\n");
 
-	    PVE::LXC::vm_stop($vmid, 0, $timeout, $param->{forceStop});
-	};
+		my $conf = PVE::LXC::Config->load_config($vmid);
+		PVE::LXC::Config->check_lock($conf);
 
-	my $lockcmd = sub {
-	    return $rpcenv->fork_worker('vzshutdown', $vmid, $authuser, $realcmd);
+		PVE::LXC::vm_stop($vmid, 0, $timeout, $param->{forceStop});
+	    });
 	};
 
-	return PVE::LXC::Config->lock_config($vmid, $lockcmd);
+	return $rpcenv->fork_worker('vzshutdown', $vmid, $authuser, $realcmd);
     }});
 
 __PACKAGE__->register_method({
@@ -388,20 +383,18 @@ __PACKAGE__->register_method({
 	my $realcmd = sub {
 	    my $upid = shift;
 
-	    syslog('info', "suspend CT $vmid: $upid\n");
-
-	    my $conf = PVE::LXC::Config->load_config($vmid);
-	    PVE::LXC::Config->check_lock($conf);
+	    PVE::LXC::Config->lock_config($vmid, sub {
+		syslog('info', "suspend CT $vmid: $upid\n");
 
-	    my $cmd = ['lxc-checkpoint', '-n', $vmid, '-s', '-D', '/var/lib/vz/dump'];
-	    run_command($cmd);
-	};
+		my $conf = PVE::LXC::Config->load_config($vmid);
+		PVE::LXC::Config->check_lock($conf);
 
-	my $lockcmd = sub {
-	    return $rpcenv->fork_worker('vzsuspend', $vmid, $authuser, $realcmd);
+		my $cmd = ['lxc-checkpoint', '-n', $vmid, '-s', '-D', '/var/lib/vz/dump'];
+		run_command($cmd);
+	    });
 	};
 
-	return PVE::LXC::Config->lock_config($vmid, $lockcmd);
+	return $rpcenv->fork_worker('vzsuspend', $vmid, $authuser, $realcmd);
     }});
 
 __PACKAGE__->register_method({
-- 
2.39.2