Re: [PATCH v2 0/2] Create menus in iothread

[Akihiko Odaki]

On Tue, Mar 8, 2022 at 12:32 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 3/7/22 14:49, Akihiko Odaki wrote:
> > ui/cocoa: Create menus in iothread
> >
> > Commit 0439c5a4623d674efa0c72abd62ca6e98bb7cf87 introduced an
> > assertion that blk_all_next is called in the main thread. The function
> > is called in the following chain:
> > - blk_all_next
> > - qmp_query_block
> > - addRemovableDevicesMenuItems
> > - main
> >
> > This change moves the menu creation to the iothread. This also changes
> > the menu creation procedure to construct the entire menu tree before
> > setting to NSApp, which is necessary because a menu set once cannot be
> > modified if NSApp is already running.
>
> I wonder if you actually need the iothread/secondary thread separation
> during initialization.  It's needed to run the "secondary" main loop,
> but until that point nobody should care what thread things run on.
> cocoa_display_init() is close enough to the end of qemu_init() that I
> think you can just do:
>
>    main()
>      qemu_init()
>        /* takes iothread lock */
>        cocoa_display_init()
>          /* just save a few values from opts */
>      ... create menus ...
>      [NSApp run]
>        applicationDidFinishLaunching:
>          /* do what was in cocoa_display_init() */
>          qemu_unlock_mutex_iothread();
>          qemu_thread_create(call_qemu_main_loop)
>                                                     call_qemu_main_loop()
>                                                       qemu_main_loop()
>
> This might even obsolete the allow_events hack, because now the main
> thread has the iothread lock until applicationDidFinishLaunching:.

allow_events was necessary not because of the separation of the
thread, but because cocoa_display_init waits the main thread for
finishing the initialization. It can be simply removed if it doesn't
wait for app_started_sem.

Regards,
Akihiko Odaki

>
> Paolo
>
> > v2: Separate a change moving create_initial_menus (Peter Maydell)
> >
> > Akihiko Odaki (2):
> >    ui/cocoa: Move create_initial_menus
> >    ui/cocoa: Create menus in iothread
> >
> >   ui/cocoa.m | 209 +++++++++++++++++++++++++----------------------------
> >   1 file changed, 98 insertions(+), 111 deletions(-)
> >
>