[pve-devel] [PATCH common v2 1/4] fix #3502: VM start timeout config parameter

[pve-devel] [PATCH common v2 1/4] fix #3502: VM start timeout config parameter

[Daniel Tschlatscher]

This allows setting the 'startoptions' property string in the config.
For now this only implements the 'timeout' parameter but should be
rather easily extensible and allow related VM start config options
to be also configurable here.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
Changes from v1:
* This is now a property string instead of a simple parameter to make
  it more "future-proof"
* Maximum timeout reduced to 86400
* Revised description

I did not include the timeout property under the existing "startup"
property string because, while it is a similar feature, it is not
inherently associated with the "startup/shutdown order" functionality.
Also, this makes it more easily extensible for all start-options
that might be added in the future.
Still, as having "startup" and "startoptions" could be confusing for
some, I am open for suggestions on this!

 src/PVE/JSONSchema.pm | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index ab718f3..adabb8e 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -633,6 +633,17 @@ sub pve_verify_startup_order {
     die "unable to parse startup options\n";
 }
 
+register_format('start-options', \&pve_verify_startup_options);
+sub pve_verify_startup_options {
+    my ($value, $noerr) = @_;
+
+    return $value if pve_parse_startup_options($value);
+
+    return undef if $noerr;
+
+    die "unable to parse vm start options\n";
+}
+
 my %bwlimit_opt = (
     optional => 1,
     type => 'number', minimum => '0',
@@ -739,6 +750,33 @@ PVE::JSONSchema::register_standard_option('pve-startup-order', {
     typetext => '[[order=]\d+] [,up=\d+] [,down=\d+] ',
 });
 
+sub pve_parse_startup_options {
+    my ($value) = @_;
+
+    return undef if !$value;
+
+    my $res = {};
+
+    foreach my $p (split(/,/, $value)) {
+	next if $p =~ m/^\s*$/;
+
+	if ($p =~ m/^timeout=(\d+)$/ && int($1) <= 86400) {
+	    $res->{timeout} = $1;
+	} else {
+	    return undef;
+	}
+    }
+
+    return $res;
+}
+
+register_standard_option('start-options', {
+    description => "Start up options for the VM. This only allows setting the VM start timeout for now, which is the maximum VM startup timeout in seconds. The maximum value for timeout is 86400, the minimum 0, which disables the timeout. If timeout is unset, the timeout will either be the memory of the VM in GiBs or 30, depending on which is greater. If unset and hibernated, the value will at least be 300 seconds, with hugepages at least 150 seconds.",
+    optional => 1,
+    type => 'string', format => 'start-options',
+    typetext => 'timeout=\d+',
+});
+
 register_format('pve-tfa-secret', \&pve_verify_tfa_secret);
 sub pve_verify_tfa_secret {
     my ($key, $noerr) = @_;
-- 
2.30.2

[pve-devel] [PATCH qemu-server v2 2/4] fix #3502: VM start timeout config parameter

[Daniel Tschlatscher]

This patch makes it possible to now set the starting timeout value for
a VM via the config parameter 'startoptions'.
Now, if the timeout parameter is set, it will override the heuristic
calculation of the VM start timeout.

The maximum value for the timeout parameter is 86400 seconds, which is
one day. The minimum value is 0, which disables the timeout.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
Changes from v1:
* Now uses the "startoptions" propertystring
* Maximum timeout reduced to 86400

 PVE/API2/Qemu.pm          | 2 ++
 PVE/QemuServer.pm         | 1 +
 PVE/QemuServer/Helpers.pm | 4 ++++
 3 files changed, 7 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index a824657..7449b9f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -517,6 +517,7 @@ my $generaloptions = {
     'reboot' => 1,
     'startdate' => 1,
     'startup' => 1,
+    'startoptions' => 1,
     'tdf' => 1,
     'template' => 1,
     'tags' => 1,
@@ -2494,6 +2495,7 @@ __PACKAGE__->register_method({
 		description => "Wait maximal timeout seconds.",
 		type => 'integer',
 		minimum => 0,
+		maximum => 86400,
 		default => 'max(30, vm memory in GiB)',
 		optional => 1,
 	    },
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index e9aa248..48e43f8 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -713,6 +713,7 @@ EODESCR
 	description => "Some (read-only) meta-information about this guest.",
 	optional => 1,
     },
+    startoptions => get_standard_option('start-options'),
 };
 
 my $cicustom_fmt = {
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index c10d842..ce09db7 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -142,6 +142,10 @@ sub version_cmp {
 
 sub config_aware_timeout {
     my ($config, $is_suspended) = @_;
+
+    my $startup = PVE::JSONSchema::pve_parse_startup_options($config->{startoptions});
+    return $startup->{timeout} if defined($startup->{timeout});
+
     my $memory = $config->{memory};
     my $timeout = 30;
 
-- 
2.30.2

[pve-devel] [PATCH qemu-server v2 3/4] kill/await lingering KVM thread when VM start reaches timeout

[Daniel Tschlatscher]

In some cases the VM API start method would return before the detached
KVM process would have exited. This is especially problematic with HA,
because the HA manager would think the VM started successfully, later
see that it exited and start it again in an endless loop.

Moreover, another case exists when resuming a hibernated VM. In this
case, the qemu thread will attempt to load the whole vmstate into
memory before exiting.
Depending on vmstate size, disk read speed, and similar factors this
can take quite a while though and it is not possible to start the VM
normally during this time.

To get around this, this patch intercepts the error, looks whether a
corresponding KVM thread is still running, and waits for/kills it,
before continuing.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
Changes from v1:
* New patch

 PVE/QemuServer.pm | 40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index ac0b68f..f137f11 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5660,15 +5660,41 @@ sub vm_start_nolock {
 		$tpmpid = start_swtpm($storecfg, $vmid, $tpm, $migratedfrom);
 	    }
 
-	    my $exitcode = run_command($cmd, %run_params);
-	    if ($exitcode) {
-		if ($tpmpid) {
-		    warn "stopping swtpm instance (pid $tpmpid) due to QEMU startup error\n";
-		    kill 'TERM', $tpmpid;
+	    eval {
+		my $exitcode = run_command($cmd, %run_params);
+
+		if ($exitcode) {
+		    if ($tpmpid) {
+			warn "stopping swtpm instance (pid $tpmpid) due to QEMU startup error\n";
+			kill 'TERM', $tpmpid;
+		    }
+		    die "QEMU exited with code $exitcode\n";
 		}
-		die "QEMU exited with code $exitcode\n";
+	    };
+
+	    if (my $err = $@) {
+		my $pid = PVE::QemuServer::Helpers::vm_running_locally($vmid);
+
+		if ($pid ne "") {
+		    warn "Received error, waiting for detached qemu process $pid to exit\n";
+
+		    my $count = 0;
+		    my $timeout = 300;
+		    while (($count < $timeout) &&
+			PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
+			$count++;
+			sleep(1);
+		    }
+
+		    if ($count >= $timeout) {
+			warn "Reached timeout. Terminating now with SIGKILL\n";
+			kill(9, $pid);
+		    }
+		}
+
+		die $err;
 	    }
-	};
+	}
     };
 
     if ($conf->{hugepages}) {
-- 
2.30.2

[pve-devel] [PATCH manager v2 4/4] VM start Timeout "Options" parameter in the GUI

[Daniel Tschlatscher]

This makes it possible to set the newly introduced config parameter
for timeout via the 'startoptions' property string.
For now this only implements setting the timeout value when starting
a VM, though this should be rather easily exentensible to include
other future start options parameters.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
Changes from v1:
* Now uses a property string to encode the timeout
 www/manager6/qemu/Options.js | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
index a1def4bb..27b3ad93 100644
--- a/www/manager6/qemu/Options.js
+++ b/www/manager6/qemu/Options.js
@@ -76,6 +76,37 @@ Ext.define('PVE.qemu.Options', {
 			onlineHelp: 'qm_startup_and_shutdown',
 		    } : undefined,
 	    },
+	    startoptions: {
+		header: gettext('VM startup options'),
+		defaultValue: Proxmox.Utils.defaultText,
+		renderer: val => val,
+		editor: caps.vms['VM.Config.Options'] ? {
+		    xtype: 'proxmoxWindowEdit',
+		    subject: gettext('VM start timeout'),
+		    setValues: function(values) {
+			Ext.Array.each(this.query('inputpanel'), function(panel) {
+			    panel.setValues(PVE.Parser.parsePropertyString(values.startoptions));
+			});
+		    },
+		    items: {
+			xtype: 'inputpanel',
+			items: {
+			    xtype: 'proxmoxintegerfield',
+			    name: 'timeout',
+			    minValue: 0,
+			    maxValue: 86400,
+			    fieldLabel: gettext('Timeout (sec)'),
+			    emptyText: Proxmox.Utils.defaultText,
+			},
+			onGetValues: function(values) {
+			    if (values === undefined || Object.keys(values).length === 0) {
+				return { 'delete': 'startoptions' };
+			    }
+			    return { 'startoptions': PVE.Parser.printPropertyString(values) };
+			},
+		    },
+		} : undefined,
+	    },
 	    ostype: {
 		header: gettext('OS Type'),
 		editor: caps.vms['VM.Config.Options'] ? 'PVE.qemu.OSTypeEdit' : undefined,
-- 
2.30.2

[pve-devel] [PATCH qemu-server v2 1/1] make the timeout value editable when the VM is locked

[Daniel Tschlatscher]

In some cases the VM could no longer start when the timeout value was
set and afterwards, for example, hibernated. In this case the VM is
arguably soft locked, because the API would not allow changing the
timeout value anymore. (The only way out here would be to change the
value manually in the config)

To avoid unwanted side effects, it is possible to change the value for
the new 'startoptions' parameter, only if the VM is currently locked
with lock 'suspended'.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
Changes from v1:
* New patch

 PVE/API2/Qemu.pm | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 7449b9f..30b7f60 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -8,6 +8,7 @@ use POSIX;
 use IO::Socket::IP;
 use URI::Escape;
 use Crypt::OpenSSL::Random;
+use List::Util qw(first);
 
 use PVE::Cluster qw (cfs_read_file cfs_write_file);;
 use PVE::RRD;
@@ -626,6 +627,28 @@ my $check_vm_modify_config_perm = sub {
     return 1;
 };
 
+# Certain parameter fields should still be editable and deletable when the VM is locked
+# Returns true only if all parameter fields to be edited or deleted are defined in @allowed
+sub skiplock_for_allowed_fields {
+    my ($param, @deleted) = @_;
+
+    my @allowed = qw"startoptions";
+    my $skiplock = 1;
+
+    my @to_check = @deleted;
+    for (keys %$param) {
+	push(@to_check, $_);
+    }
+
+    my $idx = 0;
+    while ($skiplock && $idx < keys @to_check) {
+	$skiplock &= defined(first { $_ eq $to_check[$idx] } @allowed);
+	$idx++;
+    }
+
+    return $skiplock;
+}
+
 __PACKAGE__->register_method({
     name => 'vmlist',
     path => '',
@@ -1456,6 +1479,8 @@ my $update_vm_api  = sub {
 	    push @delete, 'runningcpu' if $conf->{runningcpu};
 	}
 
+	$skiplock |= $conf->{lock} eq "suspended" && skiplock_for_allowed_fields($param, @delete);
+
 	PVE::QemuConfig->check_lock($conf) if !$skiplock;
 
 	foreach my $opt (keys %$revert) {
-- 
2.30.2