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 > > The problem is apparently that the migration thread is still active > (migration_shutdown() only asks it to stop the current migration, but > does not wait for it to finish), while the main thread continues to > bdrv_close_all() that will destroy all block drivers. So the two threads > are racing here for the destruction of the migration-related block drivers. > > I was able to bisect the problem and the race has apparently been introduced > by commit c2a189976e211c9ff782 ("migration/block-active: Remove global active > flag"), so reverting it might be an option as well, but waiting for the > migration thread to finish before continuing with the further clean-ups > during shutdown seems less intrusive. > > Note: I used the Claude AI assistant for analyzing the crash, and it > came up with the idea of waiting for the migration thread to finish > in migration_shutdown() before proceeding with the further clean-up, > but the patch itself has been 100% written by myself.
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.
>
> Fixes: c2a189976e ("migration/block-active: Remove global active flag")
> Signed-off-by: Thomas Huth <[email protected]>
> ---
> migration/migration.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index b316ee01ab2..6f4bb6d8438 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -380,6 +380,16 @@ void migration_bh_schedule(QEMUBHFunc *cb, void *opaque)
> qemu_bh_schedule(bh);
> }
>
> +static void migration_thread_join(MigrationState *s)
> +{
> + if (s && s->migration_thread_running) {
> + bql_unlock();
> + qemu_thread_join(&s->thread);
> + s->migration_thread_running = false;
> + bql_lock();
> + }
> +}
> +
> void migration_shutdown(void)
> {
> /*
> @@ -393,6 +403,13 @@ void migration_shutdown(void)
> * stop the migration using this structure
> */
> migration_cancel();
> + /*
> + * Wait for migration thread to finish to prevent a possible race where
> + * the migration thread is still running and accessing host block drivers
> + * while the main cleanup proceeds to remove them in bdrv_close_all()
> + * later.
> + */
> + migration_thread_join(migrate_get_current());
> object_unref(OBJECT(current_migration));
>
> /*
> @@ -1499,12 +1516,7 @@ static void migration_cleanup(MigrationState *s)
>
> close_return_path_on_source(s);
>
> - if (s->migration_thread_running) {
> - bql_unlock();
> - qemu_thread_join(&s->thread);
> - s->migration_thread_running = false;
> - bql_lock();
> - }
> + migration_thread_join(s);
>
> WITH_QEMU_LOCK_GUARD(&s->qemu_file_lock) {
> /*
> --
> 2.52.0
>
signature.asc
Description: PGP signature
