All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread
@ 2022-03-16 16:03 Philippe Mathieu-Daudé
  2022-03-16 16:22 ` Akihiko Odaki
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-16 16:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Emanuele Giuseppe Esposito,
	Philippe Mathieu-Daudé,
	Gerd Hoffmann, Akihiko Odaki, Paolo Bonzini

From: Paolo Bonzini <pbonzini@redhat.com>

Simplify the initialization dance by running qemu_init() in the main
thread before the Cocoa event loop starts.  The cocoa_display_init()
code that is post-applicationDidFinishLaunching: moves to the
application delegate itself, and the secondary thread only runs
the rest of qemu_main(), namely qemu_main_loop() and qemu_cleanup().

This fixes a case where addRemovableDevicesMenuItems() calls
qmp_query_block() while expecting the main thread to still hold
the BQL.  The newly-introduced assertions in the block layer
complain about this.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20220307151004.578069-1-pbonzini@redhat.com>
[PMD: Fixed trivial build failures & rebased]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Akihiko Odaki <akihiko.odaki@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Emanuele Giuseppe Esposito <eesposit@redhat.com>

v1: https://lore.kernel.org/qemu-devel/20220307151004.578069-1-pbonzini@redhat.com/
---
 softmmu/main.c |  12 ++--
 ui/cocoa.m     | 153 ++++++++++++++++++++++---------------------------
 2 files changed, 74 insertions(+), 91 deletions(-)

diff --git a/softmmu/main.c b/softmmu/main.c
index 639c67ff48..0c4384e980 100644
--- a/softmmu/main.c
+++ b/softmmu/main.c
@@ -39,16 +39,18 @@ int main(int argc, char **argv)
 #endif
 #endif /* CONFIG_SDL */
 
-#ifdef CONFIG_COCOA
-#undef main
-#define main qemu_main
-#endif /* CONFIG_COCOA */
-
+#ifndef CONFIG_COCOA
 int main(int argc, char **argv, char **envp)
 {
+    /*
+     * ui/cocoa.m relies on this being the exact content of main(),
+     * because it has to run everything after qemu_init in a secondary
+     * thread.
+     */
     qemu_init(argc, argv, envp);
     qemu_main_loop();
     qemu_cleanup();
 
     return 0;
 }
+#endif /* CONFIG_COCOA */
diff --git a/ui/cocoa.m b/ui/cocoa.m
index cb6e7c41dc..e69ce97f44 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -75,6 +75,9 @@ typedef struct {
     int height;
 } QEMUScreen;
 
+@interface QemuCocoaPasteboardTypeOwner : NSObject<NSPasteboardTypeOwner>
+@end
+
 static void cocoa_update(DisplayChangeListener *dcl,
                          int x, int y, int w, int h);
 
@@ -97,20 +100,23 @@ static int last_buttons;
 static int cursor_hide = 1;
 static int left_command_key_enabled = 1;
 static bool swap_opt_cmd;
+static bool full_screen;
+static bool full_grab;
+static bool have_cocoa_ui;
 
-static int gArgc;
-static char **gArgv;
 static bool stretch_video;
 static NSTextField *pauseLabel;
 
-static QemuSemaphore display_init_sem;
-static QemuSemaphore app_started_sem;
 static bool allow_events;
 
 static NSInteger cbchangecount = -1;
 static QemuClipboardInfo *cbinfo;
 static QemuEvent cbevent;
 
+static QemuCocoaPasteboardTypeOwner *cbowner;
+static QemuClipboardPeer cbpeer;
+static QemuThread main_thread;
+
 // Utility functions to run specified code block with iothread lock held
 typedef void (^CodeBlock)(void);
 typedef bool (^BoolCodeBlock)(void);
@@ -142,6 +148,33 @@ static bool bool_with_iothread_lock(BoolCodeBlock block)
     return val;
 }
 
+/*
+ * 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 qemu_main_loop():
+ *
+ * Initial thread:                    2nd thread:
+ * in main():
+ *  qemu_init()
+ *  create application, menus, etc
+ *  enter OSX run loop
+ * in applicationDidFinishLaunching:
+ *  fullscreen if needed
+ *  create main loop thread
+ *                                    call qemu_main_loop()
+ */
+
+static void *call_qemu_main_loop(void *opaque)
+{
+    COCOA_DEBUG("Second thread: calling qemu_main()\n");
+    qemu_mutex_lock_iothread();
+    qemu_main_loop();
+    COCOA_DEBUG("Second thread: qemu_main_loop() returned, exiting\n");
+    qemu_cleanup();
+    [cbowner release];
+    exit(0);
+}
+
 // Mac to QKeyCode conversion
 static const int mac_to_qkeycode_map[] = {
     [kVK_ANSI_A] = Q_KEY_CODE_A,
@@ -780,9 +813,7 @@ static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEven
         /*
          * Just let OSX have all events that arrive before
          * applicationDidFinishLaunching.
-         * This avoids a deadlock on the iothread lock, which cocoa_display_init()
-         * will not drop until after the app_started_sem is posted. (In theory
-         * there should not be any such events, but OSX Catalina now emits some.)
+         * This may not be needed anymore?
          */
         return false;
     }
@@ -1280,8 +1311,22 @@ static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEven
 {
     COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
     allow_events = true;
-    /* Tell cocoa_display_init to proceed */
-    qemu_sem_post(&app_started_sem);
+
+    // register vga output callbacks
+    register_displaychangelistener(&dcl);
+
+    qemu_clipboard_peer_register(&cbpeer);
+    qemu_mutex_unlock_iothread();
+    qemu_thread_create(&main_thread, "qemu_main_loop", call_qemu_main_loop,
+                       NULL, QEMU_THREAD_DETACHED);
+
+    if (full_screen) {
+        [NSApp activateIgnoringOtherApps: YES];
+        [self toggleFullScreen: nil];
+    }
+    if (full_grab) {
+        [self setFullGrab: nil];
+    }
 }
 
 - (void)applicationWillTerminate:(NSNotification *)aNotification
@@ -1804,9 +1849,6 @@ static void addRemovableDevicesMenuItems(void)
     qapi_free_BlockInfoList(pointerToFree);
 }
 
-@interface QemuCocoaPasteboardTypeOwner : NSObject<NSPasteboardTypeOwner>
-@end
-
 @implementation QemuCocoaPasteboardTypeOwner
 
 - (void)pasteboard:(NSPasteboard *)sender provideDataForType:(NSPasteboardType)type
@@ -1841,8 +1883,6 @@ static void addRemovableDevicesMenuItems(void)
 
 @end
 
-static QemuCocoaPasteboardTypeOwner *cbowner;
-
 static void cocoa_clipboard_notify(Notifier *notifier, void *data);
 static void cocoa_clipboard_request(QemuClipboardInfo *info,
                                     QemuClipboardType type);
@@ -1903,60 +1943,18 @@ 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 vl.c qemu_main():
- *
- * Initial thread:                    2nd thread:
- * in main():
- *  create qemu-main thread
- *  wait on display_init semaphore
- *                                    call qemu_main()
- *                                    ...
- *                                    in cocoa_display_init():
- *                                     post the display_init semaphore
- *                                     wait on app_started semaphore
- *  create application, menus, etc
- *  enter OSX run loop
- * in applicationDidFinishLaunching:
- *  post app_started semaphore
- *                                     tell main thread to fullscreen if needed
- *                                    [...]
- *                                    run qemu main-loop
- *
- * We do this in two stages so that we don't do the creation of the
- * GUI application menus and so on for command line options like --help
- * where we want to just print text to stdout and exit immediately.
- */
-
-static void *call_qemu_main(void *opaque)
+int main(int argc, char **argv, char **envp)
 {
-    int status;
-
-    COCOA_DEBUG("Second thread: calling qemu_main()\n");
-    status = qemu_main(gArgc, gArgv, *_NSGetEnviron());
-    COCOA_DEBUG("Second thread: qemu_main() returned, exiting\n");
-    [cbowner release];
-    exit(status);
-}
-
-int main (int argc, char **argv) {
-    QemuThread thread;
-
     COCOA_DEBUG("Entered main()\n");
-    gArgc = argc;
-    gArgv = argv;
+    qemu_event_init(&cbevent, false);
 
-    qemu_sem_init(&display_init_sem, 0);
-    qemu_sem_init(&app_started_sem, 0);
-
-    qemu_thread_create(&thread, "qemu_main", call_qemu_main,
-                       NULL, QEMU_THREAD_DETACHED);
-
-    COCOA_DEBUG("Main thread: waiting for display_init_sem\n");
-    qemu_sem_wait(&display_init_sem);
-    COCOA_DEBUG("Main thread: initializing app\n");
+    /* Takes iothread lock, released in applicationDidFinishLaunching:.  */
+    qemu_init(argc, argv, envp);
+    if (!have_cocoa_ui) {
+         qemu_main_loop();
+         qemu_cleanup();
+         return 0;
+    }
 
     NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
 
@@ -1978,6 +1976,7 @@ int main (int argc, char **argv) {
      */
     add_console_menu_entries();
     addRemovableDevicesMenuItems();
+    cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];
 
     // Create an Application controller
     QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
@@ -2071,24 +2070,13 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
 static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
 {
     COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
+    have_cocoa_ui = 1;
 
-    /* Tell main thread to go ahead and create the app and enter the run loop */
-    qemu_sem_post(&display_init_sem);
-    qemu_sem_wait(&app_started_sem);
-    COCOA_DEBUG("cocoa_display_init: app start completed\n");
-
-    QemuCocoaAppController *controller = (QemuCocoaAppController *)[[NSApplication sharedApplication] delegate];
-    /* if fullscreen mode is to be used */
     if (opts->has_full_screen && opts->full_screen) {
-        dispatch_async(dispatch_get_main_queue(), ^{
-            [NSApp activateIgnoringOtherApps: YES];
-            [controller toggleFullScreen: nil];
-        });
+        full_screen = 1;
     }
     if (opts->u.cocoa.has_full_grab && opts->u.cocoa.full_grab) {
-        dispatch_async(dispatch_get_main_queue(), ^{
-            [controller setFullGrab: nil];
-        });
+        full_grab = 1;
     }
 
     if (opts->has_show_cursor && opts->show_cursor) {
@@ -2101,13 +2089,6 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
     if (opts->u.cocoa.has_left_command_key && !opts->u.cocoa.left_command_key) {
         left_command_key_enabled = 0;
     }
-
-    // register vga output callbacks
-    register_displaychangelistener(&dcl);
-
-    qemu_event_init(&cbevent, false);
-    cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];
-    qemu_clipboard_peer_register(&cbpeer);
 }
 
 static QemuDisplay qemu_display_cocoa = {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread
  2022-03-16 16:03 [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread Philippe Mathieu-Daudé
@ 2022-03-16 16:22 ` Akihiko Odaki
  2022-03-16 17:31   ` Paolo Bonzini
  2022-03-16 16:47 ` Akihiko Odaki
  2022-03-17 11:57 ` Paolo Bonzini
  2 siblings, 1 reply; 9+ messages in thread
From: Akihiko Odaki @ 2022-03-16 16:22 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Emanuele Giuseppe Esposito,
	Philippe Mathieu-Daudé,
	Gerd Hoffmann, Paolo Bonzini

On 2022/03/17 1:03, Philippe Mathieu-Daudé wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> Simplify the initialization dance by running qemu_init() in the main
> thread before the Cocoa event loop starts.  The cocoa_display_init()
> code that is post-applicationDidFinishLaunching: moves to the
> application delegate itself, and the secondary thread only runs
> the rest of qemu_main(), namely qemu_main_loop() and qemu_cleanup().
> 
> This fixes a case where addRemovableDevicesMenuItems() calls
> qmp_query_block() while expecting the main thread to still hold
> the BQL.  The newly-introduced assertions in the block layer
> complain about this.

I was thinking that it may be better to let softmmu/main.c do the 
details if it involves the internals of qemu_main() like qemu_main_loop().

More concretely, softmmu/main.c would provide a function to register a 
function pointer to take over the main thread. main() implemented in 
softmmu/main.c would call qemu_init(). If a function pointer gets 
registered in qemu_init(), it would create a thread for main loop and 
call the registered function pointer. Otherwise, it would directly call 
qemu_main_loop().

It would be a semantically appropriate division of ui/cocoa.m and 
softmmu/main.c. It would also be beneficial for end-users as it would 
also allow to isolate ui/cocoa.m into a separate module when 
--enable-modules in the future. (With "In the future", I mean sometime 
when we have time to hack Meson build files and some details we cannot 
fill by 7.0.)

Regards,
Akihiko Odaki

> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Message-Id: <20220307151004.578069-1-pbonzini@redhat.com>
> [PMD: Fixed trivial build failures & rebased]
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Cc: Akihiko Odaki <akihiko.odaki@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> 
> v1: https://lore.kernel.org/qemu-devel/20220307151004.578069-1-pbonzini@redhat.com/
> ---
>   softmmu/main.c |  12 ++--
>   ui/cocoa.m     | 153 ++++++++++++++++++++++---------------------------
>   2 files changed, 74 insertions(+), 91 deletions(-)
> 
> diff --git a/softmmu/main.c b/softmmu/main.c
> index 639c67ff48..0c4384e980 100644
> --- a/softmmu/main.c
> +++ b/softmmu/main.c
> @@ -39,16 +39,18 @@ int main(int argc, char **argv)
>   #endif
>   #endif /* CONFIG_SDL */
>   
> -#ifdef CONFIG_COCOA
> -#undef main
> -#define main qemu_main
> -#endif /* CONFIG_COCOA */
> -
> +#ifndef CONFIG_COCOA
>   int main(int argc, char **argv, char **envp)
>   {
> +    /*
> +     * ui/cocoa.m relies on this being the exact content of main(),
> +     * because it has to run everything after qemu_init in a secondary
> +     * thread.
> +     */
>       qemu_init(argc, argv, envp);
>       qemu_main_loop();
>       qemu_cleanup();
>   
>       return 0;
>   }
> +#endif /* CONFIG_COCOA */
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index cb6e7c41dc..e69ce97f44 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -75,6 +75,9 @@ typedef struct {
>       int height;
>   } QEMUScreen;
>   
> +@interface QemuCocoaPasteboardTypeOwner : NSObject<NSPasteboardTypeOwner>
> +@end
> +
>   static void cocoa_update(DisplayChangeListener *dcl,
>                            int x, int y, int w, int h);
>   
> @@ -97,20 +100,23 @@ static int last_buttons;
>   static int cursor_hide = 1;
>   static int left_command_key_enabled = 1;
>   static bool swap_opt_cmd;
> +static bool full_screen;
> +static bool full_grab;
> +static bool have_cocoa_ui;
>   
> -static int gArgc;
> -static char **gArgv;
>   static bool stretch_video;
>   static NSTextField *pauseLabel;
>   
> -static QemuSemaphore display_init_sem;
> -static QemuSemaphore app_started_sem;
>   static bool allow_events;
>   
>   static NSInteger cbchangecount = -1;
>   static QemuClipboardInfo *cbinfo;
>   static QemuEvent cbevent;
>   
> +static QemuCocoaPasteboardTypeOwner *cbowner;
> +static QemuClipboardPeer cbpeer;
> +static QemuThread main_thread;
> +
>   // Utility functions to run specified code block with iothread lock held
>   typedef void (^CodeBlock)(void);
>   typedef bool (^BoolCodeBlock)(void);
> @@ -142,6 +148,33 @@ static bool bool_with_iothread_lock(BoolCodeBlock block)
>       return val;
>   }
>   
> +/*
> + * 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 qemu_main_loop():
> + *
> + * Initial thread:                    2nd thread:
> + * in main():
> + *  qemu_init()
> + *  create application, menus, etc
> + *  enter OSX run loop
> + * in applicationDidFinishLaunching:
> + *  fullscreen if needed
> + *  create main loop thread
> + *                                    call qemu_main_loop()
> + */
> +
> +static void *call_qemu_main_loop(void *opaque)
> +{
> +    COCOA_DEBUG("Second thread: calling qemu_main()\n");
> +    qemu_mutex_lock_iothread();
> +    qemu_main_loop();
> +    COCOA_DEBUG("Second thread: qemu_main_loop() returned, exiting\n");
> +    qemu_cleanup();
> +    [cbowner release];
> +    exit(0);
> +}
> +
>   // Mac to QKeyCode conversion
>   static const int mac_to_qkeycode_map[] = {
>       [kVK_ANSI_A] = Q_KEY_CODE_A,
> @@ -780,9 +813,7 @@ static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEven
>           /*
>            * Just let OSX have all events that arrive before
>            * applicationDidFinishLaunching.
> -         * This avoids a deadlock on the iothread lock, which cocoa_display_init()
> -         * will not drop until after the app_started_sem is posted. (In theory
> -         * there should not be any such events, but OSX Catalina now emits some.)
> +         * This may not be needed anymore?
>            */
>           return false;
>       }
> @@ -1280,8 +1311,22 @@ static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEven
>   {
>       COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
>       allow_events = true;
> -    /* Tell cocoa_display_init to proceed */
> -    qemu_sem_post(&app_started_sem);
> +
> +    // register vga output callbacks
> +    register_displaychangelistener(&dcl);
> +
> +    qemu_clipboard_peer_register(&cbpeer);
> +    qemu_mutex_unlock_iothread();
> +    qemu_thread_create(&main_thread, "qemu_main_loop", call_qemu_main_loop,
> +                       NULL, QEMU_THREAD_DETACHED);
> +
> +    if (full_screen) {
> +        [NSApp activateIgnoringOtherApps: YES];
> +        [self toggleFullScreen: nil];
> +    }
> +    if (full_grab) {
> +        [self setFullGrab: nil];
> +    }
>   }
>   
>   - (void)applicationWillTerminate:(NSNotification *)aNotification
> @@ -1804,9 +1849,6 @@ static void addRemovableDevicesMenuItems(void)
>       qapi_free_BlockInfoList(pointerToFree);
>   }
>   
> -@interface QemuCocoaPasteboardTypeOwner : NSObject<NSPasteboardTypeOwner>
> -@end
> -
>   @implementation QemuCocoaPasteboardTypeOwner
>   
>   - (void)pasteboard:(NSPasteboard *)sender provideDataForType:(NSPasteboardType)type
> @@ -1841,8 +1883,6 @@ static void addRemovableDevicesMenuItems(void)
>   
>   @end
>   
> -static QemuCocoaPasteboardTypeOwner *cbowner;
> -
>   static void cocoa_clipboard_notify(Notifier *notifier, void *data);
>   static void cocoa_clipboard_request(QemuClipboardInfo *info,
>                                       QemuClipboardType type);
> @@ -1903,60 +1943,18 @@ 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 vl.c qemu_main():
> - *
> - * Initial thread:                    2nd thread:
> - * in main():
> - *  create qemu-main thread
> - *  wait on display_init semaphore
> - *                                    call qemu_main()
> - *                                    ...
> - *                                    in cocoa_display_init():
> - *                                     post the display_init semaphore
> - *                                     wait on app_started semaphore
> - *  create application, menus, etc
> - *  enter OSX run loop
> - * in applicationDidFinishLaunching:
> - *  post app_started semaphore
> - *                                     tell main thread to fullscreen if needed
> - *                                    [...]
> - *                                    run qemu main-loop
> - *
> - * We do this in two stages so that we don't do the creation of the
> - * GUI application menus and so on for command line options like --help
> - * where we want to just print text to stdout and exit immediately.
> - */
> -
> -static void *call_qemu_main(void *opaque)
> +int main(int argc, char **argv, char **envp)
>   {
> -    int status;
> -
> -    COCOA_DEBUG("Second thread: calling qemu_main()\n");
> -    status = qemu_main(gArgc, gArgv, *_NSGetEnviron());
> -    COCOA_DEBUG("Second thread: qemu_main() returned, exiting\n");
> -    [cbowner release];
> -    exit(status);
> -}
> -
> -int main (int argc, char **argv) {
> -    QemuThread thread;
> -
>       COCOA_DEBUG("Entered main()\n");
> -    gArgc = argc;
> -    gArgv = argv;
> +    qemu_event_init(&cbevent, false);
>   
> -    qemu_sem_init(&display_init_sem, 0);
> -    qemu_sem_init(&app_started_sem, 0);
> -
> -    qemu_thread_create(&thread, "qemu_main", call_qemu_main,
> -                       NULL, QEMU_THREAD_DETACHED);
> -
> -    COCOA_DEBUG("Main thread: waiting for display_init_sem\n");
> -    qemu_sem_wait(&display_init_sem);
> -    COCOA_DEBUG("Main thread: initializing app\n");
> +    /* Takes iothread lock, released in applicationDidFinishLaunching:.  */
> +    qemu_init(argc, argv, envp);
> +    if (!have_cocoa_ui) {
> +         qemu_main_loop();
> +         qemu_cleanup();
> +         return 0;
> +    }
>   
>       NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
>   
> @@ -1978,6 +1976,7 @@ int main (int argc, char **argv) {
>        */
>       add_console_menu_entries();
>       addRemovableDevicesMenuItems();
> +    cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];
>   
>       // Create an Application controller
>       QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
> @@ -2071,24 +2070,13 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
>   static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>   {
>       COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
> +    have_cocoa_ui = 1;
>   
> -    /* Tell main thread to go ahead and create the app and enter the run loop */
> -    qemu_sem_post(&display_init_sem);
> -    qemu_sem_wait(&app_started_sem);
> -    COCOA_DEBUG("cocoa_display_init: app start completed\n");
> -
> -    QemuCocoaAppController *controller = (QemuCocoaAppController *)[[NSApplication sharedApplication] delegate];
> -    /* if fullscreen mode is to be used */
>       if (opts->has_full_screen && opts->full_screen) {
> -        dispatch_async(dispatch_get_main_queue(), ^{
> -            [NSApp activateIgnoringOtherApps: YES];
> -            [controller toggleFullScreen: nil];
> -        });
> +        full_screen = 1;
>       }
>       if (opts->u.cocoa.has_full_grab && opts->u.cocoa.full_grab) {
> -        dispatch_async(dispatch_get_main_queue(), ^{
> -            [controller setFullGrab: nil];
> -        });
> +        full_grab = 1;
>       }
>   
>       if (opts->has_show_cursor && opts->show_cursor) {
> @@ -2101,13 +2089,6 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>       if (opts->u.cocoa.has_left_command_key && !opts->u.cocoa.left_command_key) {
>           left_command_key_enabled = 0;
>       }
> -
> -    // register vga output callbacks
> -    register_displaychangelistener(&dcl);
> -
> -    qemu_event_init(&cbevent, false);
> -    cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];
> -    qemu_clipboard_peer_register(&cbpeer);
>   }
>   
>   static QemuDisplay qemu_display_cocoa = {



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread
  2022-03-16 16:03 [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread Philippe Mathieu-Daudé
  2022-03-16 16:22 ` Akihiko Odaki
