* [PATCH 0/3] ui/cocoa: Run qemu_init in the main thread
@ 2022-07-06 2:13 Akihiko Odaki
2022-07-06 2:13 ` [PATCH 1/3] " Akihiko Odaki
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Akihiko Odaki @ 2022-07-06 2:13 UTC (permalink / raw)
Cc: qemu-devel, Peter Maydell, Paolo Bonzini, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé, Akihiko Odaki
This work is based on:
https://patchew.org/QEMU/20220317125534.38706-1-philippe.mathieu.daude@gmail.com/
Simplify the initialization dance by running qemu_init() in the main
thread before the Cocoa event loop starts. The secondary thread only
runs only 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.
Overriding the code after calling qemu_init() is done by dynamically
replacing a function pointer variable, qemu_main when initializing
ui/cocoa, which unifies the static implementation of main() for
builds with ui/cocoa and ones without ui/cocoa.
Akihiko Odaki (3):
ui/cocoa: Run qemu_init in the main thread
Revert "main-loop: Disable block backend global state assertion on
Cocoa"
meson: Allow to enable gtk and sdl while cocoa is enabled
docs/devel/fuzzing.rst | 4 +-
include/qemu-main.h | 3 +-
include/qemu/main-loop.h | 13 ---
include/sysemu/sysemu.h | 2 +-
meson.build | 10 +--
softmmu/main.c | 14 +--
softmmu/vl.c | 2 +-
tests/qtest/fuzz/fuzz.c | 2 +-
ui/cocoa.m | 185 ++++++++++++---------------------------
9 files changed, 71 insertions(+), 164 deletions(-)
--
2.32.1 (Apple Git-133)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] ui/cocoa: Run qemu_init in the main thread
2022-07-06 2:13 [PATCH 0/3] ui/cocoa: Run qemu_init in the main thread Akihiko Odaki
@ 2022-07-06 2:13 ` Akihiko Odaki
2022-07-12 21:40 ` Philippe Mathieu-Daudé via
2022-07-06 2:13 ` [PATCH 2/3] Revert "main-loop: Disable block backend global state assertion on Cocoa" Akihiko Odaki
2022-07-06 2:13 ` [PATCH 3/3] meson: Allow to enable gtk and sdl while cocoa is enabled Akihiko Odaki
2 siblings, 1 reply; 6+ messages in thread
From: Akihiko Odaki @ 2022-07-06 2:13 UTC (permalink / raw)
Cc: qemu-devel, Peter Maydell, Paolo Bonzini, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé, Akihiko Odaki
This work is based on:
https://patchew.org/QEMU/20220317125534.38706-1-philippe.mathieu.daude@gmail.com/
Simplify the initialization dance by running qemu_init() in the main
thread before the Cocoa event loop starts. The secondary thread only
runs only 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.
Overriding the code after calling qemu_init() is done by dynamically
replacing a function pointer variable, qemu_main when initializing
ui/cocoa, which unifies the static implementation of main() for
builds with ui/cocoa and ones without ui/cocoa.
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
docs/devel/fuzzing.rst | 4 +-
include/qemu-main.h | 3 +-
include/sysemu/sysemu.h | 2 +-
softmmu/main.c | 14 +--
softmmu/vl.c | 2 +-
tests/qtest/fuzz/fuzz.c | 2 +-
ui/cocoa.m | 185 ++++++++++++----------------------------
7 files changed, 69 insertions(+), 143 deletions(-)
diff --git a/docs/devel/fuzzing.rst b/docs/devel/fuzzing.rst
index 784ecb99e66..715330c8561 100644
--- a/docs/devel/fuzzing.rst
+++ b/docs/devel/fuzzing.rst
@@ -287,8 +287,8 @@ select the fuzz target. Then, the qtest client is initialized. If the target
requires qos, qgraph is set up and the QOM/LIBQOS modules are initialized.
Then the QGraph is walked and the QEMU cmd_line is determined and saved.
-After this, the ``vl.c:qemu_main`` is called to set up the guest. There are
-target-specific hooks that can be called before and after qemu_main, for
+After this, the ``vl.c:main`` is called to set up the guest. There are
+target-specific hooks that can be called before and after main, for
additional setup(e.g. PCI setup, or VM snapshotting).
``LLVMFuzzerTestOneInput``: Uses qtest/qos functions to act based on the fuzz
diff --git a/include/qemu-main.h b/include/qemu-main.h
index 6a3e90d0ad5..6889375e7c2 100644
--- a/include/qemu-main.h
+++ b/include/qemu-main.h
@@ -5,6 +5,7 @@
#ifndef QEMU_MAIN_H
#define QEMU_MAIN_H
-int qemu_main(int argc, char **argv, char **envp);
+void qemu_default_main(void);
+extern void (*qemu_main)(void);
#endif /* QEMU_MAIN_H */
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 812f66a31a9..254c1eabf57 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -102,7 +102,7 @@ void qemu_boot_set(const char *boot_order, Error **errp);
bool defaults_enabled(void);
-void qemu_init(int argc, char **argv, char **envp);
+void qemu_init(int argc, char **argv);
void qemu_main_loop(void);
void qemu_cleanup(void);
diff --git a/softmmu/main.c b/softmmu/main.c
index c00432ff098..41a091f2c72 100644
--- a/softmmu/main.c
+++ b/softmmu/main.c
@@ -30,18 +30,18 @@
#include <SDL.h>
#endif
-int qemu_main(int argc, char **argv, char **envp)
+void qemu_default_main(void)
{
- qemu_init(argc, argv, envp);
qemu_main_loop();
qemu_cleanup();
-
- return 0;
}
-#ifndef CONFIG_COCOA
+void (*qemu_main)(void) = qemu_default_main;
+
int main(int argc, char **argv)
{
- return qemu_main(argc, argv, NULL);
+ qemu_init(argc, argv);
+ qemu_main();
+
+ return 0;
}
-#endif
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 3f264d4b093..e8c73d0bb40 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2589,7 +2589,7 @@ void qmp_x_exit_preconfig(Error **errp)
}
}
-void qemu_init(int argc, char **argv, char **envp)
+void qemu_init(int argc, char **argv)
{
QemuOpts *opts;
QemuOpts *icount_opts = NULL, *accel_opts = NULL;
diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index 2062b40d82b..0bde925bf83 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -221,7 +221,7 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
g_free(pretty_cmd_line);
}
- qemu_init(result.we_wordc, result.we_wordv, NULL);
+ qemu_init(result.we_wordc, result.we_wordv);
/* re-enable the rcu atfork, which was previously disabled in qemu_init */
rcu_enable_atfork();
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 6a4dccff7f0..55413594d14 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -100,15 +100,11 @@ static void cocoa_switch(DisplayChangeListener *dcl,
static int left_command_key_enabled = 1;
static bool swap_opt_cmd;
-static int gArgc;
-static char **gArgv;
+static QemuThread qemu_main_thread;
+static bool qemu_main_terminating;
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;
@@ -581,18 +577,6 @@ - (void) updateUIInfoLocked
- (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.
- */
- return;
- }
-
with_iothread_lock(^{
[self updateUIInfoLocked];
});
@@ -778,16 +762,6 @@ - (void) handleMonitorInput:(NSEvent *)event
- (bool) handleEvent:(NSEvent *)event
{
- if(!allow_events) {
- /*
- * 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.)
- */
- return false;
- }
return bool_with_iothread_lock(^{
return [self handleEventLocked:event];
});
@@ -1279,29 +1253,18 @@ - (void) dealloc
[super dealloc];
}
-- (void)applicationDidFinishLaunching: (NSNotification *) note
-{
- COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
- allow_events = true;
- /* Tell cocoa_display_init to proceed */
- qemu_sem_post(&app_started_sem);
-}
-
- (void)applicationWillTerminate:(NSNotification *)aNotification
{
COCOA_DEBUG("QemuCocoaAppController: applicationWillTerminate\n");
with_iothread_lock(^{
- shutdown_action = SHUTDOWN_ACTION_POWEROFF;
- qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+ if (!qemu_main_terminating) {
+ shutdown_action = SHUTDOWN_ACTION_POWEROFF;
+ qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+ }
});
- /*
- * Sleep here, because returning will cause OSX to kill us
- * immediately; the QEMU main loop will handle the shutdown
- * request and terminate the process.
- */
- [NSThread sleepForTimeInterval:INFINITY];
+ qemu_thread_join(&qemu_main_thread);
}
- (BOOL)applicationShouldTerminateAfterLastWindowClosed:(NSApplication *)theApplication
@@ -1313,7 +1276,7 @@ - (NSApplicationTerminateReply)applicationShouldTerminate:
(NSApplication *)sender
{
COCOA_DEBUG("QemuCocoaAppController: applicationShouldTerminate\n");
- return [self verifyQuit];
+ return qatomic_read(&qemu_main_terminating) || [self verifyQuit];
}
- (void)windowDidChangeScreen:(NSNotification *)notification
@@ -1915,92 +1878,35 @@ 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.
+ * need to start a second thread which runs the qemu_default_main().
*/
static void *call_qemu_main(void *opaque)
{
- 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");
+ COCOA_DEBUG("Second thread: calling qemu_default_main()\n");
+ qemu_mutex_lock_iothread();
+ qemu_default_main();
+ qatomic_set(&qemu_main_terminating, true);
+ qemu_mutex_unlock_iothread();
+ COCOA_DEBUG("Second thread: qemu_default_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;
+ [NSApp terminate:nil];
- 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");
-
- NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
-
- // Pull this console process up to being a fully-fledged graphical
- // app with a menubar and Dock icon
- ProcessSerialNumber psn = { 0, kCurrentProcess };
- TransformProcessType(&psn, kProcessTransformToForegroundApplication);
-
- [QemuApplication sharedApplication];
-
- create_initial_menus();
+ return NULL;
+}
- /*
- * Create the menu entries which depend on QEMU state (for consoles
- * and removeable devices). These make calls back into QEMU functions,
- * which is OK because at this point we know that the second thread
- * holds the iothread lock and is synchronously waiting for us to
- * finish.
- */
- add_console_menu_entries();
- addRemovableDevicesMenuItems();
+static void cocoa_main()
+{
+ COCOA_DEBUG("Entered %s()\n", __func__);
- // Create an Application controller
- QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
- [NSApp setDelegate:appController];
+ qemu_mutex_unlock_iothread();
+ qemu_thread_create(&qemu_main_thread, "qemu_main", call_qemu_main,
+ NULL, QEMU_THREAD_JOINABLE);
// Start the main event loop
COCOA_DEBUG("Main thread: entering OSX run loop\n");
[NSApp run];
COCOA_DEBUG("Main thread: left OSX run loop, exiting\n");
-
- [appController release];
- [pool release];
-
- return 0;
}
@@ -2079,25 +1985,42 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
{
+ NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+
COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
- /* 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");
+ 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 };
+ TransformProcessType(&psn, kProcessTransformToForegroundApplication);
+
+ [QemuApplication sharedApplication];
+
+ create_initial_menus();
+
+ /*
+ * Create the menu entries which depend on QEMU state (for consoles
+ * and removeable devices). These make calls back into QEMU functions,
+ * which is OK because at this point we know that the second thread
+ * holds the iothread lock and is synchronously waiting for us to
+ * finish.
+ */
+ add_console_menu_entries();
+ addRemovableDevicesMenuItems();
+
+ // Create an Application controller
+ QemuCocoaAppController *controller = [[QemuCocoaAppController alloc] init];
+ [NSApp setDelegate:controller];
- 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];
- });
+ [NSApp activateIgnoringOtherApps: YES];
+ [controller toggleFullScreen: nil];
}
if (opts->u.cocoa.has_full_grab && opts->u.cocoa.full_grab) {
- dispatch_async(dispatch_get_main_queue(), ^{
- [controller setFullGrab: nil];
- });
+ [controller setFullGrab: nil];
}
if (opts->has_show_cursor && opts->show_cursor) {
@@ -2117,6 +2040,8 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
qemu_event_init(&cbevent, false);
cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];
qemu_clipboard_peer_register(&cbpeer);
+
+ [pool release];
}
static QemuDisplay qemu_display_cocoa = {
--
2.32.1 (Apple Git-133)
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] Revert "main-loop: Disable block backend global state assertion on Cocoa"
2022-07-06 2:13 [PATCH 0/3] ui/cocoa: Run qemu_init in the main thread Akihiko Odaki
2022-07-06 2:13 ` [PATCH 1/3] " Akihiko Odaki
@ 2022-07-06 2:13 ` Akihiko Odaki
2022-07-06 7:32 ` Emanuele Giuseppe Esposito
2022-07-06 2:13 ` [PATCH 3/3] meson: Allow to enable gtk and sdl while cocoa is enabled Akihiko Odaki
2 siblings, 1 reply; 6+ messages in thread
From: Akihiko Odaki @ 2022-07-06 2:13 UTC (permalink / raw)
Cc: qemu-devel, Peter Maydell, Paolo Bonzini, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé, Akihiko Odaki
This reverts commit 47281859f66bdab1974fb122cab2cbb4a1c9af7f.
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
include/qemu/main-loop.h | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 5518845299d..0aa36a4f17e 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -280,23 +280,10 @@ 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_COCOA
-/*
- * When using the Cocoa UI, addRemovableDevicesMenuItems() is called from
- * a thread different from the QEMU main thread and can not take the BQL,
- * triggering this assertions in the block layer (commit 0439c5a462).
- * As the Cocoa fix is not trivial, disable this assertion for the v7.0.0
- * release (when using Cocoa); we will restore it immediately after the
- * release.
- * This issue is tracked as https://gitlab.com/qemu-project/qemu/-/issues/926
- */
-#define GLOBAL_STATE_CODE()
-#else
#define GLOBAL_STATE_CODE() \
do { \
assert(qemu_in_main_thread()); \
} while (0)
-#endif /* CONFIG_COCOA */
/* Mark and check that the function is part of the I/O API. */
#define IO_CODE() \
--
2.32.1 (Apple Git-133)
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] meson: Allow to enable gtk and sdl while cocoa is enabled
2022-07-06 2:13 [PATCH 0/3] ui/cocoa: Run qemu_init in the main thread Akihiko Odaki
2022-07-06 2:13 ` [PATCH 1/3] " Akihiko Odaki
2022-07-06 2:13 ` [PATCH 2/3] Revert "main-loop: Disable block backend global state assertion on Cocoa" Akihiko Odaki
@ 2022-07-06 2:13 ` Akihiko Odaki
2 siblings, 0 replies; 6+ messages in thread
From: Akihiko Odaki @ 2022-07-06 2:13 UTC (permalink / raw)
Cc: qemu-devel, Peter Maydell, Paolo Bonzini, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf,
Philippe Mathieu-Daudé, Akihiko Odaki
As ui/cocoa does no longer override main(), ui/gtk and ui/sdl
can be enabled even ui/cocoa is enabled.
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
meson.build | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/meson.build b/meson.build
index 6e1c3eb2bc5..4714a0d5cf8 100644
--- a/meson.build
+++ b/meson.build
@@ -587,12 +587,6 @@ if get_option('attr').allowed()
endif
cocoa = dependency('appleframeworks', modules: 'Cocoa', required: get_option('cocoa'))
-if cocoa.found() and get_option('sdl').enabled()
- error('Cocoa and SDL cannot be enabled at the same time')
-endif
-if cocoa.found() and get_option('gtk').enabled()
- error('Cocoa and GTK+ cannot be enabled at the same time')
-endif
vmnet = dependency('appleframeworks', modules: 'vmnet', required: get_option('vmnet'))
if vmnet.found() and not cc.has_header_symbol('vmnet/vmnet.h',
@@ -919,7 +913,7 @@ if not get_option('brlapi').auto() or have_system
endif
sdl = not_found
-if not get_option('sdl').auto() or (have_system and not cocoa.found())
+if not get_option('sdl').auto() or have_system
sdl = dependency('sdl2', required: get_option('sdl'), kwargs: static_kwargs)
sdl_image = not_found
endif
@@ -1185,7 +1179,7 @@ endif
gtk = not_found
gtkx11 = not_found
vte = not_found
-if not get_option('gtk').auto() or (have_system and not cocoa.found())
+if not get_option('gtk').auto() or have_system
gtk = dependency('gtk+-3.0', version: '>=3.22.0',
method: 'pkg-config',
required: get_option('gtk'),
--
2.32.1 (Apple Git-133)
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] Revert "main-loop: Disable block backend global state assertion on Cocoa"
2022-07-06 2:13 ` [PATCH 2/3] Revert "main-loop: Disable block backend global state assertion on Cocoa" Akihiko Odaki
@ 2022-07-06 7:32 ` Emanuele Giuseppe Esposito
0 siblings, 0 replies; 6+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-07-06 7:32 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel, Peter Maydell, Paolo Bonzini, Gerd Hoffmann,
Kevin Wolf, Philippe Mathieu-Daudé
Am 06/07/2022 um 04:13 schrieb Akihiko Odaki:
> This reverts commit 47281859f66bdab1974fb122cab2cbb4a1c9af7f.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> ---
> include/qemu/main-loop.h | 13 -------------
> 1 file changed, 13 deletions(-)
Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] ui/cocoa: Run qemu_init in the main thread
2022-07-06 2:13 ` [PATCH 1/3] " Akihiko Odaki
@ 2022-07-12 21:40 ` Philippe Mathieu-Daudé via
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-07-12 21:40 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel, Peter Maydell, Paolo Bonzini, Gerd Hoffmann,
Emanuele Giuseppe Esposito, Kevin Wolf
Hi Akihiko,
On 6/7/22 04:13, Akihiko Odaki wrote:
> This work is based on:
> https://patchew.org/QEMU/20220317125534.38706-1-philippe.mathieu.daude@gmail.com/
>
> Simplify the initialization dance by running qemu_init() in the main
> thread before the Cocoa event loop starts. The secondary thread only
> runs only 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.
>
> Overriding the code after calling qemu_init() is done by dynamically
> replacing a function pointer variable, qemu_main when initializing
> ui/cocoa, which unifies the static implementation of main() for
> builds with ui/cocoa and ones without ui/cocoa.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> ---
> docs/devel/fuzzing.rst | 4 +-
> include/qemu-main.h | 3 +-
> include/sysemu/sysemu.h | 2 +-
> softmmu/main.c | 14 +--
> softmmu/vl.c | 2 +-
> tests/qtest/fuzz/fuzz.c | 2 +-
> ui/cocoa.m | 185 ++++++++++++----------------------------
> 7 files changed, 69 insertions(+), 143 deletions(-)
>
> diff --git a/docs/devel/fuzzing.rst b/docs/devel/fuzzing.rst
> index 784ecb99e66..715330c8561 100644
> --- a/docs/devel/fuzzing.rst
> +++ b/docs/devel/fuzzing.rst
> @@ -287,8 +287,8 @@ select the fuzz target. Then, the qtest client is initialized. If the target
> requires qos, qgraph is set up and the QOM/LIBQOS modules are initialized.
> Then the QGraph is walked and the QEMU cmd_line is determined and saved.
>
> -After this, the ``vl.c:qemu_main`` is called to set up the guest. There are
> -target-specific hooks that can be called before and after qemu_main, for
> +After this, the ``vl.c:main`` is called to set up the guest. There are
> +target-specific hooks that can be called before and after main, for
> additional setup(e.g. PCI setup, or VM snapshotting).
>
> ``LLVMFuzzerTestOneInput``: Uses qtest/qos functions to act based on the fuzz
> diff --git a/include/qemu-main.h b/include/qemu-main.h
> index 6a3e90d0ad5..6889375e7c2 100644
> --- a/include/qemu-main.h
> +++ b/include/qemu-main.h
> @@ -5,6 +5,7 @@
> #ifndef QEMU_MAIN_H
> #define QEMU_MAIN_H
>
> -int qemu_main(int argc, char **argv, char **envp);
> +void qemu_default_main(void);
> +extern void (*qemu_main)(void);
>
> #endif /* QEMU_MAIN_H */
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 812f66a31a9..254c1eabf57 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -102,7 +102,7 @@ void qemu_boot_set(const char *boot_order, Error **errp);
>
> bool defaults_enabled(void);
>
> -void qemu_init(int argc, char **argv, char **envp);
> +void qemu_init(int argc, char **argv);
> void qemu_main_loop(void);
> void qemu_cleanup(void);
>
> diff --git a/softmmu/main.c b/softmmu/main.c
> index c00432ff098..41a091f2c72 100644
> --- a/softmmu/main.c
> +++ b/softmmu/main.c
> @@ -30,18 +30,18 @@
> #include <SDL.h>
> #endif
>
> -int qemu_main(int argc, char **argv, char **envp)
> +void qemu_default_main(void)
> {
> - qemu_init(argc, argv, envp);
> qemu_main_loop();
> qemu_cleanup();
> -
> - return 0;
> }
>
> -#ifndef CONFIG_COCOA
> +void (*qemu_main)(void) = qemu_default_main;
> +
> int main(int argc, char **argv)
> {
> - return qemu_main(argc, argv, NULL);
> + qemu_init(argc, argv);
> + qemu_main();
> +
> + return 0;
> }
> -#endif
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 3f264d4b093..e8c73d0bb40 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2589,7 +2589,7 @@ void qmp_x_exit_preconfig(Error **errp)
> }
> }
>
> -void qemu_init(int argc, char **argv, char **envp)
> +void qemu_init(int argc, char **argv)
> {
> QemuOpts *opts;
> QemuOpts *icount_opts = NULL, *accel_opts = NULL;
> diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
> index 2062b40d82b..0bde925bf83 100644
> --- a/tests/qtest/fuzz/fuzz.c
> +++ b/tests/qtest/fuzz/fuzz.c
> @@ -221,7 +221,7 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
> g_free(pretty_cmd_line);
> }
>
> - qemu_init(result.we_wordc, result.we_wordv, NULL);
> + qemu_init(result.we_wordc, result.we_wordv);
>
> /* re-enable the rcu atfork, which was previously disabled in qemu_init */
> rcu_enable_atfork();
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 6a4dccff7f0..55413594d14 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -100,15 +100,11 @@ static void cocoa_switch(DisplayChangeListener *dcl,
> static int left_command_key_enabled = 1;
> static bool swap_opt_cmd;
>
> -static int gArgc;
> -static char **gArgv;
> +static QemuThread qemu_main_thread;
> +static bool qemu_main_terminating;
> 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;
> @@ -581,18 +577,6 @@ - (void) updateUIInfoLocked
>
> - (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.
> - */
> - return;
> - }
> -
> with_iothread_lock(^{
> [self updateUIInfoLocked];
> });
> @@ -778,16 +762,6 @@ - (void) handleMonitorInput:(NSEvent *)event
>
> - (bool) handleEvent:(NSEvent *)event
> {
> - if(!allow_events) {
> - /*
> - * 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.)
> - */
> - return false;
> - }
> return bool_with_iothread_lock(^{
> return [self handleEventLocked:event];
> });
> @@ -1279,29 +1253,18 @@ - (void) dealloc
> [super dealloc];
> }
>
> -- (void)applicationDidFinishLaunching: (NSNotification *) note
> -{
> - COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
> - allow_events = true;
> - /* Tell cocoa_display_init to proceed */
> - qemu_sem_post(&app_started_sem);
> -}
> -
> - (void)applicationWillTerminate:(NSNotification *)aNotification
> {
> COCOA_DEBUG("QemuCocoaAppController: applicationWillTerminate\n");
>
> with_iothread_lock(^{
> - shutdown_action = SHUTDOWN_ACTION_POWEROFF;
> - qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
> + if (!qemu_main_terminating) {
> + shutdown_action = SHUTDOWN_ACTION_POWEROFF;
> + qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
> + }
> });
>
> - /*
> - * Sleep here, because returning will cause OSX to kill us
> - * immediately; the QEMU main loop will handle the shutdown
> - * request and terminate the process.
> - */
> - [NSThread sleepForTimeInterval:INFINITY];
> + qemu_thread_join(&qemu_main_thread);
> }
>
> - (BOOL)applicationShouldTerminateAfterLastWindowClosed:(NSApplication *)theApplication
> @@ -1313,7 +1276,7 @@ - (NSApplicationTerminateReply)applicationShouldTerminate:
> (NSApplication *)sender
> {
> COCOA_DEBUG("QemuCocoaAppController: applicationShouldTerminate\n");
> - return [self verifyQuit];
> + return qatomic_read(&qemu_main_terminating) || [self verifyQuit];
> }
>
> - (void)windowDidChangeScreen:(NSNotification *)notification
> @@ -1915,92 +1878,35 @@ 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.
> + * need to start a second thread which runs the qemu_default_main().
> */
>
> static void *call_qemu_main(void *opaque)
> {
> - 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");
> + COCOA_DEBUG("Second thread: calling qemu_default_main()\n");
> + qemu_mutex_lock_iothread();
> + qemu_default_main();
> + qatomic_set(&qemu_main_terminating, true);
> + qemu_mutex_unlock_iothread();
> + COCOA_DEBUG("Second thread: qemu_default_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;
> + [NSApp terminate:nil];
>
> - 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");
> -
> - NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
> -
> - // Pull this console process up to being a fully-fledged graphical
> - // app with a menubar and Dock icon
> - ProcessSerialNumber psn = { 0, kCurrentProcess };
> - TransformProcessType(&psn, kProcessTransformToForegroundApplication);
> -
> - [QemuApplication sharedApplication];
> -
> - create_initial_menus();
> + return NULL;
> +}
>
> - /*
> - * Create the menu entries which depend on QEMU state (for consoles
> - * and removeable devices). These make calls back into QEMU functions,
> - * which is OK because at this point we know that the second thread
> - * holds the iothread lock and is synchronously waiting for us to
> - * finish.
> - */
> - add_console_menu_entries();
> - addRemovableDevicesMenuItems();
> +static void cocoa_main()
> +{
> + COCOA_DEBUG("Entered %s()\n", __func__);
>
> - // Create an Application controller
> - QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
> - [NSApp setDelegate:appController];
> + qemu_mutex_unlock_iothread();
> + qemu_thread_create(&qemu_main_thread, "qemu_main", call_qemu_main,
> + NULL, QEMU_THREAD_JOINABLE);
>
> // Start the main event loop
> COCOA_DEBUG("Main thread: entering OSX run loop\n");
> [NSApp run];
> COCOA_DEBUG("Main thread: left OSX run loop, exiting\n");
> -
> - [appController release];
> - [pool release];
> -
> - return 0;
> }
>
>
> @@ -2079,25 +1985,42 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
>
> static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
> {
> + NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
> +
> COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
>
> - /* 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");
> + 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 };
> + TransformProcessType(&psn, kProcessTransformToForegroundApplication);
> +
> + [QemuApplication sharedApplication];
> +
> + create_initial_menus();
> +
> + /*
> + * Create the menu entries which depend on QEMU state (for consoles
> + * and removeable devices). These make calls back into QEMU functions,
> + * which is OK because at this point we know that the second thread
> + * holds the iothread lock and is synchronously waiting for us to
> + * finish.
> + */
> + add_console_menu_entries();
> + addRemovableDevicesMenuItems();
> +
> + // Create an Application controller
> + QemuCocoaAppController *controller = [[QemuCocoaAppController alloc] init];
> + [NSApp setDelegate:controller];
>
> - 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];
> - });
> + [NSApp activateIgnoringOtherApps: YES];
> + [controller toggleFullScreen: nil];
> }
> if (opts->u.cocoa.has_full_grab && opts->u.cocoa.full_grab) {
> - dispatch_async(dispatch_get_main_queue(), ^{
> - [controller setFullGrab: nil];
> - });
> + [controller setFullGrab: nil];
> }
>
> if (opts->has_show_cursor && opts->show_cursor) {
> @@ -2117,6 +2040,8 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
> qemu_event_init(&cbevent, false);
> cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];
> qemu_clipboard_peer_register(&cbpeer);
> +
> + [pool release];
> }
>
> static QemuDisplay qemu_display_cocoa = {
This doesn't work for me:
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason =
EXC_BAD_ACCESS (code=1, address=0x18)
* frame #0: 0x0000000100012bf0
qemu-system-x86_64`update_displaychangelistener(dcl=0x00000001007d5f90,
interval=16) at console.c:1671:14
frame #1: 0x000000010001d150 qemu-system-x86_64`-[QemuCocoaView
updateUIInfoLocked](self=<unavailable>, _cmd=<unavailable>) at
cocoa.m:568:17
frame #2: 0x00000001ae31d2a8 AppKit`-[NSView _setWindow:] + 2196
frame #3: 0x00000001ae3266bc AppKit`-[NSView addSubview:] + 176
frame #4: 0x00000001ae32d898 AppKit`-[NSFrameView addSubview:] + 52
frame #5: 0x00000001ae32d84c AppKit`-[NSThemeFrame addSubview:] + 468
frame #6: 0x00000001ae32d394 AppKit`-[NSView
addSubview:positioned:relativeTo:] + 216
frame #7: 0x00000001ae32d220 AppKit`-[NSThemeFrame
addSubview:positioned:relativeTo:] + 52
frame #8: 0x00000001ae32d1d4 AppKit`-[NSThemeFrame
_addKnownSubview:positioned:relativeTo:] + 52
frame #9: 0x00000001ae34d9a4 AppKit`-[NSWindow setContentView:] + 376
frame #10: 0x000000010001e574
qemu-system-x86_64`-[QemuCocoaAppController
init](self=0x00006000000265f0, _cmd=<unavailable>) at cocoa.m:1230:9
frame #11: 0x0000000100020be4
qemu-system-x86_64`cocoa_display_init(ds=<unavailable>,
opts=0x0000000100b5d678) at cocoa.m:2018:42
frame #12: 0x00000001001c17b4 qemu-system-x86_64`qemu_init
[inlined] qemu_init_displays at vl.c:2449:5
frame #13: 0x00000001001c17a4
qemu-system-x86_64`qemu_init(argc=<unavailable>, argv=<unavailable>) at
vl.c:3566:5
frame #14: 0x000000010000dc5c
qemu-system-x86_64`main(argc=<unavailable>, argv=<unavailable>) at
main.c:43:5
frame #15: 0x000000010107108c dyld`start + 520
* thread #1, queue = 'com.apple.main-thread', stop reason =
EXC_BAD_ACCESS (code=1, address=0x18)
frame #0: 0x0000000100012bf0
qemu-system-x86_64`update_displaychangelistener(dcl=0x00000001007d5f90,
interval=16) at console.c:1671:14
1668 DisplayState *ds = dcl->ds;
1669
1670 dcl->update_interval = interval;
-> 1671 if (!ds->refreshing && ds->update_interval > interval) {
1672 timer_mod(ds->gui_timer, ds->last_update + interval);
1673 }
1674 }
(lldb) p ds
(DisplayState *) $0 = NULL
It seems what was discussed here:
https://lore.kernel.org/qemu-devel/89a0316d-7e9a-46d9-31cc-c3507483fffb@redhat.com/
register_displaychangelistener() not yet called.
Regards,
Phil.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-12 21:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-06 2:13 [PATCH 0/3] ui/cocoa: Run qemu_init in the main thread Akihiko Odaki
2022-07-06 2:13 ` [PATCH 1/3] " Akihiko Odaki
2022-07-12 21:40 ` Philippe Mathieu-Daudé via
2022-07-06 2:13 ` [PATCH 2/3] Revert "main-loop: Disable block backend global state assertion on Cocoa" Akihiko Odaki
2022-07-06 7:32 ` Emanuele Giuseppe Esposito
2022-07-06 2:13 ` [PATCH 3/3] meson: Allow to enable gtk and sdl while cocoa is enabled Akihiko Odaki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).