Thomas Huth <[email protected]> writes: > On 08/12/2025 16.26, Fabiano Rosas wrote: >> Stefan Hajnoczi <[email protected]> writes: >> >>> On Mon, Dec 08, 2025 at 02:51:01PM +0100, Thomas Huth wrote: >>>> From: Thomas Huth <[email protected]> >>>> >>>> When shutting down a guest that is currently in progress of being >>>> migrated, there is a chance that QEMU might crash during bdrv_delete(). >>>> The backtrace looks like this: >>>> >>>> Thread 74 "mig/src/main" received signal SIGSEGV, Segmentation fault. >>>> >>>> [Switching to Thread 0x3f7de7fc8c0 (LWP 2161436)] >>>> 0x000002aa00664012 in bdrv_delete (bs=0x2aa00f875c0) at >>>> ../../devel/qemu/block.c:5560 >>>> 5560 QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list); >>>> (gdb) bt >>>> #0 0x000002aa00664012 in bdrv_delete (bs=0x2aa00f875c0) at >>>> ../../devel/qemu/block.c:5560 >>>> #1 bdrv_unref (bs=0x2aa00f875c0) at ../../devel/qemu/block.c:7170 >>>> Backtrace stopped: Cannot access memory at address 0x3f7de7f83e0 >>>> >> >> How does the migration thread reaches here? Is this from >> migration_block_inactivate()? > > Unfortunately, gdb was not very helpful here (claiming that it cannot access > the memory and stack anymore), so I had to do some printf debugging. This is > what seems to happen: > > Main thread: qemu_cleanup() calls migration_shutdown() --> > migration_cancel() which signals the migration thread to cancel the migration. > > Migration thread: migration_thread() got kicked out the loop and calls > migration_iteration_finish(), which tries to get the BQL via bql_lock() but > that is currently held by another thread, so the migration thread is blocked > here. > > Main thread: qemu_cleanup() advances to bdrv_close_all() that uses > blockdev_close_all_bdrv_states() to unref all BDS. The BDS with the name > 'libvirt-1-storage' gets deleted via bdrv_delete() that way. >
Has qmp_blockdev_del() ever been called to remove the BDS from the monitor_bdrv_states list? Otherwise your debugging seems to indicate blockdev_close_all_bdrv_states() is dropping the last reference to bs, but it's still accessible from bdrv_next() via bdrv_next_monitor_owned(). > Migration thread: Later, migration_iteration_finish() finally gets the BQL, > and calls the migration_block_activate() function in the > MIGRATION_STATUS_CANCELLING case statement. This calls bdrv_activate_all(). > bdrv_activate_all() gets a pointer to that 'libvirt-1-storage' BDS again > from bdrv_first(), and during the bdrv_next() that BDS gets unref'ed again > which is causing the crash. > > ==> Why is bdrv_first() still providing a BDS that have been deleted by > other threads earlier? > >>> It sounds like the migration thread does not hold block graph refcounts >>> and assumes the BlockDriverStates it uses have a long enough lifetime. >>> >>> I don't know the migration code well enough to say whether joining in >>> migration_shutdown() is okay. Another option would be expicitly holding >>> the necessary refcounts in the migration thread. >> >> I agree. In principle and also because shuffling the joining around >> feels like something that's prone to introduce other bugs. > > I'm a little bit lost here right now ... Can you suggest a place where we > would need to increase the refcounts in the migration thread? > > Thomas
