[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 2/4] vl: Initialise main loop earlier
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v5 2/4] vl: Initialise main loop earlier |
Date: |
Wed, 19 Feb 2020 15:57:34 +0100 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
Am 19.02.2020 um 15:07 hat Wolfgang Bumiller geschrieben:
> On Tue, Feb 18, 2020 at 04:40:34PM +0100, Kevin Wolf wrote:
> > We want to be able to use qemu_aio_context in the monitor
> > initialisation.
> >
> > Signed-off-by: Kevin Wolf <address@hidden>
> > Reviewed-by: Marc-André Lureau <address@hidden>
> > Reviewed-by: Markus Armbruster <address@hidden>
> > ---
> > vl.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index 794f2e5733..98bc51e089 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2894,6 +2894,11 @@ int main(int argc, char **argv, char **envp)
> > runstate_init();
> > precopy_infrastructure_init();
> > postcopy_infrastructure_init();
> > +
> > + if (qemu_init_main_loop(&main_loop_err)) {
> > + error_report_err(main_loop_err);
> > + exit(1);
> > + }
> > monitor_init_globals();
>
> This is a tiny bit scary, as we now have around 1kloc of code between
> here and os_daemonize() where in the future we may accidentally cause
> the aio context's on-demand thread pool to spawn before fork()ing
> (silently losing the threads again - we did have such an issue right
> there in monitor_init_globals() in the past)
I don't think it's that bad, because while it is many lines, it's just
boring option parsing code. I certainly don't think it's likely to start
using a thread pool anywhere.
However, I also wonder now if this patch is actually necessary. Maybe
some intermediate version of the series actually used qemu_aio_context
in monitor_init_globals_core(), but none of the versions actually sent
to the list do. They all use iohandler_get_aio_context(), which should
be safe before the main loop is initialised. To make sure that I didn't
miss anything, 'make check' passes without the patch.
So maybe we can just drop this patch after all.
Kevin