From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id F417F1FF0B3 for ; Wed, 11 Nov 2026 17:21:56 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 1473D2164F; Wed, 11 Nov 2026 17:21:53 +0100 (CET) Message-ID: Date: Wed, 11 Nov 2026 17:21:48 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: David Riley Subject: Re: [PATCH qemu-server v2 4/8] fix #7213: config: add `powercycle` sub-property to `reboot` To: Jakob Klocker , pve-devel@lists.proxmox.com References: <20261022125111.236538-1-j.klocker@proxmox.com> <20261022125111.236538-5-j.klocker@proxmox.com> Content-Language: en-US In-Reply-To: <20261022125111.236538-5-j.klocker@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1794414108846 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.619 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: OHGPBKSZUGLTPZCM5W3I3TTDSJNNB4NE X-Message-ID-Hash: OHGPBKSZUGLTPZCM5W3I3TTDSJNNB4NE X-MailFrom: d.riley@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: see comment inline. On 10/22/26 2:52 PM, Jakob Klocker wrote: > 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 > --- > 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 1efeeb66..011d175f 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, whether the reboot was requested from inside the guest or" > + . " through the API. An explicit reset through the API still restarts the VM.", > + }, > + 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. Ignored" > + . " 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, > @@ -1685,6 +1706,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 // {}; 'parse_reboot' returns the default { enabled => 1 } for empty inputs, but if 'parse_property_string' fails, it returns an empty hash {}. Because of this, the callers have to defensively coalesce the value later on '$reboot->{enabled} // 1' in patches 3, 4, 5 and 7. It might be better to guarantee the default values here on eval failure as well. That way, we keep the default logic contained entirely within this helper, and can safely drop the '// 1' checks at all the call sites. > +} > + > sub qemu_created_version_fixups { > my ($conf, $forcemachine, $kvmver) = @_; > > @@ -3231,6 +3262,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; > @@ -3414,8 +3447,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);