public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH qemu-server 0/1] fix #7590: qemu-server: apply timeout to QEMU start fork
@ 2026-06-25 11:44 Samuel Rufinatscha
  2026-06-25 11:45 ` [PATCH qemu-server 1/1] " Samuel Rufinatscha
  0 siblings, 1 reply; 3+ messages in thread
From: Samuel Rufinatscha @ 2026-06-25 11:44 UTC (permalink / raw)
  To: pve-devel

This series makes the forked QEMU start path use the existing start
timeout.

If the timeout expires, it terminates processes in the VM scope and
cleans up QSD before reporting the start failure. This prevents a stuck
qmstart task from holding the VM config lock indefinitely or leaving a
partially started QEMU process running.

Tested with QEMU blocked on a FIFO (see the reproducer at [0]).
qmstart timed out and released the lock, leaving no active scope or
QEMU process. Normal VM startup still works.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=7590

Samuel Rufinatscha (1):
  fix #7590: qemu-server: apply timeout to QEMU start fork

 src/PVE/QemuServer.pm | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

-- 
2.47.3





^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH qemu-server 1/1] fix #7590: qemu-server: apply timeout to QEMU start fork
  2026-06-25 11:44 [PATCH qemu-server 0/1] fix #7590: qemu-server: apply timeout to QEMU start fork Samuel Rufinatscha
@ 2026-06-25 11:45 ` Samuel Rufinatscha
  2026-07-13 14:37   ` Fabian Grünbichler
  0 siblings, 1 reply; 3+ messages in thread
From: Samuel Rufinatscha @ 2026-06-25 11:45 UTC (permalink / raw)
  To: pve-devel

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.

Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7590
---
 src/PVE/QemuServer.pm | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

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 {
     };
 
     my $run_qemu = sub {
-        PVE::Tools::run_fork sub {
+        my $run_qemu_child = sub {
             PVE::Systemd::enter_systemd_scope($vmid, "Proxmox VE VM $vmid",
                 %systemd_properties);
 
@@ -5791,6 +5791,32 @@ sub vm_start_nolock {
                 die "QEMU exited with code $exitcode\n";
             }
         };
+
+        my (undef, $timed_out) =
+            PVE::Tools::run_fork_with_timeout($start_timeout || undef, $run_qemu_child);
+
+        if ($timed_out) {
+            eval {
+                run_command(
+                    [
+                        '/bin/systemctl',
+                        'kill',
+                        '--kill-whom=all',
+                        '--signal=KILL',
+                        "$vmid.scope",
+                    ],
+                    %silence_std_outs,
+                    noerr => 1,
+                    timeout => 10,
+                );
+                PVE::Systemd::wait_for_unit_removed("$vmid.scope", 20);
+            };
+            warn "failed to clean up timed-out VM start scope - $@" if $@;
+
+            $cleanup_qsd->();
+
+            die "QEMU start timed out after $start_timeout seconds\n";
+        }
     };
 
     if ($conf->{hugepages}) {
-- 
2.47.3





^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH qemu-server 1/1] fix #7590: qemu-server: apply timeout to QEMU start fork
  2026-06-25 11:45 ` [PATCH qemu-server 1/1] " Samuel Rufinatscha
@ 2026-07-13 14:37   ` Fabian Grünbichler
  0 siblings, 0 replies; 3+ messages in thread
From: Fabian Grünbichler @ 2026-07-13 14:37 UTC (permalink / raw)
  To: pve-devel, Samuel Rufinatscha

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.
> 
> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=7590
> ---
>  src/PVE/QemuServer.pm | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> 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 {
>      };
>  
>      my $run_qemu = sub {
> -        PVE::Tools::run_fork sub {
> +        my $run_qemu_child = sub {
>              PVE::Systemd::enter_systemd_scope($vmid, "Proxmox VE VM $vmid",
>                  %systemd_properties);
>  
> @@ -5791,6 +5791,32 @@ sub vm_start_nolock {
>                  die "QEMU exited with code $exitcode\n";
>              }
>          };
> +
> +        my (undef, $timed_out) =
> +            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=all',
> +                        '--signal=KILL',
> +                        "$vmid.scope",
> +                    ],
> +                    %silence_std_outs,
> +                    noerr => 1,
> +                    timeout => 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";
> +        }
>      };
>  
>      if ($conf->{hugepages}) {
> -- 
> 2.47.3
> 
> 
> 
> 
> 
> 




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-13 14:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-25 11:44 [PATCH qemu-server 0/1] fix #7590: qemu-server: apply timeout to QEMU start fork Samuel Rufinatscha
2026-06-25 11:45 ` [PATCH qemu-server 1/1] " Samuel Rufinatscha
2026-07-13 14:37   ` Fabian Grünbichler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal