[PATCH docs/manager/qemu-server 0/6] fix #7213: add `powercycle` reboot behavior

[PATCH docs/manager/qemu-server 0/6] fix #7213: add `powercycle` reboot behavior

[Jakob Klocker]

A reboot initiated inside the guest is handled by QEMU itself: the 
process keeps running and only the guest OS boots again. That is fast,
but the VM stays on the configuration and QEMU binary it was started
with, so pending changes remain pending.

This series adds a `powercycle` sub-property to the existing `reboot`
option. With it set, the VM is started with `-no-reboot`, so QEMU exits
on a reset; qmeventd reports the reason, and `qm cleanup` starts
the VM again. The result is a full start, so pending changes take
effect.

To carry the new sub-property, `reboot` is converted to a property
string. Existing configurations stay valid, as the previous boolean
value is the default key.

Note two behavior changes beyond the new sub-property: `reboot: 0` now
also prevents a restart for reboots triggered through the web interface,
`qm` or the API - previously only guest-initiated reboots were affected.
The documented meaning of the option is that the VM does not reboot, so
this seemed like the more consistent reading. And with `powercycle` set,
a reset requested through the web interface, `qm` or the API also becomes
a full stop and start, since `-no-reboot` makes QEMU exit on any reset.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7213


qemu-server:

Jakob Klocker (4):
  qm: do not restart VM when 'reboot' is disabled
  fix #7213: config: add `powercycle` sub-property to `reboot`
  fix #7213: qm: cleanup: add `reset` parameter to honor `powercycle`
  fix #7213: qmeventd: pass `reset` to `qm cleanup`

 src/PVE/CLI/qm.pm       | 22 +++++++++++++++++++---
 src/PVE/QemuServer.pm   | 41 ++++++++++++++++++++++++++++++++++++-----
 src/qmeventd/qmeventd.c | 36 ++++++++++++++++++++++++++++--------
 src/qmeventd/qmeventd.h |  1 +
 4 files changed, 84 insertions(+), 16 deletions(-)


pve-manager:

Jakob Klocker (1):
  ui: qemu: options: add editor for reboot behavior

 www/manager6/Makefile                      |  1 +
 www/manager6/Utils.js                      | 17 +++++++
 www/manager6/form/RebootFeatureSelector.js | 59 ++++++++++++++++++++++
 www/manager6/qemu/Options.js               | 14 +++++
 4 files changed, 91 insertions(+)
 create mode 100644 www/manager6/form/RebootFeatureSelector.js


pve-docs:

Jakob Klocker (1):
  qm: add reboot behavior information

 qm.adoc | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)


Summary over all repositories:
  9 files changed, 212 insertions(+), 16 deletions(-)

-- 
Generated by murpp 0.12.0

[PATCH qemu-server 1/6] qm: do not restart VM when 'reboot' is disabled

[Jakob Klocker]

The `reboot` option is documented as making the VM exit instead of
rebooting when set to `0`, but `qm cleanup` restarts it regardless.

Only restart if the option is not disabled. This also covers reboots
requested through `qm reboot` or the API, which so far restarted the VM
even with `reboot` set to '0'.

Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
---
 src/PVE/CLI/qm.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/CLI/qm.pm b/src/PVE/CLI/qm.pm
index b903c1f1..f35b8a30 100755
--- a/src/PVE/CLI/qm.pm
+++ b/src/PVE/CLI/qm.pm
@@ -1160,8 +1160,9 @@ __PACKAGE__->register_method({
                     PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-stop');
                 }
 
-                $restart = eval { PVE::QemuServer::clear_reboot_request($vmid) };
+                my $reboot_requested = eval { PVE::QemuServer::clear_reboot_request($vmid) };
                 warn $@ if $@;
+                $restart = $reboot_requested && ($conf->{reboot} // 1);
             },
         );
 
-- 
2.47.3

