Re: [PATCH v5 2/4] vl: Initialise main loop earlier

[Wolfgang Bumiller]

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)

For *now* it should be fine since currently that wouldn't have worked,
but we may need to keep an eye out for that in future patches.
Basically, not everything we do between here and os_daemonize() way down
below is actually allowed to freely use the main loop's aio context even
if now it already exists from this point onward.

I wonder if aio_get_thread_pool() should check the daemonization state
(maybe with some debug #ifdef)?

>  
>      if (qcrypto_init(&err) < 0) {
> @@ -3811,11 +3816,6 @@ int main(int argc, char **argv, char **envp)
>      qemu_unlink_pidfile_notifier.notify = qemu_unlink_pidfile;
>      qemu_add_exit_notifier(&qemu_unlink_pidfile_notifier);
>  
> -    if (qemu_init_main_loop(&main_loop_err)) {
> -        error_report_err(main_loop_err);
> -        exit(1);
> -    }
> -
>  #ifdef CONFIG_SECCOMP
>      olist = qemu_find_opts_err("sandbox", NULL);
>      if (olist) {
> -- 
> 2.20.1