Re: [PATCH docs/guest-common/manager/qemu-server v2 0/8] fix #7213: add `powercycle` reboot behavior

[David Riley <d.riley@proxmox.com>]

Thanks for the patch series! This series seems to be coming along nicely. I did
however notice a compatibility issue with PDM and a potentially dangerous
edge case that I want to mention here.

PDM compatibility:
Currently, there doesn't seem to be a companion patch for PDM. Because this
series changes the reboot option from a boolean to a property string, viewing
a VM configuration in PDM with this new property set throws the following error:
"error:invalid value: string "1,powercycle=1", expected a boolean-like value"

Pending config edge case:
This introduces a dangerous edge case for separated teams (e.g., Guest Admins
vs. Sys Admins). Because "-no-reboot" terminates the QEMU process immediately,
PVE must perform a cold start.

If a PVE Admin previously staged an invalid pending configuration (e.g.,
overprovisioning vCPUs), a simple guest-initiated reboot will permanently kill the
VM until a Sys Admin intervenes to fix the config.
"TASK ERROR: MAX 4 vcpus allowed per VM on this node"

While this is technically 'user error' by whoever staged the bad config, the
powercycle feature seems to amplify the blast radius by turning a normally safe,
action, like a guest OS reboot into a fatal state change.

I think a hint in the WebUI and Documentation mentioning this would be good.

I also left some additional comments on the patches themselves.

On 10/22/26 2:51 PM, Jakob Klocker wrote:
> 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.
>
> changes from v1 to v2 :
>   - fix pending delete for options with value 0
>   - warn when restarting a VM with reboot disabled
>   - reboot on `qm reset` even if reboot is disabled, feels more
>     intuitive
>   - style fixes according to proxmox style guide (thanks @david)
>
>
> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7213
>
>
> pve-guest-common:
>
> Jakob Klocker (1):
>    AbstractConfig: fix pending delete for options with value 0
>
>   src/PVE/AbstractConfig.pm | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
>
> pve-docs:
>
> Jakob Klocker (1):
>    qm: add reboot behavior information
>
>   qm.adoc | 40 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 40 insertions(+)
>
>
> qemu-server:
>
> Jakob Klocker (5):
>    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`
>    vm_reboot: warn when rebooting a VM with `reboot` disabled
>
>   src/PVE/CLI/qm.pm       | 25 ++++++++++++++++++---
>   src/PVE/QemuServer.pm   | 50 ++++++++++++++++++++++++++++++++++++-----
>   src/qmeventd/qmeventd.c | 36 ++++++++++++++++++++++-------
>   src/qmeventd/qmeventd.h |  1 +
>   4 files changed, 96 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
>
>
> Summary over all repositories:
>    10 files changed, 228 insertions(+), 17 deletions(-)
>