Re: [PATCH v6 01/33] main-loop.h: introduce qemu_in_main_thread()

[Kevin Wolf]

Am 01.02.2022 um 12:55 hat Kevin Wolf geschrieben:
> Am 31.01.2022 um 16:49 hat Paolo Bonzini geschrieben:
> > > > However, commit 5f50be9b5810293141bb53cfd0cb46e765367d56 changed the
> > > > stub to return false for a specific reason.
> > > 
> > > I must admit I don't really understand the reasoning there:
> > > 
> > >      With this change, the stub qemu_mutex_iothread_locked() must be 
> > > changed
> > >      from true to false.  The previous value of true was needed because 
> > > the
> > >      main thread did not have an AioContext in the thread-local variable,
> > >      but now it does have one.
> > > 
> > > This explains why it _can_ be changed to false for this caller, but not
> > > why it _must_ be changed.
> > 
> > See above: because it returns the wrong value for all threads except one,
> > and there are better ways to do a meaningful check in that one thread: using
> > qemu_get_current_aio_context(), which is what aio_co_enter did at the time
> > and qemu_in_main_thread() does with Emanuele's change.
> > 
> > > So is the problem with the unit tests really just that they never call
> > > qemu_init_main_loop() and therefore never set the AioContext for the
> > > main thread?
> > 
> > No, most of them do (and if some are missing it we can add it).
> 
> But if they do, why doesn't qemu_get_current_aio_context() already
> return the right result? In this case, my_aiocontext is set and it
> should never even call qemu_mutex_iothread_locked() - at least not in
> any case where qemu_in_main_thread() would return true.
> 
> So provided that qemu_init_main_loop() is called, both functions should
> be equivalent and we wouldn't need a second one.

Sorry, I was confused and comparing the wrong two functions.
qemu_get_current_aio_context() does return the right result and it's
exactly what qemu_in_main_thread() uses. So yes, it's right and it's
different from qemu_mutex_iothread_locked().

It would be less confusing if qemu_get_current_aio_context() used
qemu_in_main_thread() (with two different implementations of
qemu_in_main_thread() for the system emulator and tools) instead of the
other way around, but I guess that's harder to implement because we
would need a different way to figure out whether we're in the main
thread, at least as long as my_aiocontext is static and can't be
accessed in stubs. We could probably make it public, though.

Kevin