Re: [PATCH-for-7.0] qemu/main-loop: Disable block backend global state assertion on Darwin

["Philippe Mathieu-Daudé" <philippe.mathieu.daude@gmail.com>]

On 21/3/22 23:08, Akihiko Odaki wrote:
> On 2022/03/21 23:55, Philippe Mathieu-Daudé wrote:
>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> Since commit 0439c5a462 ("block/block-backend.c: assertions for
>> block-backend") QEMU crashes on Darwin hosts, example on macOS:
>>
>>    $ qemu-system-i386
>>    Assertion failed: (qemu_in_main_thread()), function blk_all_next, 
>> file block-backend.c, line 552.
>>    Abort trap: 6
>>
>> Looking with lldb:
>>
>>    Assertion failed: (qemu_in_main_thread()), function blk_all_next, 
>> file block-backend.c, line 552.
>>    Process 76914 stopped
>>    * thread #1, queue = 'com.apple.main-thread', stop reason = hit 
>> program assert
>>       frame #4: 0x000000010057c2d4 qemu-system-i386`blk_all_next.cold.1
>>    at block-backend.c:552:5 [opt]
>>        549    */
>>        550   BlockBackend *blk_all_next(BlockBackend *blk)
>>        551   {
>>    --> 552       GLOBAL_STATE_CODE();
>>        553       return blk ? QTAILQ_NEXT(blk, link)
>>        554                  : QTAILQ_FIRST(&block_backends);
>>        555   }
>>    Target 1: (qemu-system-i386) stopped.
>>
>>    (lldb) bt
>>    * thread #1, queue = 'com.apple.main-thread', stop reason = hit 
>> program assert
>>       frame #0: 0x00000001908c99b8 
>> libsystem_kernel.dylib`__pthread_kill + 8
>>       frame #1: 0x00000001908fceb0 
>> libsystem_pthread.dylib`pthread_kill + 288
>>       frame #2: 0x000000019083a314 libsystem_c.dylib`abort + 164
>>       frame #3: 0x000000019083972c libsystem_c.dylib`__assert_rtn + 300
>>     * frame #4: 0x000000010057c2d4 
>> qemu-system-i386`blk_all_next.cold.1 at block-backend.c:552:5 [opt]
>>       frame #5: 0x00000001003c00b4 
>> qemu-system-i386`blk_all_next(blk=<unavailable>) at 
>> block-backend.c:552:5 [opt]
>>       frame #6: 0x00000001003d8f04 
>> qemu-system-i386`qmp_query_block(errp=0x0000000000000000) at 
>> qapi.c:591:16 [opt]
>>       frame #7: 0x000000010003ab0c qemu-system-i386`main [inlined] 
>> addRemovableDevicesMenuItems at cocoa.m:1756:21 [opt]
>>       frame #8: 0x000000010003ab04 
>> qemu-system-i386`main(argc=<unavailable>, argv=<unavailable>) at 
>> cocoa.m:1980:5 [opt]
>>       frame #9: 0x00000001012690f4 dyld`start + 520
>>
>> As we are in passed release 7.0 hard freeze, disable the block
>> backend assertion which, while being valuable during development,
>> is not helpful to users. We'll restore this assertion immediately
>> once 7.0 is released and work on a fix.
>>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Cc: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> Suggested-by: Akihiko Odaki <akihiko.odaki@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   include/qemu/main-loop.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
>> index 7a4d6a0920..c27968ce33 100644
>> --- a/include/qemu/main-loop.h
>> +++ b/include/qemu/main-loop.h
>> @@ -270,10 +270,14 @@ bool qemu_mutex_iothread_locked(void);
>>   bool qemu_in_main_thread(void);
>>   /* Mark and check that the function is part of the global state API. */
>> +#ifdef CONFIG_DARWIN
> 
> You may use CONFIG_COCOA instead. The assertion can still do its job on 
> Darwin if ui/cocoa is not in use.

Yeah better.

> Also, some code comment is nice to have since the intention is rather 
> unclear from the code even though this is temporary and few people would 
> stumble upon it.

Indeed, I thought about that during the night ;)

> Regards,
> Akihiko Odaki
> 
>> +#define GLOBAL_STATE_CODE()
>> +#else
>>   #define GLOBAL_STATE_CODE()                                         \
>>       do {                                                            \
>>           assert(qemu_in_main_thread());                              \
>>       } while (0)
>> +#endif /* CONFIG_DARWIN */
>>   /* Mark and check that the function is part of the I/O API. */
>>   #define IO_CODE()                                                   \
>