Re: [PATCH qemu-server v3 3/3] fix #7119: qm cleanup: wait for process exiting for up to 30 seconds

[Fiona Ebner <f.ebner@proxmox.com>]

Am 26.02.26 um 3:08 PM schrieb Dominik Csapak:
> When qmeventd detects a vm exiting, it starts 'qm cleanup'.
> 
> 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 when either
> * the cleanup mechanism is still the old one
> * the guest was powered down from inside, not via the API
> 
> This can be reproduced by e.g. passing through a usb device, which
> delays the qemu process exit for a few seconds (for most devices).
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/CLI/qm.pm | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/CLI/qm.pm b/src/PVE/CLI/qm.pm
> index 6aff5b7a..ee3ccedd 100755
> --- a/src/PVE/CLI/qm.pm
> +++ b/src/PVE/CLI/qm.pm
> @@ -1101,7 +1101,7 @@ __PACKAGE__->register_method({
>              60,
>              sub {
>                  my $conf = PVE::QemuConfig->load_config($vmid);
> -                my $pid = PVE::QemuServer::check_running($vmid);
> +                my $pid = PVE::QemuServer::Helpers::vm_running_locally($vmid);
>  
>                  # With a stop mode backup, we might run here into a running vm with a backup
>                  # lock, but this already did the cleanup and is an expected state, so abort
> @@ -1109,7 +1109,19 @@ __PACKAGE__->register_method({
>                  die "skipping cleanup - 'backup' lock is present and vm is running again\n"
>                      if $pid && $clean && $conf->{lock} && $conf->{lock} eq 'backup';
>  
> -                die "vm still running\n" if $pid;
> +                # wait for some time until the QEMU process exits after the QMP
> +                # 'SHUTDOWN' event, since this might not be instant
> +                my $timeout = 30;
> +                my $starttime = time();
> +                warn "QEMU process $pid for VM $vmid still running (or newly started)\n"
> +                    if $pid;

Should we maybe warn once after 10? seconds rather than instantly? Not
sure if we should warn at all if it's expected to take a while in some
cases. If we time out, we still get the error below.

> +
> +                while ($pid && (time() - $starttime) < $timeout) {
> +                    sleep(1);
> +                    $pid = PVE::QemuServer::check_running($vmid);

Should also use the non-deprecated helper.

> +                }
> +
> +                die "vm still running after timeout - 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