| Message ID | 20220307134946.61407-1-akihiko.odaki@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Create menus in iothread | expand |
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:. 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(-) >
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(-) > > >
On 7/3/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. > > 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 Patch #2 doesn't apply anymore, can you respin?