On Mon, Oct 16, 2017 at 02:59:16PM -0200, Eduardo Habkost wrote:
> On Mon, Oct 16, 2017 at 06:22:54PM +0200, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > ---
> > include/sysemu/sysemu.h | 1 +
> > qemu-options.hx | 15 ++++++++++++++
> > qmp.c | 5 +++++
> > vl.c | 54
> > ++++++++++++++++++++++++++++++++++++++++++++++++-
> > 4 files changed, 74 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index b213696..3feb94f 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -66,6 +66,7 @@ typedef enum WakeupReason {
> > QEMU_WAKEUP_REASON_OTHER,
> > } WakeupReason;
> >
> > +void qemu_exit_preconfig_request(void);
> > void qemu_system_reset_request(ShutdownCause reason);
> > void qemu_system_suspend_request(void);
> > void qemu_register_suspend_notifier(Notifier *notifier);
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 39225ae..bd44db8 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -3498,6 +3498,21 @@ STEXI
> > Run the emulation in single step mode.
> > ETEXI
> >
> > +DEF("paused", HAS_ARG, QEMU_OPTION_paused, \
> > + "-paused [state=]postconf|preconf\n"
> > + " postconf: pause QEMU after machine is initialized\n"
> > + " preconf: pause QEMU before machine is initialized\n",
> > + QEMU_ARCH_ALL)
>
> I would like to allow pausing before machine-type is selected, so
> management could run query-machines before choosing a
> machine-type. Would that need a third "-pause" mode, or will we
> be able to change "preconf" to pause before select_machine() is
> called?
>
> The same probably applies to other things initialized before
> machine_run_board_init() that could be configurable using QMP,
> including but not limited to:
> * Accelerator configuration
> * Registering global properties
> * RAM size
> * SMP/CPU configuration
Yeah.. having a bunch of different possible pause stages to select
doesn't sound great. Could we avoid this by instead changing -S to
pause at the earliest possible spot, but having any monitor commands
that require a later stage automatically "fast forwarding" to the
right phase?
>
>
> > +STEXI
> > +@item -paused
> > +@findex -paused
> > +if set enabled interactive configuration stages before machine emulation
> > starts.
> > +'postconf' option value mimics -S option behaviour where machine is created
> > +but emulation isn't started. 'preconf' option value pauses QEMU before
> > machine
> > +is created, which allows to query and configure properties affecting
> > machine
> > +initialization. Use monitor/QMP command 'cont' to go to exit paused state.
>
> What if "-S" is used at the same time"? Will "cont" only
> initialize the machine and wait for another "cont" command to
> start the VCPUs, or will it unpause everything?
>
>
> > +ETEXI
> > +
> > DEF("S", 0, QEMU_OPTION_S, \
> > "-S freeze CPU at startup (use 'c' to start execution)\n",
> > QEMU_ARCH_ALL)
> > diff --git a/qmp.c b/qmp.c
> > index e8c3031..49e9a5c 100644
> > --- a/qmp.c
> > +++ b/qmp.c
> > @@ -167,6 +167,11 @@ void qmp_cont(Error **errp)
> > BlockBackend *blk;
> > Error *local_err = NULL;
> >
> > + if (runstate_check(RUN_STATE_PRELAUNCH)) {
> > + qemu_exit_preconfig_request();
> > + return;
> > + }
> > +
> > /* if there is a dump in background, we should wait until the dump
> > * finished */
> > if (dump_in_progress()) {
> > diff --git a/vl.c b/vl.c
> > index 3fed457..30631fd 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -555,6 +555,20 @@ static QemuOptsList qemu_fw_cfg_opts = {
> > },
> > };
> >
> > +static QemuOptsList qemu_paused_opts = {
> > + .name = "paused",
> > + .implied_opt_name = "state",
> > + .head = QTAILQ_HEAD_INITIALIZER(qemu_paused_opts.head),
> > + .desc = {
> > + {
> > + .name = "state",
> > + .type = QEMU_OPT_STRING,
> > + .help = "Pause state of QEMU on startup",
> > + },
> > + { /* end of list */ }
> > + },
> > +};
> > +
> > /**
> > * Get machine options
> > *
> > @@ -1689,6 +1703,11 @@ static pid_t shutdown_pid;
> > static int powerdown_requested;
> > static int debug_requested;
> > static int suspend_requested;
> > +static enum {
> > + PRECONFIG_CONT = 0,
> > + PRECONFIG_PAUSE,
> > + PRECONFIG_SKIP,
> > +} preconfig_requested;
> > static WakeupReason wakeup_reason;
> > static NotifierList powerdown_notifiers =
> > NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
> > @@ -1773,6 +1792,11 @@ static int qemu_debug_requested(void)
> > return r;
> > }
> >
> > +void qemu_exit_preconfig_request(void)
> > +{
> > + preconfig_requested = PRECONFIG_CONT;
> > +}
> > +
> > /*
> > * Reset the VM. Issue an event unless @reason is SHUTDOWN_CAUSE_NONE.
> > */
> > @@ -1939,6 +1963,12 @@ static bool main_loop_should_exit(void)
> > RunState r;
> > ShutdownCause request;
> >
> > + if (runstate_check(RUN_STATE_PRELAUNCH)) {
> > + if (preconfig_requested == PRECONFIG_CONT) {
> > + preconfig_requested = PRECONFIG_SKIP;
> > + return true;
> > + }
> > + }
> > if (qemu_debug_requested()) {
> > vm_stop(RUN_STATE_DEBUG);
> > }
> > @@ -3177,6 +3207,7 @@ int main(int argc, char **argv, char **envp)
> > qemu_add_opts(&qemu_icount_opts);
> > qemu_add_opts(&qemu_semihosting_config_opts);
> > qemu_add_opts(&qemu_fw_cfg_opts);
> > + qemu_add_opts(&qemu_paused_opts);
> > module_call_init(MODULE_INIT_OPTS);
> >
> > runstate_init();
> > @@ -3845,6 +3876,26 @@ int main(int argc, char **argv, char **envp)
> > exit(1);
> > }
> > break;
> > + case QEMU_OPTION_paused:
> > + {
> > + const char *value;
> > +
> > + opts =
> > qemu_opts_parse_noisily(qemu_find_opts("paused"),
> > + optarg, true);
> > + if (opts == NULL) {
> > + exit(1);
> > + }
> > + value = qemu_opt_get(opts, "state");
> > + if (!strcmp(value, "postconf")) {
> > + autostart = 0;
> > + } else if (!strcmp(value, "preconf")) {
> > + preconfig_requested = PRECONFIG_PAUSE;
> > + } else {
> > + error_report("incomplete '-paused' option\n");
> > + exit(1);
> > + }
> > + break;
> > + }
> > case QEMU_OPTION_enable_kvm:
> > olist = qemu_find_opts("machine");
> > qemu_opts_parse_noisily(olist, "accel=kvm", false);
> > @@ -4731,7 +4782,6 @@ int main(int argc, char **argv, char **envp)
> > current_machine->boot_order = boot_order;
> > current_machine->cpu_model = cpu_model;
> >
> > -
> > /* parse features once if machine provides default cpu_type */
> > if (machine_class->default_cpu_type) {
> > current_machine->cpu_type = machine_class->default_cpu_type;
> > @@ -4741,6 +4791,8 @@ int main(int argc, char **argv, char **envp)
> > }
> > }
> >
> > + main_loop(); /* do monitor/qmp handling at preconfig state if
> > requested */
> > +
>
> I'm impressed by the simplicity of the implementation. I though
> this would involve moving everything between this line and the
> next main_loop() call outside main(), so they would be called by
> qmp_cont().
>
> Any expert on GLib's Event Loop sees any gotcha in this method?
>
> I would like to do a careful review of main_loop_wait() and
> main_loop_should_exit(), to ensure those functions don't depend
> on anything that's initialized after this line. Probably a few
> existing QMP commands can crash if machine is not initialized
> yet?
>
> The rules and expectations on initialization ordering are very
> subtle, I suggest including test code for the new feature to
> ensure nothing crashes or breaks in the future.
>
>
> > machine_run_board_init(current_machine);
> >
> > realtime_init();
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
