On Tue, Sep 05, 2023 at 04:50:02PM +0200, Kevin Wolf wrote: > virtio_load() as a whole should run in coroutine context because it > reads from the migration stream and we don't want this to block.
Is that "should" a "must" or a "can"? If it's a "must" then virtio_load() needs assert(qemu_in_coroutine()). But the previous patch mentioned that loadvm for snapshots calls it outside coroutine context. So maybe it's a "can"? > > However, it calls virtio_set_features_nocheck() and devices don't > expect their .set_features callback to run in a coroutine and therefore > call functions that may not be called in coroutine context. To fix this, > drop out of coroutine context for calling virtio_set_features_nocheck(). > > Without this fix, the following crash was reported: > > #0 __pthread_kill_implementation (threadid=<optimized out>, > signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44 > #1 0x00007efc738c05d3 in __pthread_kill_internal (signo=6, > threadid=<optimized out>) at pthread_kill.c:78 > #2 0x00007efc73873d26 in __GI_raise (sig=sig@entry=6) at > ../sysdeps/posix/raise.c:26 > #3 0x00007efc738477f3 in __GI_abort () at abort.c:79 > #4 0x00007efc7384771b in __assert_fail_base (fmt=0x7efc739dbcb8 "", > assertion=assertion@entry=0x560aebfbf5cf "!qemu_in_coroutine()", > file=file@entry=0x560aebfcd2d4 "../block/graph-lock.c", > line=line@entry=275, function=function@entry=0x560aebfcd34d "void > bdrv_graph_rdlock_main_loop(void)") at assert.c:92 > #5 0x00007efc7386ccc6 in __assert_fail (assertion=0x560aebfbf5cf > "!qemu_in_coroutine()", file=0x560aebfcd2d4 "../block/graph-lock.c", line=275, > function=0x560aebfcd34d "void bdrv_graph_rdlock_main_loop(void)") at > assert.c:101 > #6 0x0000560aebcd8dd6 in bdrv_register_buf () > #7 0x0000560aeb97ed97 in ram_block_added.llvm () > #8 0x0000560aebb8303f in ram_block_add.llvm () > #9 0x0000560aebb834fa in qemu_ram_alloc_internal.llvm () > #10 0x0000560aebb2ac98 in vfio_region_mmap () > #11 0x0000560aebb3ea0f in vfio_bars_register () > #12 0x0000560aebb3c628 in vfio_realize () > #13 0x0000560aeb90f0c2 in pci_qdev_realize () > #14 0x0000560aebc40305 in device_set_realized () > #15 0x0000560aebc48e07 in property_set_bool.llvm () > #16 0x0000560aebc46582 in object_property_set () > #17 0x0000560aebc4cd58 in object_property_set_qobject () > #18 0x0000560aebc46ba7 in object_property_set_bool () > #19 0x0000560aeb98b3ca in qdev_device_add_from_qdict () > #20 0x0000560aebb1fbaf in virtio_net_set_features () > #21 0x0000560aebb46b51 in virtio_set_features_nocheck () > #22 0x0000560aebb47107 in virtio_load () > #23 0x0000560aeb9ae7ce in vmstate_load_state () > #24 0x0000560aeb9d2ee9 in qemu_loadvm_state_main () > #25 0x0000560aeb9d45e1 in qemu_loadvm_state () > #26 0x0000560aeb9bc32c in process_incoming_migration_co.llvm () > #27 0x0000560aebeace56 in coroutine_trampoline.llvm () > > Cc: [email protected] > Buglink: https://issues.redhat.com/browse/RHEL-832 > Signed-off-by: Kevin Wolf <[email protected]> > --- > hw/virtio/virtio.c | 45 ++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 40 insertions(+), 5 deletions(-) Reviewed-by: Stefan Hajnoczi <[email protected]>
signature.asc
Description: PGP signature