@ 2022-03-16 16:47 ` Akihiko Odaki
  2022-03-17 11:57 ` Paolo Bonzini
  2 siblings, 0 replies; 9+ messages in thread
From: Akihiko Odaki @ 2022-03-16 16:47 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Emanuele Giuseppe Esposito,
	Philippe Mathieu-Daudé,
	Gerd Hoffmann, Paolo Bonzini

On 2022/03/17 1:03, Philippe Mathieu-Daudé wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> Simplify the initialization dance by running qemu_init() in the main
> thread before the Cocoa event loop starts.  The cocoa_display_init()
> code that is post-applicationDidFinishLaunching: moves to the
> application delegate itself, and the secondary thread only runs
> the rest of qemu_main(), namely qemu_main_loop() and qemu_cleanup().
> 
> This fixes a case where addRemovableDevicesMenuItems() calls
> qmp_query_block() while expecting the main thread to still hold
> the BQL.  The newly-introduced assertions in the block layer
> complain about this.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Message-Id: <20220307151004.578069-1-pbonzini@redhat.com>
> [PMD: Fixed trivial build failures & rebased]
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Cc: Akihiko Odaki <akihiko.odaki@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> 
> v1: https://lore.kernel.org/qemu-devel/20220307151004.578069-1-pbonzini@redhat.com/
> ---
>   softmmu/main.c |  12 ++--
>   ui/cocoa.m     | 153 ++++++++++++++++++++++---------------------------
>   2 files changed, 74 insertions(+), 91 deletions(-)
> 
> diff --git a/softmmu/main.c b/softmmu/main.c
> index 639c67ff48..0c4384e980 100644
> --- a/softmmu/main.c
> +++ b/softmmu/main.c
> @@ -39,16 +39,18 @@ int main(int argc, char **argv)
>   #endif
>   #endif /* CONFIG_SDL */
>   
> -#ifdef CONFIG_COCOA
> -#undef main
> -#define main qemu_main
> -#endif /* CONFIG_COCOA */
> -
> +#ifndef CONFIG_COCOA
>   int main(int argc, char **argv, char **envp)
>   {
> +    /*
> +     * ui/cocoa.m relies on this being the exact content of main(),
> +     * because it has to run everything after qemu_init in a secondary
> +     * thread.
> +     */
>       qemu_init(argc, argv, envp);
>       qemu_main_loop();
>       qemu_cleanup();
>   
>       return 0;
>   }
> +#endif /* CONFIG_COCOA */
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index cb6e7c41dc..e69ce97f44 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -75,6 +75,9 @@ typedef struct {
>       int height;
>   } QEMUScreen;
>   
> +@interface QemuCocoaPasteboardTypeOwner : NSObject<NSPasteboardTypeOwner>
> +@end
> +
>   static void cocoa_update(DisplayChangeListener *dcl,
>                            int x, int y, int w, int h);
>   
> @@ -97,20 +100,23 @@ static int last_buttons;
>   static int cursor_hide = 1;
>   static int left_command_key_enabled = 1;
>   static bool swap_opt_cmd;
> +static bool full_screen;
> +static bool full_grab;
> +static bool have_cocoa_ui;
>   
> -static int gArgc;
> -static char **gArgv;
>   static bool stretch_video;
>   static NSTextField *pauseLabel;
>   
> -static QemuSemaphore display_init_sem;
> -static QemuSemaphore app_started_sem;
>   static bool allow_events;
>   
>   static NSInteger cbchangecount = -1;
>   static QemuClipboardInfo *cbinfo;
>   static QemuEvent cbevent;
>   
> +static QemuCocoaPasteboardTypeOwner *cbowner;
> +static QemuClipboardPeer cbpeer;
> +static QemuThread main_thread;
> +
>   // Utility functions to run specified code block with iothread lock held
>   typedef void (^CodeBlock)(void);
>   typedef bool (^BoolCodeBlock)(void);
> @@ -142,6 +148,33 @@ static bool bool_with_iothread_lock(BoolCodeBlock block)
>       return val;
>   }
>   
> +/*
> + * 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 qemu_main_loop():
> + *
> + * Initial thread:                    2nd thread:
> + * in main():
> + *  qemu_init()
> + *  create application, menus, etc
> + *  enter OSX run loop
> + * in applicationDidFinishLaunching:
> + *  fullscreen if needed
> + *  create main loop thread
> + *                                    call qemu_main_loop()
> + */
> +
> +static void *call_qemu_main_loop(void *opaque)
> +{
> +    COCOA_DEBUG("Second thread: calling qemu_main()\n");
> +    qemu_mutex_lock_iothread();
> +    qemu_main_loop();
> +    COCOA_DEBUG("Second thread: qemu_main_loop() returned, exiting\n");
> +    qemu_cleanup();
> +    [cbowner release];
> +    exit(0);
> +}
> +
>   // Mac to QKeyCode conversion
>   static const int mac_to_qkeycode_map[] = {
>       [kVK_ANSI_A] = Q_KEY_CODE_A,
> @@ -780,9 +813,7 @@ static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEven
>           /*
>            * Just let OSX have all events that arrive before
>            * applicationDidFinishLaunching.
> -         * This avoids a deadlock on the iothread lock, which cocoa_display_init()
> -         * will not drop until after the app_started_sem is posted. (In theory
> -         * there should not be any such events, but OSX Catalina now emits some.)
> +         * This may not be needed anymore?
>            */
>           return false;
>       }
> @@ -1280,8 +1311,22 @@ static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEven
>   {
>       COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
>       allow_events = true;
> -    /* Tell cocoa_display_init to proceed */
> -    qemu_sem_post(&app_started_sem);
> +
> +    // register vga output callbacks
> +    register_displaychangelistener(&dcl);
> +
> +    qemu_clipboard_peer_register(&cbpeer);
> +    qemu_mutex_unlock_iothread();
> +    qemu_thread_create(&main_thread, "qemu_main_loop", call_qemu_main_loop,
> +                       NULL, QEMU_THREAD_DETACHED);
> +
> +    if (full_screen) {
> +        [NSApp activateIgnoringOtherApps: YES];
> +        [self toggleFullScreen: nil];
> +    }
> +    if (full_grab) {
> +        [self setFullGrab: nil];
> +    }
>   }
>   
>   - (void)applicationWillTerminate:(NSNotification *)aNotification
> @@ -1804,9 +1849,6 @@ static void addRemovableDevicesMenuItems(void)
>       qapi_free_BlockInfoList(pointerToFree);
>   }
>   
> -@interface QemuCocoaPasteboardTypeOwner : NSObject<NSPasteboardTypeOwner>
> -@end
> -
>   @implementation QemuCocoaPasteboardTypeOwner
>   
>   - (void)pasteboard:(NSPasteboard *)sender provideDataForType:(NSPasteboardType)type
> @@ -1841,8 +1883,6 @@ static void addRemovableDevicesMenuItems(void)
>   
>   @end
>   
> -static QemuCocoaPasteboardTypeOwner *cbowner;
> -
>   static void cocoa_clipboard_notify(Notifier *notifier, void *data);
>   static void cocoa_clipboard_request(QemuClipboardInfo *info,
>                                       QemuClipboardType type);
> @@ -1903,60 +1943,18 @@ 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 vl.c qemu_main():
> - *
> - * Initial thread:                    2nd thread:
> - * in main():
> - *  create qemu-main thread
> - *  wait on display_init semaphore
> - *                                    call qemu_main()
> - *                                    ...
> - *                                    in cocoa_display_init():
> - *                                     post the display_init semaphore
> - *                                     wait on app_started semaphore
> - *  create application, menus, etc
> - *  enter OSX run loop
> - * in applicationDidFinishLaunching:
> - *  post app_started semaphore
> - *                                     tell main thread to fullscreen if needed
> - *                                    [...]
> - *                                    run qemu main-loop
> - *
> - * We do this in two stages so that we don't do the creation of the
> - * GUI application menus and so on for command line options like --help
> - * where we want to just print text to stdout and exit immediately.
> - */
> -
> -static void *call_qemu_main(void *opaque)
> +int main(int argc, char **argv, char **envp)
>   {
> -    int status;
> -
> -    COCOA_DEBUG("Second thread: calling qemu_main()\n");
> -    status = qemu_main(gArgc, gArgv, *_NSGetEnviron());
> -    COCOA_DEBUG("Second thread: qemu_main() returned, exiting\n");
> -    [cbowner release];
> -    exit(status);
> -}
> -
> -int main (int argc, char **argv) {
> -    QemuThread thread;
> -
>       COCOA_DEBUG("Entered main()\n");
> -    gArgc = argc;
> -    gArgv = argv;
> +    qemu_event_init(&cbevent, false);
>   
> -    qemu_sem_init(&display_init_sem, 0);
> -    qemu_sem_init(&app_started_sem, 0);
> -
> -    qemu_thread_create(&thread, "qemu_main", call_qemu_main,
> -                       NULL, QEMU_THREAD_DETACHED);
> -
> -    COCOA_DEBUG("Main thread: waiting for display_init_sem\n");
> -    qemu_sem_wait(&display_init_sem);
> -    COCOA_DEBUG("Main thread: initializing app\n");
> +    /* Takes iothread lock, released in applicationDidFinishLaunching:.  */
> +    qemu_init(argc, argv, envp);
> +    if (!have_cocoa_ui) {
> +         qemu_main_loop();
> +         qemu_cleanup();
> +         return 0;
> +    }
>   
>       NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
>   
> @@ -1978,6 +1976,7 @@ int main (int argc, char **argv) {
>        */
>       add_console_menu_entries();
>       addRemovableDevicesMenuItems();
> +    cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];
>   
>       // Create an Application controller
>       QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
> @@ -2071,24 +2070,13 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
>   static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>   {
>       COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
> +    have_cocoa_ui = 1;
>   
> -    /* Tell main thread to go ahead and create the app and enter the run loop */
> -    qemu_sem_post(&display_init_sem);
> -    qemu_sem_wait(&app_started_sem);
> -    COCOA_DEBUG("cocoa_display_init: app start completed\n");
> -
> -    QemuCocoaAppController *controller = (QemuCocoaAppController *)[[NSApplication sharedApplication] delegate];
> -    /* if fullscreen mode is to be used */
>       if (opts->has_full_screen && opts->full_screen) {
> -        dispatch_async(dispatch_get_main_queue(), ^{
> -            [NSApp activateIgnoringOtherApps: YES];
> -            [controller toggleFullScreen: nil];
> -        });
> +        full_screen = 1;

We could just save opts and use it later. sdl2 does this.

Regards,
Akihiko Odaki

>       }
>       if (opts->u.cocoa.has_full_grab && opts->u.cocoa.full_grab) {
> -        dispatch_async(dispatch_get_main_queue(), ^{
> -            [controller setFullGrab: nil];
> -        });
> +        full_grab = 1;
>       }
>   
>       if (opts->has_show_cursor && opts->show_cursor) {
> @@ -2101,13 +2089,6 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>       if (opts->u.cocoa.has_left_command_key && !opts->u.cocoa.left_command_key) {
>           left_command_key_enabled = 0;
>       }
> -
> -    // register vga output callbacks
> -    register_displaychangelistener(&dcl);
> -
> -    qemu_event_init(&cbevent, false);
> -    cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];
> -    qemu_clipboard_peer_register(&cbpeer);
>   }
>   
>   static QemuDisplay qemu_display_cocoa = {



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread
  2022-03-16 16:22 ` Akihiko Odaki
@ 2022-03-16 17:31   ` Paolo Bonzini
  2022-03-16 19:29     ` Peter Maydell
  0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2022-03-16 17:31 UTC (permalink / raw)
  To: Akihiko Odaki, Philippe Mathieu-Daudé, qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Emanuele Giuseppe Esposito,
	Philippe Mathieu-Daudé,
	Gerd Hoffmann

On 3/16/22 17:22, Akihiko Odaki wrote:
> I was thinking that it may be better to let softmmu/main.c do the 
> details if it involves the internals of qemu_main() like qemu_main_loop().
> 
> More concretely, softmmu/main.c would provide a function to register a 
> function pointer to take over the main thread. main() implemented in 
> softmmu/main.c would call qemu_init(). If a function pointer gets 
> registered in qemu_init(), it would create a thread for main loop and 
> call the registered function pointer. Otherwise, it would directly call 
> qemu_main_loop().
> 
> It would be a semantically appropriate division of ui/cocoa.m and 
> softmmu/main.c. It would also be beneficial for end-users as it would 
> also allow to isolate ui/cocoa.m into a separate module when 
> --enable-modules in the future. (With "In the future", I mean sometime 
> when we have time to hack Meson build files and some details we cannot 
> fill by 7.0.)

I would like this for 7.1.

Basically rename qemu_main_loop to qemu_default_main_loop, and 
cocoa_display_init would do

     qemu_main_loop = qemu_cocoa_main_loop;

qemu_cocoa_main_loop would include the bulk of the current main of 
ui/cocoa.m.  Seems like a good idea.

Paolo


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread
  2022-03-16 17:31   ` Paolo Bonzini
@ 2022-03-16 19:29     ` Peter Maydell
  2022-03-16 21:06       ` Peter Maydell
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2022-03-16 19:29 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, Emanuele Giuseppe Esposito, qemu-devel,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	Gerd Hoffmann, Akihiko Odaki

On Wed, 16 Mar 2022 at 17:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 3/16/22 17:22, Akihiko Odaki wrote:
> > I was thinking that it may be better to let softmmu/main.c do the
> > details if it involves the internals of qemu_main() like qemu_main_loop().
> >
> > More concretely, softmmu/main.c would provide a function to register a
> > function pointer to take over the main thread. main() implemented in
> > softmmu/main.c would call qemu_init(). If a function pointer gets
> > registered in qemu_init(), it would create a thread for main loop and
> > call the registered function pointer. Otherwise, it would directly call
> > qemu_main_loop().
> >
> > It would be a semantically appropriate division of ui/cocoa.m and
> > softmmu/main.c. It would also be beneficial for end-users as it would
> > also allow to isolate ui/cocoa.m into a separate module when
> > --enable-modules in the future. (With "In the future", I mean sometime
> > when we have time to hack Meson build files and some details we cannot
> > fill by 7.0.)
>
> I would like this for 7.1.
>
> Basically rename qemu_main_loop to qemu_default_main_loop, and
> cocoa_display_init would do
>
>      qemu_main_loop = qemu_cocoa_main_loop;
>
> qemu_cocoa_main_loop would include the bulk of the current main of
> ui/cocoa.m.  Seems like a good idea.

Speaking of 7.1, is cocoa currently completely broken, ie in need
of an interim fix for 7.0 ? If so, which of the various patches/approaches
should it be ?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread
  2022-03-16 19:29     ` Peter Maydell
@ 2022-03-16 21:06       ` Peter Maydell
  2022-03-17 11:51         ` Paolo Bonzini
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2022-03-16 21:06 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, Emanuele Giuseppe Esposito, qemu-devel,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	Gerd Hoffmann, Akihiko Odaki

On Wed, 16 Mar 2022 at 19:29, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 16 Mar 2022 at 17:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > On 3/16/22 17:22, Akihiko Odaki wrote:
> > > I was thinking that it may be better to let softmmu/main.c do the
> > > details if it involves the internals of qemu_main() like qemu_main_loop().
> > >
> > > More concretely, softmmu/main.c would provide a function to register a
> > > function pointer to take over the main thread. main() implemented in
> > > softmmu/main.c would call qemu_init(). If a function pointer gets
> > > registered in qemu_init(), it would create a thread for main loop and
> > > call the registered function pointer. Otherwise, it would directly call
> > > qemu_main_loop().
> > >
> > > It would be a semantically appropriate division of ui/cocoa.m and
> > > softmmu/main.c. It would also be beneficial for end-users as it would
> > > also allow to isolate ui/cocoa.m into a separate module when
> > > --enable-modules in the future. (With "In the future", I mean sometime
> > > when we have time to hack Meson build files and some details we cannot
> > > fill by 7.0.)
> >
> > I would like this for 7.1.
> >
> > Basically rename qemu_main_loop to qemu_default_main_loop, and
> > cocoa_display_init would do
> >
> >      qemu_main_loop = qemu_cocoa_main_loop;
> >
> > qemu_cocoa_main_loop would include the bulk of the current main of
> > ui/cocoa.m.  Seems like a good idea.
>
> Speaking of 7.1, is cocoa currently completely broken, ie in need
> of an interim fix for 7.0 ? If so, which of the various patches/approaches
> should it be ?

To answer the first half of my question, yes, the cocoa UI is
currently completely broken as it asserts on startup.

-- PMM


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread
  2022-03-16 21:06       ` Peter Maydell
@ 2022-03-17 11:51         ` Paolo Bonzini
  0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2022-03-17 11:51 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Kevin Wolf, Emanuele Giuseppe Esposito, qemu-devel,
	Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
	Gerd Hoffmann, Akihiko Odaki

On 3/16/22 22:06, Peter Maydell wrote:
>> Speaking of 7.1, is cocoa currently completely broken, ie in need
>> of an interim fix for 7.0 ? If so, which of the various patches/approaches
>> should it be ?
> 
> To answer the first half of my question, yes, the cocoa UI is
> currently completely broken as it asserts on startup.

I think this patch is a good interim fix for 7.0 and an improvement in 
general.  Any other changes can be deferred to 7.1.  For example it's 
trivial for Cocoa not to block GTK+/SDL anymore, but no need to have 
that in 7.0.  Same for removing main() from ui/cocoa.m.

Paolo


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread
  2022-03-16 16:03 [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread Philippe Mathieu-Daudé
  2022-03-16 16:22 ` Akihiko Odaki
  2022-03-16 16:47 ` Akihiko Odaki
@ 2022-03-17 11:57 ` Paolo Bonzini
  2022-03-17 12:44   ` Philippe Mathieu-Daudé
  2 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2022-03-17 11:57 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Emanuele Giuseppe Esposito,
	Philippe Mathieu-Daudé,
	Gerd Hoffmann, Akihiko Odaki

Just one change to aid future reading of the code, possibly.  Move this 
line:

On 3/16/22 17:03, Philippe Mathieu-Daudé wrote:
> +    qemu_event_init(&cbevent, false);

just before

+    cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];

i.e. the place where it was before the patch, in cocoa_display_init.

Paolo


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread
  2022-03-17 11:57 ` Paolo Bonzini
@ 2022-03-17 12:44   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-17 12:44 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Emanuele Giuseppe Esposito,
	Philippe Mathieu-Daudé,
	Gerd Hoffmann, Akihiko Odaki

On 17/3/22 12:57, Paolo Bonzini wrote:
> Just one change to aid future reading of the code, possibly.  Move this 
> line:
> 
> On 3/16/22 17:03, Philippe Mathieu-Daudé wrote:
>> +    qemu_event_init(&cbevent, false);
> 
> just before
> 
> +    cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];
> 
> i.e. the place where it was before the patch, in cocoa_display_init.

OK. I'll also include this change
(s/cocoa_display_init/applicationDidFinishLaunching):

@@ -611,17 +611,17 @@
  - (void) updateUIInfo
  {
      if (!allow_events) {
          /*
           * Don't try to tell QEMU about UI information in the application
           * startup phase -- we haven't yet registered dcl with the QEMU UI
           * layer, and also trying to take the iothread lock would 
deadlock.
-         * When cocoa_display_init() does register the dcl, the UI layer
-         * will call cocoa_switch(), which will call updateUIInfo, so
-         * we don't lose any information here.
+         * When applicationDidFinishLaunching() does register the dcl, the
+         * UI layer will call cocoa_switch(), which will call updateUIInfo,
+         * so we don't lose any information here.
           */
          return;
      }


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-03-17 12:47 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-16 16:03 [RFC PATCH-for-7.0 v2] cocoa: run qemu_init in the main thread Philippe Mathieu-Daudé
2022-03-16 16:22 ` Akihiko Odaki
2022-03-16 17:31   ` Paolo Bonzini
2022-03-16 19:29     ` Peter Maydell
2022-03-16 21:06       ` Peter Maydell
2022-03-17 11:51         ` Paolo Bonzini
2022-03-16 16:47 ` Akihiko Odaki
2022-03-17 11:57 ` Paolo Bonzini
2022-03-17 12:44   ` Philippe Mathieu-Daudé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.