* [pve-devel] [PATCH qemu-server v2 2/4] fix #3502: VM start timeout config parameter
2022-07-22 13:27 [pve-devel] [PATCH common v2 1/4] fix #3502: VM start timeout config parameter Daniel Tschlatscher
@ 2022-07-22 13:27 ` Daniel Tschlatscher
2022-07-22 13:27 ` [pve-devel] [PATCH qemu-server v2 3/4] kill/await lingering KVM thread when VM start reaches timeout Daniel Tschlatscher
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Daniel Tschlatscher @ 2022-07-22 13:27 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] [PATCH qemu-server v2 3/4] kill/await lingering KVM thread when VM start reaches timeout
2022-07-22 13:27 [pve-devel] [PATCH common v2 1/4] fix #3502: VM start timeout config parameter Daniel Tschlatscher
2022-07-22 13:27 ` [pve-devel] [PATCH qemu-server v2 2/4] " Daniel Tschlatscher
@ 2022-07-22 13:27 ` Daniel Tschlatscher
2022-07-22 13:27 ` [pve-devel] [PATCH manager v2 4/4] VM start Timeout "Options" parameter in the GUI Daniel Tschlatscher
2022-07-22 13:27 ` [pve-devel] [PATCH qemu-server v2 1/1] make the timeout value editable when the VM is locked Daniel Tschlatscher
3 siblings, 0 replies; 5+ messages in thread
From: Daniel Tschlatscher @ 2022-07-22 13:27 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] [PATCH manager v2 4/4] VM start Timeout "Options" parameter in the GUI
2022-07-22 13:27 [pve-devel] [PATCH common v2 1/4] fix #3502: VM start timeout config parameter Daniel Tschlatscher
2022-07-22 13:27 ` [pve-devel] [PATCH qemu-server v2 2/4] " Daniel Tschlatscher
2022-07-22 13:27 ` [pve-devel] [PATCH qemu-server v2 3/4] kill/await lingering KVM thread when VM start reaches timeout Daniel Tschlatscher
@ 2022-07-22 13:27 ` Daniel Tschlatscher
2022-07-22 13:27 ` [pve-devel] [PATCH qemu-server v2 1/1] make the timeout value editable when the VM is locked Daniel Tschlatscher
3 siblings, 0 replies; 5+ messages in thread
From: Daniel Tschlatscher @ 2022-07-22 13:27 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] [PATCH qemu-server v2 1/1] make the timeout value editable when the VM is locked
2022-07-22 13:27 [pve-devel] [PATCH common v2 1/4] fix #3502: VM start timeout config parameter Daniel Tschlatscher
` (2 preceding siblings ...)
2022-07-22 13:27 ` [pve-devel] [PATCH manager v2 4/4] VM start Timeout "Options" parameter in the GUI Daniel Tschlatscher
@ 2022-07-22 13:27 ` Daniel Tschlatscher
3 siblings, 0 replies; 5+ messages in thread
From: Daniel Tschlatscher @ 2022-07-22 13:27 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread