public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>, pve-devel@lists.proxmox.com
Subject: Re: [PATCH qemu-server v2] fix #7119: qm cleanup: wait for process exiting for up to 30 seconds
Date: Fri, 13 Feb 2026 12:40:17 +0100	[thread overview]
Message-ID: <1770982503.z90o85edfu.astroid@yuna.none> (raw)
In-Reply-To: <20260210111612.2017883-1-d.csapak@proxmox.com>

On February 10, 2026 12:15 pm, Dominik Csapak wrote:
> When qmeventd detects a vm exiting, it starts 'qm cleanup' to cleanup
> files, executing hookscripts, etc.
> 
> Since the vm process exits is sometimes not instant, wait up to 30
> seconds here to start the cleanup process instead of immediately
> aborting if the pid still exits. This prevented executing the hookscript
> on the 'post-stop' phase.
> 
> This can be easily reproduced by e.g. passing through a usb device,
> which delays the qemu process exit for a few seconds.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v1:
> * use correct while condition (time() is always >= $starttime)
> 
> original comment:
> 
> The 30 second timeout was arbitrarily chosen, but we could probably
> start with something smaller, like 10 seconds? Could be adapted on
> applying though.
> 
> In my (short) tests the usb passthrough part only adds a single second,
> but i can imagine different devices on other systems could block it for
> much longer.
> 
>  src/PVE/CLI/qm.pm | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/CLI/qm.pm b/src/PVE/CLI/qm.pm
> index bdae9641..16875ed2 100755
> --- a/src/PVE/CLI/qm.pm
> +++ b/src/PVE/CLI/qm.pm
> @@ -1101,8 +1101,19 @@ __PACKAGE__->register_method({
>              60,
>              sub {
>                  my $conf = PVE::QemuConfig->load_config($vmid);
> +
> +                # wait for some timeout until vm process exits, since this might not be instant
> +                my $timeout = 30;
> +                my $starttime = time();
>                  my $pid = PVE::QemuServer::check_running($vmid);
> -                die "vm still running\n" if $pid;
> +                warn "vm still running - waiting up to $timeout seconds\n" if $pid;
> +
> +                while ($pid && (time() - $starttime) < $timeout) {
> +                    sleep(1);
> +                    $pid = PVE::QemuServer::check_running($vmid);

nit: this helper is deprecated - and this call here is only running in
the context of "guest is local", we already obtained the lock and loaded
the config, so we know that invariant holds, so this new code (and the
old line above) can just use
PVE::QemuServer::Helpers::vm_running_locally instead..

> +                }
> +
> +                die "vm still running - aborting cleanup\n" if $pid;
>  
>                  # Rollback already does cleanup when preparing and afterwards temporarily drops the
>                  # lock on the configuration file to rollback the volumes. Deactivating volumes here
> -- 
> 2.47.3
> 
> 
> 
> 
> 
> 




  parent reply	other threads:[~2026-02-13 11:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-10 11:15 Dominik Csapak
2026-02-12 20:33 ` Benjamin McGuire
2026-02-13 11:40 ` Fabian Grünbichler [this message]
2026-02-13 12:14 ` Fiona Ebner
2026-02-13 12:20   ` Fabian Grünbichler
2026-02-13 13:16     ` Fiona Ebner
2026-02-16  8:42       ` Fabian Grünbichler
2026-02-16  9:15         ` Fiona Ebner
2026-02-19 10:15           ` Dominik Csapak
2026-02-19 13:27             ` Fiona Ebner
2026-02-20  9:36               ` Dominik Csapak
2026-02-20 14:30                 ` Fiona Ebner
2026-02-20 14:51                   ` Dominik Csapak
2026-02-13 12:22   ` Dominik Csapak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1770982503.z90o85edfu.astroid@yuna.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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