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

[Dominik Csapak <d.csapak@proxmox.com>]

On 5/13/26 5:14 PM, Fiona Ebner wrote:
> 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.
> 

ofc i can drop the warning, but IMO getting a warning that an operation 
might take longer is also helpful?

maybe a different wording like:

'QEMU process [..] (or newly started), waiting up to $timeout seconds\n'

?

logging only after some feels wrong since the first 10 seconds i don't 
know what's going on...

>> +
>> +                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
>