Re: [pve-devel] [PATCH qemu] avoid segfault when aborting snapshot

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

Am 02.08.22 um 11:52 schrieb Mira Limbeck:
> On 7/26/22 14:25, Fiona Ebner wrote:
>> Reported in the community forum[0].
>>
>> For 6.1.0, there were a few changes to the coroutine-sleep API, but
>> the adaptations in f376b2b ("update and rebase to QEMU v6.1.0") made
>> a mistake.
>>
>> Currently, target_close_wait is NULL when passed to
>> qemu_co_sleep_ns_wakeable(), which further passes it to
>> qemu_co_sleep(), but there, it is dereferenced when trying to access
>> the 'to_wake' member:
>>
>>> Thread 1 "kvm" received signal SIGSEGV, Segmentation fault.
>>> qemu_co_sleep (w=0x0) at ../util/qemu-coroutine-sleep.c:57
>> To fix it, create a proper struct and pass its address instead. Also
>> call qemu_co_sleep_wake unconditionally, because the NULL check (for
>> the 'to_wake' member) is done inside the function itself.
>>
>> This patch is based on what the QEMU commits introducing the changes
>> to the coroutine-sleep API did to the callers in QEMU:
>> eaee072085 ("coroutine-sleep: allow qemu_co_sleep_wake that wakes
>> nothing")
>> 29a6ea24eb ("coroutine-sleep: replace QemuCoSleepState pointer with
>> struct in the API")
>>
>> [0]: https://forum.proxmox.com/threads/112130/
>>
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
> 
> Tested-by: Mira Limbeck <m.limbeck@proxmox.com>
> 
> 
> Found a strange behavior when aborting the snapshot. It no longer
> crashes, but trying to snapshot the VM again leads to instant failure.
> 
> After the failed snapshot, the next one works again. So some state
> doesn't seem to be cleaned up the first time.

Thanks for testing! This behavior was present even before 6.1.0, so not
related to the patch, but I wasn't entirely sure if it'd be fine to just
set the status to SAVE_STATE_DONE in qmp_savevm_end() after abort.

I'll take another look and send a follow-up. It's probably fine in case
the state file has been closed successfully, but I'll need to check the
users in qemu-server and test it.