| Message ID | 20201027182144.3315885-23-pbonzini@redhat.com |
|---|---|
| State | Accepted |
| Commit | 7691bdef693c04e2a0c40846cda089c28016e043 |
| Headers | show |
| Series | cleanup qemu_init and make sense of command line processing | expand |
On Tue, 27 Oct 2020 14:21:37 -0400 Paolo Bonzini <pbonzini@redhat.com> wrote: > Displays should be available before the monitor starts, so that > it is possible to use the graphical console to interact with > the monitor itself. > > This patch is quite ugly, but all this is temporary. The double > call to qemu_init_displays will go away as soon we can unify machine > initialization between the preconfig and "normal" flows, and so will > the preconfig_exit_requested variable (that is only preconfig_requested > remains). > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Doesn't apply to yer for-6.0 branch > --- > softmmu/vl.c | 58 ++++++++++++++++++++++++++++++++-------------------- > 1 file changed, 36 insertions(+), 22 deletions(-) > > diff --git a/softmmu/vl.c b/softmmu/vl.c > index 1485aba8be..a46f1b9164 100644 > --- a/softmmu/vl.c > +++ b/softmmu/vl.c > @@ -137,6 +137,7 @@ static ram_addr_t maxram_size; > static uint64_t ram_slots; > static int display_remote; > static int snapshot; > +static bool preconfig_requested; > static QemuPluginList plugin_list = QTAILQ_HEAD_INITIALIZER(plugin_list); > static BlockdevOptionsQueue bdo_queue = QSIMPLEQ_HEAD_INITIALIZER(bdo_queue); > static bool nographic = false; > @@ -3210,12 +3211,12 @@ static void qemu_validate_options(void) > } > } > > - if (loadvm && !preconfig_exit_requested) { > + if (loadvm && preconfig_requested) { > error_report("'preconfig' and 'loadvm' options are " > "mutually exclusive"); > exit(EXIT_FAILURE); > } > - if (incoming && !preconfig_exit_requested) { > + if (incoming && preconfig_requested) { > error_report("'preconfig' and 'incoming' options are " > "mutually exclusive"); > exit(EXIT_FAILURE); > @@ -3381,6 +3382,28 @@ static void qemu_init_subsystems(void) > socket_init(); > } > > +static void qemu_init_displays(void) > +{ > + DisplayState *ds; > + > + /* init local displays */ > + ds = init_displaystate(); > + qemu_display_init(ds, &dpy); > + > + /* must be after terminal init, SDL library changes signal handlers */ > + os_setup_signal_handling(); > + > + /* init remote displays */ > +#ifdef CONFIG_VNC > + qemu_opts_foreach(qemu_find_opts("vnc"), > + vnc_init_func, NULL, &error_fatal); > +#endif > + > + if (using_spice) { > + qemu_spice.display_init(); > + } > +} > + > /* > * Called after leaving preconfig state. From here on runstate is > * RUN_STATE_PRELAUNCH or RUN_STATE_INMIGRATE. > @@ -3449,8 +3472,6 @@ static void qemu_create_cli_devices(void) > > static void qemu_machine_creation_done(void) > { > - DisplayState *ds; > - > cpu_synchronize_all_post_init(); > > /* Did we create any drives that we failed to create a device for? */ > @@ -3473,23 +3494,6 @@ static void qemu_machine_creation_done(void) > qemu_register_reset(restore_boot_order, g_strdup(boot_order)); > } > > - /* init local displays */ > - ds = init_displaystate(); > - qemu_display_init(ds, &dpy); > - > - /* must be after terminal init, SDL library changes signal handlers */ > - os_setup_signal_handling(); > - > - /* init remote displays */ > -#ifdef CONFIG_VNC > - qemu_opts_foreach(qemu_find_opts("vnc"), > - vnc_init_func, NULL, &error_fatal); > -#endif > - > - if (using_spice) { > - qemu_spice.display_init(); > - } > - > if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) { > exit(1); > } > @@ -4094,6 +4098,7 @@ void qemu_init(int argc, char **argv, char **envp) > break; > case QEMU_OPTION_preconfig: > preconfig_exit_requested = false; > + preconfig_requested = true; > break; > case QEMU_OPTION_enable_kvm: > olist = qemu_find_opts("machine"); > @@ -4515,11 +4520,20 @@ void qemu_init(int argc, char **argv, char **envp) > qemu_resolve_machine_memdev(); > parse_numa_opts(current_machine); > > + if (preconfig_requested) { > + qemu_init_displays(); > + } > + > /* do monitor/qmp handling at preconfig state if requested */ > qemu_main_loop(); > - > qemu_finish_machine_init(); > + > qemu_create_cli_devices(); > + > + /* initialize displays after all errors have been reported */ > + if (!preconfig_requested) { > + qemu_init_displays(); > + } > qemu_machine_creation_done(); > > if (loadvm) {
On 20/11/20 16:11, Igor Mammedov wrote: > On Tue, 27 Oct 2020 14:21:37 -0400 > Paolo Bonzini <pbonzini@redhat.com> wrote: > >> Displays should be available before the monitor starts, so that >> it is possible to use the graphical console to interact with >> the monitor itself. >> >> This patch is quite ugly, but all this is temporary. The double >> call to qemu_init_displays will go away as soon we can unify machine >> initialization between the preconfig and "normal" flows, and so will >> the preconfig_exit_requested variable (that is only preconfig_requested >> remains). >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > Doesn't apply to yer for-6.0 branch I updated the branch. Thanks, Paolo >> --- >> softmmu/vl.c | 58 ++++++++++++++++++++++++++++++++-------------------- >> 1 file changed, 36 insertions(+), 22 deletions(-) >> >> diff --git a/softmmu/vl.c b/softmmu/vl.c >> index 1485aba8be..a46f1b9164 100644 >> --- a/softmmu/vl.c >> +++ b/softmmu/vl.c >> @@ -137,6 +137,7 @@ static ram_addr_t maxram_size; >> static uint64_t ram_slots; >> static int display_remote; >> static int snapshot; >> +static bool preconfig_requested; >> static QemuPluginList plugin_list = QTAILQ_HEAD_INITIALIZER(plugin_list); >> static BlockdevOptionsQueue bdo_queue = QSIMPLEQ_HEAD_INITIALIZER(bdo_queue); >> static bool nographic = false; >> @@ -3210,12 +3211,12 @@ static void qemu_validate_options(void) >> } >> } >> >> - if (loadvm && !preconfig_exit_requested) { >> + if (loadvm && preconfig_requested) { >> error_report("'preconfig' and 'loadvm' options are " >> "mutually exclusive"); >> exit(EXIT_FAILURE); >> } >> - if (incoming && !preconfig_exit_requested) { >> + if (incoming && preconfig_requested) { >> error_report("'preconfig' and 'incoming' options are " >> "mutually exclusive"); >> exit(EXIT_FAILURE); >> @@ -3381,6 +3382,28 @@ static void qemu_init_subsystems(void) >> socket_init(); >> } >> >> +static void qemu_init_displays(void) >> +{ >> + DisplayState *ds; >> + >> + /* init local displays */ >> + ds = init_displaystate(); >> + qemu_display_init(ds, &dpy); >> + >> + /* must be after terminal init, SDL library changes signal handlers */ >> + os_setup_signal_handling(); >> + >> + /* init remote displays */ >> +#ifdef CONFIG_VNC >> + qemu_opts_foreach(qemu_find_opts("vnc"), >> + vnc_init_func, NULL, &error_fatal); >> +#endif >> + >> + if (using_spice) { >> + qemu_spice.display_init(); >> + } >> +} >> + >> /* >> * Called after leaving preconfig state. From here on runstate is >> * RUN_STATE_PRELAUNCH or RUN_STATE_INMIGRATE. >> @@ -3449,8 +3472,6 @@ static void qemu_create_cli_devices(void) >> >> static void qemu_machine_creation_done(void) >> { >> - DisplayState *ds; >> - >> cpu_synchronize_all_post_init(); >> >> /* Did we create any drives that we failed to create a device for? */ >> @@ -3473,23 +3494,6 @@ static void qemu_machine_creation_done(void) >> qemu_register_reset(restore_boot_order, g_strdup(boot_order)); >> } >> >> - /* init local displays */ >> - ds = init_displaystate(); >> - qemu_display_init(ds, &dpy); >> - >> - /* must be after terminal init, SDL library changes signal handlers */ >> - os_setup_signal_handling(); >> - >> - /* init remote displays */ >> -#ifdef CONFIG_VNC >> - qemu_opts_foreach(qemu_find_opts("vnc"), >> - vnc_init_func, NULL, &error_fatal); >> -#endif >> - >> - if (using_spice) { >> - qemu_spice.display_init(); >> - } >> - >> if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) { >> exit(1); >> } >> @@ -4094,6 +4098,7 @@ void qemu_init(int argc, char **argv, char **envp) >> break; >> case QEMU_OPTION_preconfig: >> preconfig_exit_requested = false; >> + preconfig_requested = true; >> break; >> case QEMU_OPTION_enable_kvm: >> olist = qemu_find_opts("machine"); >> @@ -4515,11 +4520,20 @@ void qemu_init(int argc, char **argv, char **envp) >> qemu_resolve_machine_memdev(); >> parse_numa_opts(current_machine); >> >> + if (preconfig_requested) { >> + qemu_init_displays(); >> + } >> + >> /* do monitor/qmp handling at preconfig state if requested */ >> qemu_main_loop(); >> - >> qemu_finish_machine_init(); >> + >> qemu_create_cli_devices(); >> + >> + /* initialize displays after all errors have been reported */ >> + if (!preconfig_requested) { >> + qemu_init_displays(); >> + } >> qemu_machine_creation_done(); >> >> if (loadvm) { >
On Fri, 20 Nov 2020 16:53:41 +0100 Paolo Bonzini <pbonzini@redhat.com> wrote: > On 20/11/20 16:11, Igor Mammedov wrote: > > On Tue, 27 Oct 2020 14:21:37 -0400 > > Paolo Bonzini <pbonzini@redhat.com> wrote: > > > >> Displays should be available before the monitor starts, so that > >> it is possible to use the graphical console to interact with > >> the monitor itself. > >> > >> This patch is quite ugly, but all this is temporary. The double > >> call to qemu_init_displays will go away as soon we can unify machine > >> initialization between the preconfig and "normal" flows, and so will > >> the preconfig_exit_requested variable (that is only preconfig_requested > >> remains). > >> > >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > > > Doesn't apply to yer for-6.0 branch > > I updated the branch. it probably won't help, what I do review/test is drop these patches on branch and reapply them from this thread. Having v2 on list that applies to master would be better. > Thanks, > > Paolo > [...]
On 20/11/20 17:32, Igor Mammedov wrote: > On Fri, 20 Nov 2020 16:53:41 +0100 > Paolo Bonzini <pbonzini@redhat.com> wrote: > >> On 20/11/20 16:11, Igor Mammedov wrote: >>> On Tue, 27 Oct 2020 14:21:37 -0400 >>> Paolo Bonzini <pbonzini@redhat.com> wrote: >>> >>>> Displays should be available before the monitor starts, so that >>>> it is possible to use the graphical console to interact with >>>> the monitor itself. >>>> >>>> This patch is quite ugly, but all this is temporary. The double >>>> call to qemu_init_displays will go away as soon we can unify machine >>>> initialization between the preconfig and "normal" flows, and so will >>>> the preconfig_exit_requested variable (that is only preconfig_requested >>>> remains). >>>> >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> >>> Doesn't apply to yer for-6.0 branch >> >> I updated the branch. > > it probably won't help, > what I do review/test is drop these patches on branch > and reapply them from this thread. > Having v2 on list that applies to master would be better. Yes, of course. I meant that the for-6.0 branch already _is_ the v2, I'll send it out as soon as I integrate all your feedback. Thanks, Paolo
On Fri, 20 Nov 2020 17:46:14 +0100 Paolo Bonzini <pbonzini@redhat.com> wrote: > On 20/11/20 17:32, Igor Mammedov wrote: > > On Fri, 20 Nov 2020 16:53:41 +0100 > > Paolo Bonzini <pbonzini@redhat.com> wrote: > > > >> On 20/11/20 16:11, Igor Mammedov wrote: > >>> On Tue, 27 Oct 2020 14:21:37 -0400 > >>> Paolo Bonzini <pbonzini@redhat.com> wrote: > >>> > >>>> Displays should be available before the monitor starts, so that > >>>> it is possible to use the graphical console to interact with > >>>> the monitor itself. > >>>> > >>>> This patch is quite ugly, but all this is temporary. The double > >>>> call to qemu_init_displays will go away as soon we can unify machine > >>>> initialization between the preconfig and "normal" flows, and so will > >>>> the preconfig_exit_requested variable (that is only preconfig_requested > >>>> remains). > >>>> > >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > >>> > >>> Doesn't apply to yer for-6.0 branch > >> > >> I updated the branch. > > > > it probably won't help, > > what I do review/test is drop these patches on branch > > and reapply them from this thread. > > Having v2 on list that applies to master would be better. > > Yes, of course. I meant that the for-6.0 branch already _is_ the v2, > I'll send it out as soon as I integrate all your feedback. Thanks for trying to make hornets nest (vl.c) in something manageable and less scary to touch. I'll try to review v2 faster (there is not much of it left by now) > > Thanks, > > Paolo > >
diff --git a/softmmu/vl.c b/softmmu/vl.c index 1485aba8be..a46f1b9164 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -137,6 +137,7 @@ static ram_addr_t maxram_size; static uint64_t ram_slots; static int display_remote; static int snapshot; +static bool preconfig_requested; static QemuPluginList plugin_list = QTAILQ_HEAD_INITIALIZER(plugin_list); static BlockdevOptionsQueue bdo_queue = QSIMPLEQ_HEAD_INITIALIZER(bdo_queue); static bool nographic = false; @@ -3210,12 +3211,12 @@ static void qemu_validate_options(void) } } - if (loadvm && !preconfig_exit_requested) { + if (loadvm && preconfig_requested) { error_report("'preconfig' and 'loadvm' options are " "mutually exclusive"); exit(EXIT_FAILURE); } - if (incoming && !preconfig_exit_requested) { + if (incoming && preconfig_requested) { error_report("'preconfig' and 'incoming' options are " "mutually exclusive"); exit(EXIT_FAILURE); @@ -3381,6 +3382,28 @@ static void qemu_init_subsystems(void) socket_init(); } +static void qemu_init_displays(void) +{ + DisplayState *ds; + + /* init local displays */ + ds = init_displaystate(); + qemu_display_init(ds, &dpy); + + /* must be after terminal init, SDL library changes signal handlers */ + os_setup_signal_handling(); + + /* init remote displays */ +#ifdef CONFIG_VNC + qemu_opts_foreach(qemu_find_opts("vnc"), + vnc_init_func, NULL, &error_fatal); +#endif + + if (using_spice) { + qemu_spice.display_init(); + } +} + /* * Called after leaving preconfig state. From here on runstate is * RUN_STATE_PRELAUNCH or RUN_STATE_INMIGRATE. @@ -3449,8 +3472,6 @@ static void qemu_create_cli_devices(void) static void qemu_machine_creation_done(void) { - DisplayState *ds; - cpu_synchronize_all_post_init(); /* Did we create any drives that we failed to create a device for? */ @@ -3473,23 +3494,6 @@ static void qemu_machine_creation_done(void) qemu_register_reset(restore_boot_order, g_strdup(boot_order)); } - /* init local displays */ - ds = init_displaystate(); - qemu_display_init(ds, &dpy); - - /* must be after terminal init, SDL library changes signal handlers */ - os_setup_signal_handling(); - - /* init remote displays */ -#ifdef CONFIG_VNC - qemu_opts_foreach(qemu_find_opts("vnc"), - vnc_init_func, NULL, &error_fatal); -#endif - - if (using_spice) { - qemu_spice.display_init(); - } - if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) { exit(1); } @@ -4094,6 +4098,7 @@ void qemu_init(int argc, char **argv, char **envp) break; case QEMU_OPTION_preconfig: preconfig_exit_requested = false; + preconfig_requested = true; break; case QEMU_OPTION_enable_kvm: olist = qemu_find_opts("machine"); @@ -4515,11 +4520,20 @@ void qemu_init(int argc, char **argv, char **envp) qemu_resolve_machine_memdev(); parse_numa_opts(current_machine); + if (preconfig_requested) { + qemu_init_displays(); + } + /* do monitor/qmp handling at preconfig state if requested */ qemu_main_loop(); - qemu_finish_machine_init(); + qemu_create_cli_devices(); + + /* initialize displays after all errors have been reported */ + if (!preconfig_requested) { + qemu_init_displays(); + } qemu_machine_creation_done(); if (loadvm) {
Displays should be available before the monitor starts, so that it is possible to use the graphical console to interact with the monitor itself. This patch is quite ugly, but all this is temporary. The double call to qemu_init_displays will go away as soon we can unify machine initialization between the preconfig and "normal" flows, and so will the preconfig_exit_requested variable (that is only preconfig_requested remains). Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- softmmu/vl.c | 58 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 22 deletions(-)