[PATCH qemu-server 2/6] fix #7213: config: add `powercycle` sub-property to `reboot`

[Jakob Klocker]

A guest-initiated reboot is handled by QEMU itself, so the process keeps
running and the VM stays on the configuration and QEMU binary it was
started with. Pending changes therefore remain pending.

With `powercycle` set, start the VM with `-no-reboot` so that QEMU exits
on a reset instead. `qm cleanup` then starts the VM again, which makes
it a full start and applies pending changes. Note that `-no-reboot`
covers resets requested through the API as well, so those also become a
full stop and start.

Convert `reboot` to a property string to carry the new sub-property.
Existing configurations stay valid, as the previous boolean value is the
default key.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7213
Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
---
 src/PVE/QemuServer.pm | 41 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 2f43faa7..1727f143 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -237,6 +237,27 @@ my $spice_enhancements_fmt = {
     },
 };
 
+my $reboot_fmt = {
+    enabled => {
+        default_key => 1,
+        type => 'boolean',
+        default => 1,
+        optional => 1,
+        description => "Allow reboot. If set to '0' the VM is shut down instead of rebooted"
+            . " and stays off, no matter whether the reboot was initiated by the guest or"
+            . " through the API.",
+    },
+    powercycle => {
+        type => 'boolean',
+        optional => 1,
+        default => 0,
+        description => "Stop and start the VM on a reset instead of resetting it in place, so"
+            . " pending changes are applied and the currently installed QEMU version is used."
+            . " Applies to resets initiated by the guest as well as through the API. Has no"
+            . " effect when reboot is disabled.",
+    },
+};
+
 my $confdesc = {
     onboot => {
         optional => 1,
@@ -263,9 +284,9 @@ my $confdesc = {
     },
     reboot => {
         optional => 1,
-        type => 'boolean',
-        description => "Allow reboot. If set to '0' the VM exit on reboot.",
-        default => 1,
+        type => 'string',
+        format => $reboot_fmt,
+        description => "Reboot behavior and its properties.",
     },
     lock => {
         optional => 1,
@@ -1694,6 +1715,16 @@ sub parse_vga {
     return $res;
 }
 
+sub parse_reboot {
+    my ($value) = @_;
+
+    return { enabled => 1 } if !defined($value) || $value eq '';
+
+    my $res = eval { parse_property_string($reboot_fmt, $value) };
+    warn $@ if $@;
+    return $res // {};
+}
+
 sub qemu_created_version_fixups {
     my ($conf, $forcemachine, $kvmver) = @_;
 
@@ -3216,6 +3247,8 @@ sub config_to_command {
         push @$cmd, '-name', "$vmname,debug-threads=on";
     }
 
+    my $reboot = parse_reboot($conf->{reboot});
+    push @$cmd, '-no-reboot' if !($reboot->{enabled} // 1) || $reboot->{powercycle};
     push @$cmd, '-no-shutdown';
 
     my $use_virtio = 0;
@@ -3400,8 +3433,6 @@ sub config_to_command {
 
     push $machineFlags->@*, 'acpi=off' if defined($conf->{acpi}) && $conf->{acpi} == 0;
 
-    push @$cmd, '-no-reboot' if defined($conf->{reboot}) && $conf->{reboot} == 0;
-
     if ($vga->{type} && $vga->{type} !~ m/^serial\d+$/ && $vga->{type} ne 'none') {
         push @$devices, '-device',
             print_vga_device($conf, $vga, $arch, $machine_version, undef, $qxlnum, $bridges);
-- 
2.47.3

[PATCH qemu-server 3/6] fix #7213: qm: cleanup: add `reset` parameter to honor `powercycle`

[Jakob Klocker]

With `-no-reboot`, QEMU exits on a guest-initiated reset exactly like it
does on a regular shutdown, so `qm cleanup` cannot tell the two apart on
its own. Add an optional 'reset' parameter which qmeventd sets
accordingly.

Start the VM again if the shutdown was caused by a guest reset and
`powercycle` is set, in addition to the existing case of an explicit
reboot request. Both cases stay gated on `enabled`, so a VM with reboot
disabled always stays off.

A reset also has to be treated like a guest-initiated shutdown when
deciding whether to run the cleanup, as it does not go through the
regular stop path either.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7213
Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
---
 src/PVE/CLI/qm.pm | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/PVE/CLI/qm.pm b/src/PVE/CLI/qm.pm
index f35b8a30..c64f8ebb 100755
--- a/src/PVE/CLI/qm.pm
+++ b/src/PVE/CLI/qm.pm
@@ -1078,6 +1078,15 @@ __PACKAGE__->register_method({
                 description =>
                     "Indicates if the shutdown was requested by the guest or via qmp.",
             },
+            reset => {
+                type => 'boolean',
+                description =>
+                    "Indicates if the shutdown was caused by a reset request, either from"
+                    . " inside the guest or through QMP, which QEMU turns into a shutdown"
+                    . " when started with '-no-reboot'.",
+                optional => 1,
+                default => 0,
+            },
         },
     },
     returns => { type => 'null' },
@@ -1087,6 +1096,7 @@ __PACKAGE__->register_method({
         my $vmid = $param->{vmid};
         my $clean = $param->{'clean-shutdown'};
         my $guest = $param->{'guest-requested'};
+        my $reset = $param->{reset};
         my $restart = 0;
 
         # return if we do not have the config anymore
@@ -1149,7 +1159,7 @@ __PACKAGE__->register_method({
 
                 my $can_use_cleanup_flag = PVE::QemuServer::RunState::can_use_cleanup_flag();
 
-                if (!$clean || $guest || $can_use_cleanup_flag) {
+                if (!$clean || $guest || $reset || $can_use_cleanup_flag) {
                     # either we can use the new mechanism to check if cleanup is done, or
                     # vm was shutdown from inside the guest or crashed
                     PVE::QemuServer::vm_stop_cleanup($storecfg, $vmid, $conf, 0, 0, 1);
@@ -1160,9 +1170,12 @@ __PACKAGE__->register_method({
                     PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-stop');
                 }
 
+                my $reboot = PVE::QemuServer::parse_reboot($conf->{reboot});
                 my $reboot_requested = eval { PVE::QemuServer::clear_reboot_request($vmid) };
                 warn $@ if $@;
-                $restart = $reboot_requested && ($conf->{reboot} // 1);
+
+                $restart = ($reboot_requested || ($reset && $reboot->{powercycle}))
+                    && ($reboot->{enabled} // 1);
             },
         );
 
@@ -1473,7 +1486,9 @@ our $cmddef = {
 
     importovf => [__PACKAGE__, 'importovf', ['vmid', 'manifest', 'storage']],
 
-    cleanup => [__PACKAGE__, 'cleanup', ['vmid', 'clean-shutdown', 'guest-requested'], {%node}],
+    cleanup => [
+        __PACKAGE__, 'cleanup', ['vmid', 'clean-shutdown', 'guest-requested', 'reset'], {%node},
+    ],
 
     cloudinit => {
         dump => [
-- 
2.47.3

[PATCH qemu-server 4/6] fix #7213: qmeventd: pass `reset` to `qm cleanup`

[Jakob Klocker]

With '-no-reboot', QEMU exits on a reset request instead of performing
it. The resulting SHUTDOWN event is indistinguishable from the one for
an ordinary shutdown except for its reason field. Evaluate that field
and pass the result on as a fourth argument, which `qm cleanup` needs to
decide whether the VM should be started again.

Both a reset initiated inside the guest (`guest-reset`) and one
requested through QMP (`host-qmp-system-reset`, used by `qm reset` and
the API) are reported: a VM with `powercycle` set is started with
'-no-reboot' either way, so both would otherwise leave it powered off.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7213
Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
---
 src/qmeventd/qmeventd.c | 36 ++++++++++++++++++++++++++++--------
 src/qmeventd/qmeventd.h |  1 +
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/src/qmeventd/qmeventd.c b/src/qmeventd/qmeventd.c
index 84ee11a1..14b17667 100644
--- a/src/qmeventd/qmeventd.c
+++ b/src/qmeventd/qmeventd.c
@@ -10,12 +10,15 @@
     qmeventd listens on a given socket, and waits for qemu processes to
     connect. After accepting a connection qmeventd waits for shutdown events
     followed by the closing of the socket. Once that happens `qm cleanup` will
-    be executed with following three arguments:
-    VMID <graceful> <guest>
+    be executed with following four arguments:
+    VMID <graceful> <guest> <reset>
     Where `graceful` can be `1` or `0` depending if shutdown event was observed
     before the socket got closed. The second parameter `guest` is also boolean
     `1` or `0` depending if the shutdown was requested from the guest OS
-    (i.e., the "inside").
+    (i.e., the "inside"). The third parameter `reset` is `1` if the shutdown
+    was caused by a reset request rather than an actual shutdown - either from
+    inside the guest or through QMP - which QEMU only reports as a shutdown
+    event when started with `-no-reboot`.
 */
 
 #ifndef _GNU_SOURCE
@@ -207,9 +210,20 @@ void handle_qmp_event(struct Client *client, struct json_object *obj) {
         client->qemu.graceful = 1;
         struct json_object *data;
         struct json_object *guest;
-        if (json_object_object_get_ex(obj, "data", &data) &&
-            json_object_object_get_ex(data, "guest", &guest)) {
-            client->qemu.guest = (unsigned short)json_object_get_boolean(guest);
+        struct json_object *reason;
+
+        if (json_object_object_get_ex(obj, "data", &data) && data) {
+            if (json_object_object_get_ex(data, "guest", &guest)) {
+                client->qemu.guest = (unsigned short)json_object_get_boolean(guest);
+            }
+
+            if (json_object_object_get_ex(data, "reason", &reason)) {
+                const char *reason_str = json_object_get_string(reason);
+                if (reason_str && (!strcmp(reason_str, "guest-reset") ||
+                                   !strcmp(reason_str, "host-qmp-system-reset"))) {
+                    client->qemu.reset = true;
+                }
+            }
         }
 
         // check if a backup is running and kill QEMU process if not
@@ -392,11 +406,14 @@ err:
 static void cleanup_qemu_client(struct Client *client) {
     unsigned short graceful = client->qemu.graceful;
     unsigned short guest = client->qemu.guest;
+    bool reset = client->qemu.reset;
     char vmid[sizeof(client->qemu.vmid)];
     strncpy(vmid, client->qemu.vmid, sizeof(vmid));
     vmid[sizeof(vmid) - 1] = '\0';
     g_hash_table_remove(vm_clients, &vmid); // frees key, ignore errors
-    VERBOSE_PRINT("%s: executing cleanup (graceful: %d, guest: %d)\n", vmid, graceful, guest);
+    VERBOSE_PRINT(
+        "%s: executing cleanup (graceful: %d, guest: %d, reset: %d)\n", vmid, graceful, guest, reset
+    );
 
     int pid = fork();
     if (pid < 0) {
@@ -406,7 +423,10 @@ static void cleanup_qemu_client(struct Client *client) {
     if (pid == 0) {
         char *script = "/usr/sbin/qm";
 
-        char *args[] = {script, "cleanup", vmid, graceful ? "1" : "0", guest ? "1" : "0", NULL};
+        char *args[] = {
+            script, "cleanup", vmid, graceful ? "1" : "0", guest ? "1" : "0", reset ? "1" : "0",
+            NULL
+        };
 
         execvp(script, args);
         perror("execvp");
diff --git a/src/qmeventd/qmeventd.h b/src/qmeventd/qmeventd.h
index 6a293b84..36f8bdcc 100644
--- a/src/qmeventd/qmeventd.h
+++ b/src/qmeventd/qmeventd.h
@@ -78,6 +78,7 @@ struct Client {
         unsigned short guest;
         bool term_check_queued;
         bool backup;
+        bool reset;
     } qemu;
 
     // only relevant for type=CLIENT_VZDUMP
-- 
2.47.3

[PATCH pve-manager 5/6] ui: qemu: options: add editor for reboot behavior

[Jakob Klocker]

The `reboot` option is now a property string with an added `powercycle`
sub-property, so expose both in the VM options panel: one checkbox to
allow reboots at all, and one to stop and start the VM on a
guest-initated reboot instead of resetting it in place, which makes
pending changes take effect.

Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
---
 www/manager6/Makefile                      |  1 +
 www/manager6/Utils.js                      | 17 +++++++
 www/manager6/form/RebootFeatureSelector.js | 59 ++++++++++++++++++++++
 www/manager6/qemu/Options.js               | 14 +++++
 4 files changed, 91 insertions(+)
 create mode 100644 www/manager6/form/RebootFeatureSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index eb0e9d9c..40f71b8c 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -67,6 +67,7 @@ JSSRC= 							\
 	form/PrivilegesSelector.js			\
 	form/QemuBiosSelector.js			\
 	form/QemuMachineSelector.js			\
+	form/RebootFeatureSelector.js			\
 	form/RecordSearchField.js			\
 	form/SDNControllerSelector.js			\
 	form/SDNZoneSelector.js				\
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 040b5ae0..f10c26d5 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1160,6 +1160,23 @@ Ext.define('PVE.Utils', {
             return Ext.Date.format(new Date(value * 1000), 'l d F Y H:i:s');
         },
 
+        render_reboot: function (value) {
+            let normal = gettext('Reboot normally');
+
+            if (value === undefined || value === '') {
+                return `${Proxmox.Utils.defaultText} (${normal})`;
+            }
+
+            let props = PVE.Parser.parsePropertyString(value, 'enabled');
+            if (!PVE.Parser.parseBoolean(props.enabled, 1)) {
+                return gettext('Shut down instead of rebooting');
+            }
+            if (PVE.Parser.parseBoolean(props.powercycle, 0)) {
+                return gettext('Stop and start the VM on guest reboot');
+            }
+            return normal;
+        },
+
         // render a timestamp or pending
         render_next_event: function (value) {
             if (!value) {
diff --git a/www/manager6/form/RebootFeatureSelector.js b/www/manager6/form/RebootFeatureSelector.js
new file mode 100644
index 00000000..88e93da3
--- /dev/null
+++ b/www/manager6/form/RebootFeatureSelector.js
@@ -0,0 +1,59 @@
+Ext.define('PVE.form.RebootFeatureSelector', {
+    extend: 'Proxmox.panel.InputPanel',
+    alias: ['widget.pveRebootFeatureSelector'],
+
+    viewModel: {},
+
+    items: [
+        {
+            xtype: 'proxmoxcheckbox',
+            boxLabel: gettext('Allow reboot'),
+            name: 'enabled',
+            reference: 'enabled',
+            uncheckedValue: 0,
+        },
+        {
+            xtype: 'proxmoxcheckbox',
+            boxLabel: gettext('Stop and start the VM on guest reboot (applies pending changes)'),
+            name: 'powercycle',
+            uncheckedValue: 0,
+            bind: {
+                disabled: '{!enabled.checked}',
+            },
+            disabled: true,
+        },
+        {
+            xtype: 'displayfield',
+            userCls: 'pmx-hint',
+            value: gettext('The VM is shut down instead of rebooted and stays off.'),
+            bind: {
+                hidden: '{enabled.checked}',
+            },
+        },
+    ],
+
+    onGetValues: function (values) {
+        let enabled = !Ext.isDefined(values.enabled) || String(values.enabled) === '1';
+        let powercycle = String(values.powercycle) === '1';
+
+        // equivalent to the default, so don't write the option at all
+        if (enabled && !powercycle) {
+            return { delete: 'reboot' };
+        }
+
+        let props = { enabled: enabled ? 1 : 0 };
+        if (powercycle) {
+            props.powercycle = 1;
+        }
+
+        return { reboot: PVE.Parser.printPropertyString(props, 'enabled') };
+    },
+
+    setValues: function (values) {
+        let res = PVE.Parser.parsePropertyString(values.reboot, 'enabled');
+        if (!Ext.isDefined(res.enabled)) {
+            res.enabled = 1;
+        }
+        this.callParent([res]);
+    },
+});
diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
index 8a4721a0..4fd5d1bb 100644
--- a/www/manager6/qemu/Options.js
+++ b/www/manager6/qemu/Options.js
@@ -84,6 +84,20 @@ Ext.define('PVE.qemu.Options', {
                           }
                         : undefined,
             },
+            reboot: {
+                header: gettext('Reboot behavior'),
+                defaultValue: '',
+                renderer: PVE.Utils.render_reboot,
+                editor: caps.vms['VM.Config.Options']
+                    ? {
+                          xtype: 'proxmoxWindowEdit',
+                          subject: gettext('Reboot behavior'),
+                          onlineHelp: 'qm_reboot_behavior',
+                          width: 350,
+                          items: { xtype: 'pveRebootFeatureSelector', name: 'reboot' },
+                      }
+                    : undefined,
+            },
             ostype: {
                 header: gettext('OS Type'),
                 editor: caps.vms['VM.Config.Options']
-- 
2.47.3

[PATCH pve-docs 6/6] qm: add reboot behavior information

[Jakob Klocker]

Add information about the newly created `Reboot behavior` option in PVE.

Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
---
 qm.adoc | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/qm.adoc b/qm.adoc
index 96ba68b..e900cb8 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -1540,6 +1540,43 @@ cluster-wide.
 If you require a delay between the host boot and the booting of the first VM,
 see the section on xref:first_guest_boot_delay[Proxmox VE Node Management].
 
+[[qm_reboot_behavior]]
+Reboot Behavior
+~~~~~~~~~~~~~~~
+
+When a guest reboots itself from inside the operating system, {pve} does not
+need to restart the virtual machine to service the request. By default the
+machine is reset in place: the QEMU process keeps running and only the guest
+operating system boots again.
+
+This is fast and transparent to the guest, but it means the VM keeps running
+with everything it was started with - its configuration, its machine version
+and the host resources assigned to it. Configuration changes that are marked as
+*pending* in the *Options* and hardware panels therefore stay pending, and a
+newer `pve-qemu-kvm` package installed in the meantime is not picked up. A
+reboot triggered through the web interface, `qm` or the API is not affected by
+this, as it always stops and starts the virtual machine, unless rebooting is
+disabled entirely.
+
+The `reboot` option has two properties:
+
+`enabled` (default: `1`)::
+Whether the virtual machine may reboot at all. If disabled, the VM is shut down
+instead of rebooted and stays off, no matter whether the reboot was initiated by
+the guest or through {pve}. Note that guests managed by the
+xref:chapter_ha_manager[HA stack] with the requested state `started` are started
+again by the HA manager.
+
+`powercycle` (default: `0`)::
+Stop and start the virtual machine on a reset instead of resetting it in place.
+Because this is a full start, pending configuration changes take effect and the
+VM is launched with the currently installed QEMU version. This applies to resets
+initiated inside the guest as well as to those triggered through the web
+interface, `qm` or the API. This has no effect when reboot is disabled.
+
+----
+# qm set <vmid> --reboot 1,powercycle=1
+----
 
 [[qm_qemu_agent]]
 QEMU Guest Agent
-- 
2.47.3