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 BF5BB1FF0E1 for ; Mon, 13 Jul 2026 16:38:29 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id D6AB221357; Mon, 13 Jul 2026 16:38:28 +0200 (CEST) Date: Mon, 13 Jul 2026 16:37:50 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= Subject: Re: [PATCH qemu-server 1/1] fix #7590: qemu-server: apply timeout to QEMU start fork To: pve-devel@lists.proxmox.com, Samuel Rufinatscha References: <20260625114500.159384-1-s.rufinatscha@proxmox.com> <20260625114500.159384-2-s.rufinatscha@proxmox.com> In-Reply-To: <20260625114500.159384-2-s.rufinatscha@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.17.0 (https://github.com/astroidmail/astroid) Message-Id: <1783952852.13tv1348l8.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1783953458571 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.338 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_LOW -0.7 Sender listed at https://www.dnswl.org/, low 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: 2XWYICZQ5HIZTTM5STLLXHI7IFIWSYGS X-Message-ID-Hash: 2XWYICZQ5HIZTTM5STLLXHI7IFIWSYGS X-MailFrom: f.gruenbichler@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: On June 25, 2026 1:45 pm, Samuel Rufinatscha wrote: > Apply the existing start timeout to the forked QEMU startup path and > clean up the VM scope on timeout. This prevents stuck qmstart tasks > from holding the config lock indefinitely or leaving a partially > started QEMU process running. >=20 > Signed-off-by: Samuel Rufinatscha > Link: https://bugzilla.proxmox.com/show_bug.cgi?id=3D7590 > --- > src/PVE/QemuServer.pm | 28 +++++++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) >=20 > diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm > index 55e9f520..df400376 100644 > --- a/src/PVE/QemuServer.pm > +++ b/src/PVE/QemuServer.pm > @@ -5762,7 +5762,7 @@ sub vm_start_nolock { > }; > =20 > my $run_qemu =3D sub { > - PVE::Tools::run_fork sub { > + my $run_qemu_child =3D sub { > PVE::Systemd::enter_systemd_scope($vmid, "Proxmox VE VM $vmi= d", > %systemd_properties); > =20 > @@ -5791,6 +5791,32 @@ sub vm_start_nolock { > die "QEMU exited with code $exitcode\n"; > } > }; > + > + my (undef, $timed_out) =3D > + PVE::Tools::run_fork_with_timeout($start_timeout || undef, $= run_qemu_child); the timeout here is not the same that is used by the run_command invocation in the sub above.. and it would also need to account for the time it takes to start swtpm and all virtiofsd instances on top of that. > + > + if ($timed_out) { > + eval { > + run_command( > + [ > + '/bin/systemctl', > + 'kill', > + '--kill-whom=3Dall', > + '--signal=3DKILL', > + "$vmid.scope", > + ], > + %silence_std_outs, > + noerr =3D> 1, > + timeout =3D> 10, > + ); > + PVE::Systemd::wait_for_unit_removed("$vmid.scope", 20); > + }; > + warn "failed to clean up timed-out VM start scope - $@" if $= @; > + > + $cleanup_qsd->(); isn't the order here wrong? first we should cleanup any running QSD instances, then kill the scope - after all the QSD instance is running inside the scope.. similarly, the tpmpid killing should also be done here.. and in the regular cleanup path, we do not explicitly kill the scope? I think we want unified error handling here, whether the start explicitly failed, or implicitly failed by timing out.. > + > + die "QEMU start timed out after $start_timeout seconds\n"; > + } > }; > =20 > if ($conf->{hugepages}) { > --=20 > 2.47.3 >=20 >=20 >=20 >=20 >=20 >=20