| Message ID | 20250410225550.46807-2-pierrick.bouvier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | fix record/replay on MacOS | expand |
On Fri Apr 11, 2025 at 8:55 AM AEST, Pierrick Bouvier wrote: > On MacOS, UI event loop has to be ran in the main thread of a process. > Because of that restriction, on this platform, qemu main event loop is > ran on another thread [1]. > > This breaks record/replay feature, which expects thread running qemu_init > to initialize hold this lock, breaking associated functional tests on > MacOS. > > Thus, as a generalization, and similar to how BQL is handled, we release > it after init, and reacquire the lock before entering main event loop, > avoiding a special case if a separate thread is used. > > Tested on MacOS with: > $ meson test -C build --setup thorough --print-errorlogs \ > func-x86_64-x86_64_replay func-arm-arm_replay func-aarch64-aarch64_replay > $ ./build/qemu-system-x86_64 -nographic -icount shift=auto,rr=record,rrfile=replay.log > $ ./build/qemu-system-x86_64 -nographic -icount shift=auto,rr=replay,rrfile=replay.log > > [1] https://gitlab.com/qemu-project/qemu/-/commit/f5ab12caba4f1656479c1feb5248beac1c833243 > > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2907 > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > system/main.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/system/main.c b/system/main.c > index ecb12fd397c..1c022067349 100644 > --- a/system/main.c > +++ b/system/main.c > @@ -25,6 +25,7 @@ > #include "qemu/osdep.h" > #include "qemu-main.h" > #include "qemu/main-loop.h" > +#include "system/replay.h" > #include "system/system.h" > > #ifdef CONFIG_SDL > @@ -44,10 +45,12 @@ static void *qemu_default_main(void *opaque) > { > int status; > > + replay_mutex_lock(); > bql_lock(); > status = qemu_main_loop(); > qemu_cleanup(status); > bql_unlock(); > + replay_mutex_unlock(); > > exit(status); > } > @@ -67,6 +70,7 @@ int main(int argc, char **argv) > { > qemu_init(argc, argv); > bql_unlock(); > + replay_mutex_unlock(); > if (qemu_main) { > QemuThread main_loop_thread; > qemu_thread_create(&main_loop_thread, "qemu_main", Do we actually need to hold replay mutex (or even bql) over qemu_init()? Both should get dropped before we return here. But as a simple fix, I guess this is okay. Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
On 4/11/25 22:30, Nicholas Piggin wrote: > On Fri Apr 11, 2025 at 8:55 AM AEST, Pierrick Bouvier wrote: >> On MacOS, UI event loop has to be ran in the main thread of a process. >> Because of that restriction, on this platform, qemu main event loop is >> ran on another thread [1]. >> >> This breaks record/replay feature, which expects thread running qemu_init >> to initialize hold this lock, breaking associated functional tests on >> MacOS. >> >> Thus, as a generalization, and similar to how BQL is handled, we release >> it after init, and reacquire the lock before entering main event loop, >> avoiding a special case if a separate thread is used. >> >> Tested on MacOS with: >> $ meson test -C build --setup thorough --print-errorlogs \ >> func-x86_64-x86_64_replay func-arm-arm_replay func-aarch64-aarch64_replay >> $ ./build/qemu-system-x86_64 -nographic -icount shift=auto,rr=record,rrfile=replay.log >> $ ./build/qemu-system-x86_64 -nographic -icount shift=auto,rr=replay,rrfile=replay.log >> >> [1] https://gitlab.com/qemu-project/qemu/-/commit/f5ab12caba4f1656479c1feb5248beac1c833243 >> >> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2907 >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> system/main.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/system/main.c b/system/main.c >> index ecb12fd397c..1c022067349 100644 >> --- a/system/main.c >> +++ b/system/main.c >> @@ -25,6 +25,7 @@ >> #include "qemu/osdep.h" >> #include "qemu-main.h" >> #include "qemu/main-loop.h" >> +#include "system/replay.h" >> #include "system/system.h" >> >> #ifdef CONFIG_SDL >> @@ -44,10 +45,12 @@ static void *qemu_default_main(void *opaque) >> { >> int status; >> >> + replay_mutex_lock(); >> bql_lock(); >> status = qemu_main_loop(); >> qemu_cleanup(status); >> bql_unlock(); >> + replay_mutex_unlock(); >> >> exit(status); >> } >> @@ -67,6 +70,7 @@ int main(int argc, char **argv) >> { >> qemu_init(argc, argv); >> bql_unlock(); >> + replay_mutex_unlock(); >> if (qemu_main) { >> QemuThread main_loop_thread; >> qemu_thread_create(&main_loop_thread, "qemu_main", > > Do we actually need to hold replay mutex (or even bql) over qemu_init()? > Both should get dropped before we return here. But as a simple fix, I > guess this is okay. > For the bql, I don't know the exact reason. For replay lock, we need to hold it as clock gets saved as soon as the devices are initialized, which happens before end of qemu_init. > Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
diff --git a/system/main.c b/system/main.c index ecb12fd397c..1c022067349 100644 --- a/system/main.c +++ b/system/main.c @@ -25,6 +25,7 @@ #include "qemu/osdep.h" #include "qemu-main.h" #include "qemu/main-loop.h" +#include "system/replay.h" #include "system/system.h" #ifdef CONFIG_SDL @@ -44,10 +45,12 @@ static void *qemu_default_main(void *opaque) { int status; + replay_mutex_lock(); bql_lock(); status = qemu_main_loop(); qemu_cleanup(status); bql_unlock(); + replay_mutex_unlock(); exit(status); } @@ -67,6 +70,7 @@ int main(int argc, char **argv) { qemu_init(argc, argv); bql_unlock(); + replay_mutex_unlock(); if (qemu_main) { QemuThread main_loop_thread; qemu_thread_create(&main_loop_thread, "qemu_main",
On MacOS, UI event loop has to be ran in the main thread of a process. Because of that restriction, on this platform, qemu main event loop is ran on another thread [1]. This breaks record/replay feature, which expects thread running qemu_init to initialize hold this lock, breaking associated functional tests on MacOS. Thus, as a generalization, and similar to how BQL is handled, we release it after init, and reacquire the lock before entering main event loop, avoiding a special case if a separate thread is used. Tested on MacOS with: $ meson test -C build --setup thorough --print-errorlogs \ func-x86_64-x86_64_replay func-arm-arm_replay func-aarch64-aarch64_replay $ ./build/qemu-system-x86_64 -nographic -icount shift=auto,rr=record,rrfile=replay.log $ ./build/qemu-system-x86_64 -nographic -icount shift=auto,rr=replay,rrfile=replay.log [1] https://gitlab.com/qemu-project/qemu/-/commit/f5ab12caba4f1656479c1feb5248beac1c833243 Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2907 Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> --- system/main.c | 4 ++++ 1 file changed, 4 insertions(+)