Re: [PATCH v2] migration/colo: Fix bdrv_graph_rdlock_main_loop: Assertion `!qemu_in_coroutine()' failed.

[Michael Tokarev]

17.04.2024 05:56, Li Zhijian via wrote:
> bdrv_activate_all() should not be called from the coroutine context, move
> it to the QEMU thread colo_process_incoming_thread() with the bql_lock
> protected.
>
> The backtrace is as follows:
>   #4  0x0000561af7948362 in bdrv_graph_rdlock_main_loop () at 
> ../block/graph-lock.c:260
>   #5  0x0000561af7907a68 in graph_lockable_auto_lock_mainloop 
> (x=0x7fd29810be7b) at /patch/to/qemu/include/block/graph-lock.h:259
>   #6  0x0000561af79167d1 in bdrv_activate_all (errp=0x7fd29810bed0) at 
> ../block.c:6906
>   #7  0x0000561af762b4af in colo_incoming_co () at ../migration/colo.c:935
>   #8  0x0000561af7607e57 in process_incoming_migration_co (opaque=0x0) at 
> ../migration/migration.c:793
>   #9  0x0000561af7adbeeb in coroutine_trampoline (i0=-106876144, i1=22042) at 
> ../util/coroutine-ucontext.c:175
>   #10 0x00007fd2a5cf21c0 in  () at /lib64/libc.so.6
>
> CC: Fabiano Rosas <farosas@suse.de>
> Closes: https://gitlab.com/qemu-project/qemu/-/issues/2277
> Fixes: 2b3912f135 ("block: Mark bdrv_first_blk() and bdrv_is_root_node() 
> GRAPH_RDLOCK")
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>

Commit 2b3912f135 is in 8.2 (v8.1.0-1575-g2b3912f135).  Is this fix supposed to 
go
to stable-8.2 series?  The prob here is that in 8.2, there's no bql_lock/unlock.
I guess it should use qemu_mutex_lock_iothread() instead, for before
195801d700c008 "system/cpus: rename qemu_mutex_lock_iothread() to bql_lock()".

Thanks,

/mjt

> diff --git a/migration/colo.c b/migration/colo.c
> index 84632a603e..5600a43d78 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -835,6 +835,16 @@ static void *colo_process_incoming_thread(void *opaque)
>           return NULL;
>       }
>   
> +    /* Make sure all file formats throw away their mutable metadata */
> +    bql_lock();
> +    bdrv_activate_all(&local_err);
> +    if (local_err) {
> +        bql_unlock();
> +        error_report_err(local_err);
> +        return NULL;
> +    }
> +    bql_unlock();
> +
>       failover_init_state();
>   
>       mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
> @@ -922,7 +932,6 @@ out:
>   int coroutine_fn colo_incoming_co(void)
>   {
>       MigrationIncomingState *mis = migration_incoming_get_current();
> -    Error *local_err = NULL;
>       QemuThread th;
>   
>       assert(bql_locked());
> @@ -931,13 +940,6 @@ int coroutine_fn colo_incoming_co(void)
>           return 0;
>       }
>   
> -    /* Make sure all file formats throw away their mutable metadata */
> -    bdrv_activate_all(&local_err);
> -    if (local_err) {
> -        error_report_err(local_err);
> -        return -EINVAL;
> -    }
> -
>       qemu_thread_create(&th, "COLO incoming", colo_process_incoming_thread,
>                          mis, QEMU_THREAD_JOINABLE);
>   

--
GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
New key: rsa4096/61AD3D98ECDF2C8E  9D8B E14E 3F2A 9DD7 9199  28F1 61AD 3D98 
ECDF 2C8E
Old key: rsa2048/457CE0A0804465C5  6EE1 95D1 886E 8FFB 810D  4324 457C E0A0 
8044 65C5
Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt