On Wed, 13 Nov 2024 at 19:36, Paolo Bonzini <pbonz...@redhat.com> wrote:
> On Wed, Nov 13, 2024 at 7:16 PM BALATON Zoltan <bala...@eik.bme.hu> wrote:
> > > int main(int argc, char **argv)
> > > {
> > > + QemuThread main_loop_thread;
> > > +
> > > qemu_init(argc, argv);
> > > - return qemu_main();
> > > + if (qemu_main) {
> > > + qemu_thread_create(&main_loop_thread, "qemu_main",
> > > + call_qemu_default_main, NULL,
> QEMU_THREAD_DETACHED);
> > > + bql_unlock();
> > > + return qemu_main();
> > > + } else {
> > > + qemu_default_main();
> >
> > I think you need 'return qemu_default_main()' here but I'm a bit confused
> > by all this wrapping of qemu_default_main in call_qemu_default_main. I
> see
> > that may be needed because qemu_thread_create takes a different function
> > but now that qemu_default main is static and not replaced externally
> could
> > that be changed to the right type and avoid this confusion and simplify
> > this a bit?
>
> Note that qemu_default_main() expects the BQL to be locked, whereas
> qemu_main() and call_qemu_default_main() do not (because they run in a
> separate thread).
>
> But you're right, we could push bql_lock()/bql_unlock() into
> qemu_default_main(), and do
>
> bql_unlock();
> if (qemu_main) {
> qemu_thread_create(&main_loop_thread, "qemu_main",
> call_qemu_default_main, NULL,
> QEMU_THREAD_DETACHED);
> return qemu_main();
> } else {
> return qemu_default_main();
> }
>
>
Good point, if it's safe to drop the lock on thread 0 and re-lock it on
another thread before running qemu_main_loop() there, it's also safe to
momentarily drop the lock on thread 0 and re-take it before calling into
qemu_main_loop(). I'll take that as a starting point and see how far I can
simplify things.
Thanks to both of you for the feedback!
Paolo
>
> > Regards,
> > BALATON Zoltan
> >
> > > + }
> > > }
> > > diff --git a/ui/cocoa.m b/ui/cocoa.m
> > > index 4c2dd335323..30b8920d929 100644
> > > --- a/ui/cocoa.m
> > > +++ b/ui/cocoa.m
> > > @@ -73,6 +73,8 @@
> > > int height;
> > > } QEMUScreen;
> > >
> > > +@class QemuCocoaPasteboardTypeOwner;
> > > +
> > > static void cocoa_update(DisplayChangeListener *dcl,
> > > int x, int y, int w, int h);
> > >
> > > @@ -107,6 +109,7 @@ static void cocoa_switch(DisplayChangeListener
> *dcl,
> > > static NSInteger cbchangecount = -1;
> > > static QemuClipboardInfo *cbinfo;
> > > static QemuEvent cbevent;
> > > +static QemuCocoaPasteboardTypeOwner *cbowner;
> > >
> > > // Utility functions to run specified code block with the BQL held
> > > typedef void (^CodeBlock)(void);
> > > @@ -1321,8 +1324,10 @@ - (void) dealloc
> > > {
> > > COCOA_DEBUG("QemuCocoaAppController: dealloc\n");
> > >
> > > - if (cocoaView)
> > > - [cocoaView release];
> > > + [cocoaView release];
> > > + [cbowner release];
> > > + cbowner = nil;
> > > +
> > > [super dealloc];
> > > }
> > >
> > > @@ -1938,8 +1943,6 @@ - (void)pasteboard:(NSPasteboard *)sender
> provideDataForType:(NSPasteboardType)t
> > >
> > > @end
> > >
> > > -static QemuCocoaPasteboardTypeOwner *cbowner;
> > > -
> > > static void cocoa_clipboard_notify(Notifier *notifier, void *data);
> > > static void cocoa_clipboard_request(QemuClipboardInfo *info,
> > > QemuClipboardType type);
> > > @@ -2002,43 +2005,8 @@ static void
> cocoa_clipboard_request(QemuClipboardInfo *info,
> > > }
> > > }
> > >
> > > -/*
> > > - * The startup process for the OSX/Cocoa UI is complicated, because
> > > - * OSX insists that the UI runs on the initial main thread, and so we
> > > - * need to start a second thread which runs the qemu_default_main():
> > > - * in main():
> > > - * in cocoa_display_init():
> > > - * assign cocoa_main to qemu_main
> > > - * create application, menus, etc
> > > - * in cocoa_main():
> > > - * create qemu-main thread
> > > - * enter OSX run loop
> > > - */
> > > -
> > > -static void *call_qemu_main(void *opaque)
> > > -{
> > > - int status;
> > > -
> > > - COCOA_DEBUG("Second thread: calling qemu_default_main()\n");
> > > - bql_lock();
> > > - status = qemu_default_main();
> > > - bql_unlock();
> > > - COCOA_DEBUG("Second thread: qemu_default_main() returned,
> exiting\n");
> > > - [cbowner release];
> > > - exit(status);
> > > -}
> > > -
> > > static int cocoa_main(void)
> > > {
> > > - QemuThread thread;
> > > -
> > > - COCOA_DEBUG("Entered %s()\n", __func__);
> > > -
> > > - bql_unlock();
> > > - qemu_thread_create(&thread, "qemu_main", call_qemu_main,
> > > - NULL, QEMU_THREAD_DETACHED);
> > > -
> > > - // Start the main event loop
> > > COCOA_DEBUG("Main thread: entering OSX run loop\n");
> > > [NSApp run];
> > > COCOA_DEBUG("Main thread: left OSX run loop, which should never
> happen\n");
> > > @@ -2120,8 +2088,6 @@ static void cocoa_display_init(DisplayState *ds,
> DisplayOptions *opts)
> > >
> > > COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
> > >
> > > - qemu_main = cocoa_main;
> > > -
> > > // Pull this console process up to being a fully-fledged graphical
> > > // app with a menubar and Dock icon
> > > ProcessSerialNumber psn = { 0, kCurrentProcess };
> > > @@ -2185,6 +2151,12 @@ static void cocoa_display_init(DisplayState
> *ds, DisplayOptions *opts)
> > > qemu_clipboard_peer_register(&cbpeer);
> > >
> > > [pool release];
> > > +
> > > + /*
> > > + * The Cocoa UI will run the NSApplication runloop on the main
> thread
> > > + * rather than the default Core Foundation one.
> > > + */
> > > + qemu_main = cocoa_main;
> > > }
> > >
> > > static QemuDisplay qemu_display_cocoa = {
> > > diff --git a/ui/gtk.c b/ui/gtk.c
> > > index bf9d3dd679a..fbf20161f36 100644
> > > --- a/ui/gtk.c
> > > +++ b/ui/gtk.c
> > > @@ -38,6 +38,7 @@
> > > #include "qemu/cutils.h"
> > > #include "qemu/error-report.h"
> > > #include "qemu/main-loop.h"
> > > +#include "qemu-main.h"
> > >
> > > #include "ui/console.h"
> > > #include "ui/gtk.h"
> > > @@ -2485,6 +2486,13 @@ static void gtk_display_init(DisplayState *ds,
> DisplayOptions *opts)
> > > #ifdef CONFIG_GTK_CLIPBOARD
> > > gd_clipboard_init(s);
> > > #endif /* CONFIG_GTK_CLIPBOARD */
> > > +
> > > + /*
> > > + * GTK+ calls must happen on the main thread at least on some
> platforms,
> > > + * and on macOS the main runloop is polled via GTK+'s event
> handling.
> > > + * Don't allow QEMU's event loop to be moved off the main thread.
> > > + */
> > > + qemu_main = NULL;
> > > }
> > >
> > > static void early_gtk_display_init(DisplayOptions *opts)
> > > diff --git a/ui/sdl2.c b/ui/sdl2.c
> > > index bd4f5a9da14..44ab2762262 100644
> > > --- a/ui/sdl2.c
> > > +++ b/ui/sdl2.c
> > > @@ -34,6 +34,7 @@
> > > #include "sysemu/sysemu.h"
> > > #include "ui/win32-kbd-hook.h"
> > > #include "qemu/log.h"
> > > +#include "qemu-main.h"
> > >
> > > static int sdl2_num_outputs;
> > > static struct sdl2_console *sdl2_console;
> > > @@ -965,6 +966,9 @@ static void sdl2_display_init(DisplayState *ds,
> DisplayOptions *o)
> > > }
> > >
> > > atexit(sdl_cleanup);
> > > +
> > > + /* SDL's event polling (in dpy_refresh) must happen on the main
> thread. */
> > > + qemu_main = NULL;
> > > }
> > >
> > > static QemuDisplay qemu_display_sdl2 = {
> > >
> >
>
>
>
