On Thu, Oct 19, 2017 at 09:42:18PM +1100, David Gibson wrote: > 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.
I agree. The number of externally visible pause states should be as small as possible. > 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? That would hide the internal details from the outside. Sounds nice, but adding new machine/device configuration QMP commands while hiding the QEMU state from the outside sounds impossible. For example, if we use -S today, this works: $ qemu-system-x86_64 -S -qmp stdio <- {"QMP": {"version": {"qemu": {"micro": 0, "minor": 10, "major": 2}, "package": " (v2.10.0-83-g9375da7831)"}, "capabilities": []}} -> {"execute":"qmp_capabilities"} <- {"return": {}} -> {"execute":"query-cpus"} <- {"return": [{"arch": "x86", "current": true, "props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "CPU": 0, "qom_path": "/machine/unattached/device[0]", "pc": 4294967280, "halted": false, "thread_id": 4038}]} This means "query-cpus" needs to fast-forward to the CPU creation stage if we want to keep compatibility. Now, assume we add a set-numa-node command like the one in this series. e.g.: $ qemu-system-x86_64 -S -qmp stdio <- {"QMP": {"version": {"qemu": {"micro": 0, "minor": 10, "major": 2}, "package": " (v2.10.0-83-g9375da7831)"}, "capabilities": []}} -> {"execute":"qmp_capabilities"} <- {"return": {}} -> {"execute":"set-numa-node" ... } <- {"return": ...} The command will work only if machine initialization didn't run yet. But now an innocent-looking query command would change QEMU state in an unexpected way: $ qemu-system-x86_64 -S -qmp stdio <- {"QMP": {"version": {"qemu": {"micro": 0, "minor": 10, "major": 2}, "package": " (v2.10.0-83-g9375da7831)"}, "capabilities": []}} -> {"execute":"qmp_capabilities"} <- {"return": {}} -> {"execute":"query-cpus"} [will silently fast-forward QEMU state] <- {"return": [{"arch": "x86", "current": true, "props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "CPU": 0, "qom_path": "/machine/unattached/device[0]", "pc": 4294967280, "halted": false, "thread_id": 4038}]} -> {"execute":"set-numa-node" ... } <- {"error": ...} [the command will fail because the machine was already created] This means we do have a externally visible "too late to use set-numa-node" QEMU state, and query-cpus will have a externally visible side effect. Every QMP command would need to document how it affects QEMU state in a externally visible way. If QEMU pause state is still going to be externally visible this way, I would prefer to let the client to explicitly tell what's the state they want QEMU to be, instead of making QEMU change state silently as a side effect of QMP commands. > [...] -- Eduardo