* [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP
@ 2018-04-27 15:05 Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 01/11] numa: postpone options post-processing till machine_run_board_init() Igor Mammedov
` (10 more replies)
0 siblings, 11 replies; 25+ messages in thread
From: Igor Mammedov @ 2018-04-27 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, ehabkost, pkrempa, armbru
v5->v6:
* add exit-preconfig QMP command instead of overloading meaning of 'cont' command
* add doc text to qemu-tech.texi about -S and --preconfig
* add numa configuration example into commit message of 10/11
* limit set-numa-node QMP command to preconfig mode
v4->v5:
* rebase on top of OOB changes that's now in master
* s/qobject_to_qdict(/qobject_to(QDict,/
* s/-preconfig/--preconfig/
* s/2.12/2.13/
* s/parse_NumaOptions/set_numa_options/
* drop if (err) guard around error_propagate()
* move QCO_ALLOWED_IN_PRECONFIG and do_qmp_dispatch() runstate check
from the later patch 'qapi: introduce new cmd option "allowed-in-preconfig"'
to here for better bissectability
* add TODO comment to '{ RUN_STATE_PRECONFIG, RUN_STATE_INMIGRATE }' transition
* check for incoming && preconfig outside of option parsing loop
* add 'use QMP instead" to error message, suggesting user the right interface to use
* allow query-command-line-options in preconfig state
* make sure that allowed-in-preconfig could be set only to True
* move out QCO_ALLOWED_IN_PRECONFIG check in do_qmp_dispatch() to
earlier 'cli: add --preconfig option' patch
* 2 extra test patches
'tests: let qapi-schema tests detect allowed-in-preconfig'
'tests: add allowed-in-preconfig-test for qapi-schema'
v3->v4:
* replace 'runstates' list in QMP command with a single
boolean 'ption allowed-in-preconfig' like it's done with
'allow-oob'. Which allows to simplify intrusive QAPI
changes quite a lot. (Eric Blake <eblake@redhat.com>)
* Make sure HMP is disbled for real, v3 was just printing
error mesage but allowing command to be executed
("Dr. David Alan Gilbert" <dgilbert@redhat.com>)
* Improve "cli: add -preconfig option" commit message,
explain a bit more on semantics of new state/option.
* ithe rest of minor fixups suggested at v3 review
(Eric Blake <eblake@redhat.com>)
PS:
havn't impl. test for new option in
tests/qapi-schema/qapi-schema-test.json yet,
can do it on top if approach is acceptable.
v1->v3:
* introduce PRECONFIG runstate with -preconfig option.
it's cleaner to manage transitions and do checks
than reusing existing PRELAUNCH state.
* extend QAPI schema commands with 'runstates' keyword,
so that it would be possible to specify in command definition
when it is valid to execute.
(python changes a bit hackery, since I have little to
no idea how it should work)
* add preconfig QMP and set-numa-node tests
* make mutually exclusive -preconfig and -incoming options,
for simplicity sake. Shouldn't be problem as target can
be starter with pure CLI, since mapping on source is
already known.
* Drop HMP part and leave only QMP in preconfig state.
Currently it's problematic to configure NUMA mapping for CPUs using "-numa cpu="
option, without restarting QEMU, first is to query board specific CPU layout
with query-hotpluggable-cpus QMP command (which is function of -M and -smp CLI
options). Mgmt side isn't happy with restarting QEMU to query configuration
parameters first and then run actual instance, so it keeps using old cpu_index
based option '-numa cpus' instead of new socket/core/thread-id based one.
Introduce a new '--preconfig' CLI option which allows pausing QEMU before
machine_init() is run (preconfig state). So it would be possible to query
possible CPUs layout, configure NUMA mapping based on query result with
set-numa-node QMP command and continue executing without restarting QEMU.
Difference between new --preconfig pause point vs -S is that the later pauses
QEMU after machine is constructed and ready to run guest code or in process of
incoming migration (essentially machine is in some running state (with paused
VCPUs) and any action on it is considered as hotplug). At this point it's hard
to configure or reconfigure parameters that affect machine_init() and later
stages. While the new --preconfig option pauses QEMU instance before
machine_init() and would allow to configure parameters as if doing it from CLI
but in interactve manner using QMP interface, which would allow introspecting
and configuring QEMU instance without restarting it.
Initially only limited set of commands (that are ready to work with non
initialized machine or without it) are allowed in preconfig state:
#existing commands:
qmp_capabilities
query-qmp-schema
query-commands
query-command-line-options
query-status
query-hotpluggable-cpus
#new commands
exit-preconfig
set-numa-node
Which commands are allowed at preconfig state, is defined by QAPI Schema
with help of optional 'allowed-in-preconfig' flag in command definition,
which allows users to get list of commands it can run with help of
query-qmp-schema command.
Once user done with configuration at preconfig state, it could be exited with
exit-preconfig command, in which case QEMU would proceed with VM intialization
and either start guest execution or pause in prelaunch state if -S CLI option
were specified along with --prelaunch on startup.
PS:
Later we can modify other commands to run early, for example device_add. So
that devices would be added in cold-plug context, instead of getting device
'hotplugged' with followed up fixups and workarounds to make it look like
cold-plugged.
Example of configuration CPU's NUMA mapping with --preconfig:
$QEMU -smp 2 --preconfig ...
QMP:
# get CPUs layout for current target/machine/CLI
-> {'execute': 'query-hotpluggable-cpus' }
<- {'return': [
{'props': {'core-id': 0, 'thread-id': 0, 'socket-id': 1}, ... },
{'props': {'core-id': 0, 'thread-id': 0, 'socket-id': 0}, ... }
]}
# configure 1st node
-> {'execute': 'set-numa-node', 'arguments': { 'type': 'node', 'nodeid': 0 } }
<- {'return': {}}
-> {'execute': 'set-numa-node', 'arguments': { 'type': 'cpu',
'node-id': 0, 'core-id': 0, 'thread-id': 0, 'socket-id': 1, }
}
<- {'return': {}}
# configure 2nd node
-> {'execute': 'set-numa-node', 'arguments': { 'type': 'node', 'nodeid': 1 } }
-> {'execute': 'set-numa-node', 'arguments': { 'type': 'cpu',
'node-id': 1, 'core-id': 0, 'thread-id': 0, 'socket-id': 0 }
}
<- {'return': {}}
# [optional] verify configuration
-> {'execute': 'query-hotpluggable-cpus' }
<- {'return': [
{'props': {'core-id': 0, 'thread-id': 0, 'node-id': 0, 'socket-id': 1}, ... },
{'props': {'core-id': 0, 'thread-id': 0, 'node-id': 1, 'socket-id': 0}, ... }
]}
Git tree:
https://github.com/imammedo/qemu.git qmp_preconfig_v6
CC: eblake@redhat.com
CC: ehabkost@redhat.com
CC: pkrempa@redhat.com
CC: armbru@redhat.com
Igor Mammedov (11):
numa: postpone options post-processing till machine_run_board_init()
numa: split out NumaOptions parsing into set_numa_options()
cli: add --preconfig option
hmp: disable monitor in preconfig state
qapi: introduce new cmd option "allowed-in-preconfig"
tests: let qapi-schema tests detect allowed-in-preconfig
tests: add allowed-in-preconfig-test for qapi-schema
tests: extend qmp test with preconfig checks
qmp: permit query-hotpluggable-cpus in preconfig state
qmp: add set-numa-node command
tests: functional tests for QMP command set-numa-node
include/qapi/qmp/dispatch.h | 1 +
include/sysemu/numa.h | 2 +
include/sysemu/sysemu.h | 1 +
tests/libqtest.h | 9 +++
docs/devel/qapi-code-gen.txt | 10 +++-
hw/core/machine.c | 5 +-
monitor.c | 11 +++-
numa.c | 70 ++++++++++++++++--------
qapi/introspect.json | 5 +-
qapi/misc.json | 48 ++++++++++++++--
qapi/qmp-dispatch.c | 8 +++
qapi/run-state.json | 8 ++-
qemu-options.hx | 13 +++++
qemu-tech.texi | 40 ++++++++++++++
qmp.c | 10 ++++
scripts/qapi/commands.py | 12 ++--
scripts/qapi/common.py | 19 ++++---
scripts/qapi/doc.py | 4 +-
scripts/qapi/introspect.py | 7 ++-
tests/Makefile.include | 1 +
tests/libqtest.c | 7 +++
tests/numa-test.c | 61 +++++++++++++++++++++
tests/qapi-schema/allowed-in-preconfig-test.err | 1 +
tests/qapi-schema/allowed-in-preconfig-test.exit | 1 +
tests/qapi-schema/allowed-in-preconfig-test.json | 2 +
tests/qapi-schema/allowed-in-preconfig-test.out | 0
tests/qapi-schema/doc-good.out | 4 +-
tests/qapi-schema/ident-with-escape.out | 2 +-
tests/qapi-schema/indented-expr.out | 4 +-
tests/qapi-schema/qapi-schema-test.json | 3 +
tests/qapi-schema/qapi-schema-test.out | 22 ++++----
tests/qapi-schema/test-qapi.py | 8 +--
tests/qmp-test.c | 37 +++++++++++++
tests/test-qmp-cmds.c | 4 ++
vl.c | 35 +++++++++++-
35 files changed, 403 insertions(+), 72 deletions(-)
create mode 100644 tests/qapi-schema/allowed-in-preconfig-test.err
create mode 100644 tests/qapi-schema/allowed-in-preconfig-test.exit
create mode 100644 tests/qapi-schema/allowed-in-preconfig-test.json
create mode 100644 tests/qapi-schema/allowed-in-preconfig-test.out
--
2.7.4
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 01/11] numa: postpone options post-processing till machine_run_board_init()
2018-04-27 15:05 [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP Igor Mammedov
@ 2018-04-27 15:05 ` Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 02/11] numa: split out NumaOptions parsing into set_numa_options() Igor Mammedov
` (9 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2018-04-27 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, ehabkost, pkrempa, armbru
in preparation for numa options to being handled via QMP before
machine_run_board_init(), move final numa configuration checks
and processing to machine_run_board_init() so it could take into
account both CLI (via parse_numa_opts()) and QMP input
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
v2:
- remove duplicate qemu_opts_foreach() in numa_complete_configuration()
that was causing non explicitly IDed node "-numa node" parsed twice.
---
include/sysemu/numa.h | 1 +
hw/core/machine.c | 5 +++--
numa.c | 13 ++++++++-----
3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index d99e547..21713b7 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -23,6 +23,7 @@ struct NumaNodeMem {
extern NodeInfo numa_info[MAX_NODES];
void parse_numa_opts(MachineState *ms);
+void numa_complete_configuration(MachineState *ms);
void query_numa_node_mem(NumaNodeMem node_mem[]);
extern QemuOptsList qemu_numa_opts;
void numa_legacy_auto_assign_ram(MachineClass *mc, NodeInfo *nodes,
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2040177..617e5f8 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -737,7 +737,7 @@ static char *cpu_slot_to_string(const CPUArchId *cpu)
return g_string_free(s, false);
}
-static void machine_numa_finish_init(MachineState *machine)
+static void machine_numa_finish_cpu_init(MachineState *machine)
{
int i;
bool default_mapping;
@@ -792,7 +792,8 @@ void machine_run_board_init(MachineState *machine)
MachineClass *machine_class = MACHINE_GET_CLASS(machine);
if (nb_numa_nodes) {
- machine_numa_finish_init(machine);
+ numa_complete_configuration(machine);
+ machine_numa_finish_cpu_init(machine);
}
/* If the machine supports the valid_cpu_types check and the user
diff --git a/numa.c b/numa.c
index 78a869e..d0abd7c 100644
--- a/numa.c
+++ b/numa.c
@@ -338,15 +338,11 @@ void numa_default_auto_assign_ram(MachineClass *mc, NodeInfo *nodes,
nodes[i].node_mem = size - usedmem;
}
-void parse_numa_opts(MachineState *ms)
+void numa_complete_configuration(MachineState *ms)
{
int i;
MachineClass *mc = MACHINE_GET_CLASS(ms);
- if (qemu_opts_foreach(qemu_find_opts("numa"), parse_numa, ms, NULL)) {
- exit(1);
- }
-
/*
* If memory hotplug is enabled (slots > 0) but without '-numa'
* options explicitly on CLI, guestes will break.
@@ -433,6 +429,13 @@ void parse_numa_opts(MachineState *ms)
}
}
+void parse_numa_opts(MachineState *ms)
+{
+ if (qemu_opts_foreach(qemu_find_opts("numa"), parse_numa, ms, NULL)) {
+ exit(1);
+ }
+}
+
void numa_cpu_pre_plug(const CPUArchId *slot, DeviceState *dev, Error **errp)
{
int node_id = object_property_get_int(OBJECT(dev), "node-id", &error_abort);
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 02/11] numa: split out NumaOptions parsing into set_numa_options()
2018-04-27 15:05 [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 01/11] numa: postpone options post-processing till machine_run_board_init() Igor Mammedov
@ 2018-04-27 15:05 ` Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 03/11] cli: add --preconfig option Igor Mammedov
` (8 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2018-04-27 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, ehabkost, pkrempa, armbru
it will allow to reuse set_numa_options() for parsing
configuration commands received via QMP interface
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v5:
- (Eduardo Habkost <ehabkost@redhat.com>)
* drop if (err) guard around error_propagate()
* s/parse_NumaOptions/set_numa_options/
---
include/sysemu/numa.h | 1 +
numa.c | 46 +++++++++++++++++++++++++++-------------------
2 files changed, 28 insertions(+), 19 deletions(-)
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index 21713b7..7a0ae75 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -22,6 +22,7 @@ struct NumaNodeMem {
};
extern NodeInfo numa_info[MAX_NODES];
+int parse_numa(void *opaque, QemuOpts *opts, Error **errp);
void parse_numa_opts(MachineState *ms);
void numa_complete_configuration(MachineState *ms);
void query_numa_node_mem(NumaNodeMem node_mem[]);
diff --git a/numa.c b/numa.c
index d0abd7c..63e3989 100644
--- a/numa.c
+++ b/numa.c
@@ -169,28 +169,11 @@ static void parse_numa_distance(NumaDistOptions *dist, Error **errp)
have_numa_distance = true;
}
-static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
+static
+void set_numa_options(MachineState *ms, NumaOptions *object, Error **errp)
{
- NumaOptions *object = NULL;
- MachineState *ms = opaque;
Error *err = NULL;
- {
- Visitor *v = opts_visitor_new(opts);
- visit_type_NumaOptions(v, NULL, &object, &err);
- visit_free(v);
- }
-
- if (err) {
- goto end;
- }
-
- /* Fix up legacy suffix-less format */
- if ((object->type == NUMA_OPTIONS_TYPE_NODE) && object->u.node.has_mem) {
- const char *mem_str = qemu_opt_get(opts, "mem");
- qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem);
- }
-
switch (object->type) {
case NUMA_OPTIONS_TYPE_NODE:
parse_numa_node(ms, &object->u.node, &err);
@@ -224,6 +207,31 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
}
end:
+ error_propagate(errp, err);
+}
+
+int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
+{
+ NumaOptions *object = NULL;
+ MachineState *ms = MACHINE(opaque);
+ Error *err = NULL;
+ Visitor *v = opts_visitor_new(opts);
+
+ visit_type_NumaOptions(v, NULL, &object, &err);
+ visit_free(v);
+ if (err) {
+ goto end;
+ }
+
+ /* Fix up legacy suffix-less format */
+ if ((object->type == NUMA_OPTIONS_TYPE_NODE) && object->u.node.has_mem) {
+ const char *mem_str = qemu_opt_get(opts, "mem");
+ qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem);
+ }
+
+ set_numa_options(ms, object, &err);
+
+end:
qapi_free_NumaOptions(object);
if (err) {
error_report_err(err);
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 03/11] cli: add --preconfig option
2018-04-27 15:05 [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 01/11] numa: postpone options post-processing till machine_run_board_init() Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 02/11] numa: split out NumaOptions parsing into set_numa_options() Igor Mammedov
@ 2018-04-27 15:05 ` Igor Mammedov
2018-04-27 21:44 ` Eric Blake
2018-05-01 15:07 ` Daniel P. Berrangé
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 04/11] hmp: disable monitor in preconfig state Igor Mammedov
` (7 subsequent siblings)
10 siblings, 2 replies; 25+ messages in thread
From: Igor Mammedov @ 2018-04-27 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, ehabkost, pkrempa, armbru
This option allows pausing QEMU in the new RUN_STATE_PRECONFIG state,
allowing the configuration of QEMU from QMP before the machine jumps
into board initialization code of machine_run_board_init()
The intent is to allow management to query machine state and additionally
configure it using previous query results within one QEMU instance
(i.e. eliminate the need to start QEMU twice, 1st to query board specific
parameters and 2nd for actual VM start using query results for
additional parameters).
The new option complements -S option and could be used with or without
it. The difference is that -S pauses QEMU when the machine is completely
initialized with all devices wired up and ready to execute guest code
(QEMU needs only to unpause VCPUs to let guest execute its code),
while the "preconfig" option pauses QEMU early before board specific init
callback (machine_run_board_init) is executed and allows the configuration
of machine parameters which will be used by board init code.
When early introspection/configuration is done, command 'exit-preconfig'
should be used to exit RUN_STATE_PRECONFIG and transition to the next
requested state (i.e. if -S is used then QEMU will pause the second
time when board/device initialization is completed or start guest
execution if -S isn't provided on CLI)
PS:
Initially 'preconfig' is planned to be used for configuring numa
topology depending on board specified possible cpus layout.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v6:
- (Eduardo Habkost <ehabkost@redhat.com>)
* add exit-preconfig QMP command instead of overloading meaning of 'cont' command
* add doc text to qemu-tech.texi about -S and --preconfig
v5:
- (Eric Blake <eblake@redhat.com>)
* more spelling/wording fixes
* s/-preconfig/--preconfig/
* s/2.12/2.13/
- (Eduardo Habkost <ehabkost@redhat.com>)
* move QCO_ALLOWED_IN_PRECONFIG and do_qmp_dispatch() runstate check
from the later patch 'qapi: introduce new cmd option "allowed-in-preconfig"'
to here for better bissectability
* add TODO comment to '{ RUN_STATE_PRECONFIG, RUN_STATE_INMIGRATE }' transition
* check for incoming && preconfig outside of option parsing loop
v4:
* Explain more on behaviour in commit message and use suggested
wording in message and patch (Eric Blake <eblake@redhat.com>)
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
preconfig
---
include/qapi/qmp/dispatch.h | 1 +
include/sysemu/sysemu.h | 1 +
qapi/misc.json | 22 ++++++++++++++++++++++
qapi/qmp-dispatch.c | 8 ++++++++
qapi/run-state.json | 5 ++++-
qemu-options.hx | 13 +++++++++++++
qemu-tech.texi | 40 ++++++++++++++++++++++++++++++++++++++++
qmp.c | 10 ++++++++++
vl.c | 35 ++++++++++++++++++++++++++++++++++-
9 files changed, 133 insertions(+), 2 deletions(-)
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index ffb4652..6242cfc 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -23,6 +23,7 @@ typedef enum QmpCommandOptions
QCO_NO_OPTIONS = 0x0,
QCO_NO_SUCCESS_RESP = (1U << 0),
QCO_ALLOW_OOB = (1U << 1),
+ QCO_ALLOWED_IN_PRECONFIG = (1U << 2),
} QmpCommandOptions;
typedef struct QmpCommand
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 544ab77..e893f72 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/qapi/misc.json b/qapi/misc.json
index 5636f4a..dfd390b 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -1209,6 +1209,28 @@
{ 'command': 'cont' }
##
+# @exit-preconfig:
+#
+# Exit from "preconfig" state
+#
+# Since 2.13
+#
+# Returns: nothing
+#
+# Notes: Command makes QEMU exit from preconfig state and proceeds with
+# VM initialization using configuration data provided on command line
+# and via QMP monitor at preconfig state. Command is available only at
+# preconfig state (i.e. if --preconfig command line option).
+#
+# Example:
+#
+# -> { "execute": "exit-preconfig" }
+# <- { "return": {} }
+#
+##
+{ 'command': 'exit-preconfig' }
+
+##
# @system_wakeup:
#
# Wakeup guest from suspend. Does nothing in case the guest isn't suspended.
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index dd05907..2604115 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -18,6 +18,7 @@
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qjson.h"
#include "qapi/qmp/qbool.h"
+#include "sysemu/sysemu.h"
QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
{
@@ -101,6 +102,13 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request,
return NULL;
}
+ if (runstate_check(RUN_STATE_PRECONFIG) &&
+ !(cmd->options & QCO_ALLOWED_IN_PRECONFIG)) {
+ error_setg(errp, "The command '%s' isn't permitted in '%s' state",
+ cmd->name, RunState_str(RUN_STATE_PRECONFIG));
+ return NULL;
+ }
+
if (!qdict_haskey(dict, "arguments")) {
args = qdict_new();
} else {
diff --git a/qapi/run-state.json b/qapi/run-state.json
index 1c9fff3..9694a9f 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -49,12 +49,15 @@
# @colo: guest is paused to save/restore VM state under colo checkpoint,
# VM can not get into this state unless colo capability is enabled
# for migration. (since 2.8)
+# @preconfig: QEMU is paused before board specific init callback is executed.
+# The state is reachable only if the --preconfig CLI option is used.
+# (Since 2.13)
##
{ 'enum': 'RunState',
'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
- 'guest-panicked', 'colo' ] }
+ 'guest-panicked', 'colo', 'preconfig' ] }
##
# @StatusInfo:
diff --git a/qemu-options.hx b/qemu-options.hx
index c611766..42a87db 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3304,6 +3304,19 @@ STEXI
Run the emulation in single step mode.
ETEXI
+DEF("preconfig", 0, QEMU_OPTION_preconfig, \
+ "--preconfig pause QEMU before machine is initialized\n",
+ QEMU_ARCH_ALL)
+STEXI
+@item --preconfig
+@findex --preconfig
+Pause QEMU for interactive configuration before the machine is created,
+which allows querying and configuring properties that will affect
+machine initialization. Use the QMP command 'cont' to exit the preconfig
+state and move to the next state (ie. run guest if -S isn't used or
+pause the second time if -S is used).
+ETEXI
+
DEF("S", 0, QEMU_OPTION_S, \
"-S freeze CPU at startup (use 'c' to start execution)\n",
QEMU_ARCH_ALL)
diff --git a/qemu-tech.texi b/qemu-tech.texi
index 52a56ae..ecd6a54 100644
--- a/qemu-tech.texi
+++ b/qemu-tech.texi
@@ -5,6 +5,7 @@
* CPU emulation::
* Translator Internals::
* QEMU compared to other emulators::
+* Managed start up options::
* Bibliography::
@end menu
@@ -314,6 +315,45 @@ VirtualBox [9], Xen [10] and KVM [11] are based on QEMU. QEMU-SystemC
[12] uses QEMU to simulate a system where some hardware devices are
developed in SystemC.
+@node Managed start up options
+@section Managed start up options
+
+In system mode emulation, it's possible to create VM in paused state using
+-S command line option. In this state the machine is completely initialized
+according to command line options and ready to execute VM code but VCPU threads
+are not executing any code. VM state in this paused state depends on way QEMU
+was started. It could be in:
+@table @asis
+@item initial state (after reset/power on state)
+@item with direct kernel loading initial state could be amended to execute
+code loaded by QEMU in VM's RAM and with incoming migration
+@item with incoming migration, initial state will by ammended with the migrated
+machine state after migration completes.
+@end table
+
+This paused state is typically used by users to query machine state and/or
+additionally configure machine (hotplug devices) in runtime before allowing
+VM code to run.
+
+However at -S pause point it's impossible to configure options that affect
+initial VM creation (like: -smp/-m/-numa ...) or cold plug devices. That's
+when -preconfig command line option should be used. It allows pausing QEMU
+before the initial VM creation, in a new preconfig state, where additional
+queries and configuration can be performed via QMP before moving on to
+the resulting configuration startup. In the preconfig state, QEMU only allows
+a limited set of commands over the QMP monitor, where the commands do not
+depend on an initialized machine, including but not limited to:
+@table @asis
+@item qmp_capabilities
+@item query-qmp-schema
+@item query-commands
+@item query-status
+@item exit-preconfig
+@end table
+The full list of commands is in QMP schema which could be queried with
+query-qmp-schema, where commands supported at preconfig state have option
+'allowed-in-preconfig' set to true.
+
@node Bibliography
@section Bibliography
diff --git a/qmp.c b/qmp.c
index f722616..9aace9c 100644
--- a/qmp.c
+++ b/qmp.c
@@ -161,6 +161,16 @@ SpiceInfo *qmp_query_spice(Error **errp)
};
#endif
+void qmp_exit_preconfig(Error **errp)
+{
+ if (!runstate_check(RUN_STATE_PRECONFIG)) {
+ error_setg(errp, "The command is permitted only in '%s' state",
+ RunState_str(RUN_STATE_PRECONFIG));
+ return;
+ }
+ qemu_exit_preconfig_request();
+}
+
void qmp_cont(Error **errp)
{
BlockBackend *blk;
diff --git a/vl.c b/vl.c
index 7487535..f133470 100644
--- a/vl.c
+++ b/vl.c
@@ -595,7 +595,7 @@ static int default_driver_check(void *opaque, QemuOpts *opts, Error **errp)
/***********************************************************/
/* QEMU state */
-static RunState current_run_state = RUN_STATE_PRELAUNCH;
+static RunState current_run_state = RUN_STATE_PRECONFIG;
/* We use RUN_STATE__MAX but any invalid value will do */
static RunState vmstop_requested = RUN_STATE__MAX;
@@ -608,6 +608,13 @@ typedef struct {
static const RunStateTransition runstate_transitions_def[] = {
/* from -> to */
+ { RUN_STATE_PRECONFIG, RUN_STATE_PRELAUNCH },
+ /* Early switch to inmigrate state to allow -incoming CLI option work
+ * as it used to. TODO: delay actual switching to inmigrate state to
+ * the point after machine is built and remove this hack.
+ */
+ { RUN_STATE_PRECONFIG, RUN_STATE_INMIGRATE },
+
{ RUN_STATE_DEBUG, RUN_STATE_RUNNING },
{ RUN_STATE_DEBUG, RUN_STATE_FINISH_MIGRATE },
{ RUN_STATE_DEBUG, RUN_STATE_PRELAUNCH },
@@ -1631,6 +1638,7 @@ static pid_t shutdown_pid;
static int powerdown_requested;
static int debug_requested;
static int suspend_requested;
+static bool preconfig_exit_requested = true;
static WakeupReason wakeup_reason;
static NotifierList powerdown_notifiers =
NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
@@ -1715,6 +1723,11 @@ static int qemu_debug_requested(void)
return r;
}
+void qemu_exit_preconfig_request(void)
+{
+ preconfig_exit_requested = true;
+}
+
/*
* Reset the VM. Issue an event unless @reason is SHUTDOWN_CAUSE_NONE.
*/
@@ -1888,6 +1901,13 @@ static bool main_loop_should_exit(void)
RunState r;
ShutdownCause request;
+ if (preconfig_exit_requested) {
+ if (runstate_check(RUN_STATE_PRECONFIG)) {
+ runstate_set(RUN_STATE_PRELAUNCH);
+ }
+ preconfig_exit_requested = false;
+ return true;
+ }
if (qemu_debug_requested()) {
vm_stop(RUN_STATE_DEBUG);
}
@@ -3721,6 +3741,9 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
break;
+ case QEMU_OPTION_preconfig:
+ preconfig_exit_requested = false;
+ break;
case QEMU_OPTION_enable_kvm:
olist = qemu_find_opts("machine");
qemu_opts_parse_noisily(olist, "accel=kvm", false);
@@ -4090,6 +4113,12 @@ int main(int argc, char **argv, char **envp)
replay_configure(icount_opts);
+ if (incoming && !preconfig_exit_requested) {
+ error_report("'preconfig' and 'incoming' options are "
+ "mutually exclusive");
+ exit(EXIT_FAILURE);
+ }
+
machine_class = select_machine();
set_memory_options(&ram_slots, &maxram_size, machine_class);
@@ -4621,6 +4650,10 @@ int main(int argc, char **argv, char **envp)
}
parse_numa_opts(current_machine);
+ /* do monitor/qmp handling at preconfig state if requested */
+ main_loop();
+
+ /* from here on runstate is RUN_STATE_PRELAUNCH */
machine_run_board_init(current_machine);
realtime_init();
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 04/11] hmp: disable monitor in preconfig state
2018-04-27 15:05 [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP Igor Mammedov
` (2 preceding siblings ...)
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 03/11] cli: add --preconfig option Igor Mammedov
@ 2018-04-27 15:05 ` Igor Mammedov
2018-04-27 21:48 ` Eric Blake
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 05/11] qapi: introduce new cmd option "allowed-in-preconfig" Igor Mammedov
` (6 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Igor Mammedov @ 2018-04-27 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, ehabkost, pkrempa, armbru
Ban it for now, if someone would need it to work early,
one would have to implement checks if HMP command is valid
at preconfig state.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v5:
* add 'use QMP instead" to error message, suggesting user
the right interface to use
v4:
* v3 was only printing error but not preventing command execution,
Fix it by returning after printing error message.
("Dr. David Alan Gilbert" <dgilbert@redhat.com>)
---
monitor.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/monitor.c b/monitor.c
index 39f8ee1..0ffdf1d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3374,6 +3374,12 @@ static void handle_hmp_command(Monitor *mon, const char *cmdline)
trace_handle_hmp_command(mon, cmdline);
+ if (runstate_check(RUN_STATE_PRECONFIG)) {
+ monitor_printf(mon, "HMP not available in preconfig state, "
+ "use QMP instead\n");
+ return;
+ }
+
cmd = monitor_parse_command(mon, cmdline, &cmdline, mon->cmd_table);
if (!cmd) {
return;
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 05/11] qapi: introduce new cmd option "allowed-in-preconfig"
2018-04-27 15:05 [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP Igor Mammedov
` (3 preceding siblings ...)
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 04/11] hmp: disable monitor in preconfig state Igor Mammedov
@ 2018-04-27 15:05 ` Igor Mammedov
2018-04-27 22:05 ` Eric Blake
2018-05-01 15:29 ` Daniel P. Berrangé
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 06/11] tests: let qapi-schema tests detect allowed-in-preconfig Igor Mammedov
` (5 subsequent siblings)
10 siblings, 2 replies; 25+ messages in thread
From: Igor Mammedov @ 2018-04-27 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, ehabkost, pkrempa, armbru
New option will be used to allow commands, which are prepared/need
to run, during preconfig state. Other commands that should be able
to run in preconfig state, should be ameded to not expect machine
in initialized state or deal with it.
For compatibility reasons, commands that don't use new flag
'allowed-in-preconfig' explicitly are not permitted to run in
preconfig state but allowed in all other states like they used
to be.
Within this patch allow following commands in preconfig state:
qmp_capabilities
query-qmp-schema
query-commands
query-command-line-options
query-status
exit-preconfig
to allow qmp connection, basic introspection and moving to the next
state.
PS:
set-numa-node and query-hotpluggable-cpus will be enabled later in
a separate patches.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v6:
* exclude 'cont' command from preconfig enabled, in favor of
exit-preconfig command
* makr exit-preconfig with allowed-in-preconfig=true
v5:
* allow query-command-line-options in preconfig state
* rebase on top of OOB changes that's now in master
* spelling/wording fixups
* make sure that allowed-in-preconfig could be set only to True
* move out QCO_ALLOWED_IN_PRECONFIG check in do_qmp_dispatch() to
earlier 'cli: add -preconfig option' patch
v4:
* replaces complex "universal" approach
"[PATCH v3 5/9] QAPI: allow to specify valid runstates per command"
with a simpler new command flag "allowed-in-preconfig".
(Eric Blake <eblake@redhat.com>)
---
docs/devel/qapi-code-gen.txt | 10 +++++++++-
monitor.c | 5 +++--
qapi/introspect.json | 5 ++++-
qapi/misc.json | 11 +++++++----
qapi/run-state.json | 3 ++-
scripts/qapi/commands.py | 12 ++++++++----
scripts/qapi/common.py | 19 ++++++++++++-------
scripts/qapi/doc.py | 4 ++--
scripts/qapi/introspect.py | 7 ++++---
tests/qapi-schema/test-qapi.py | 4 ++--
10 files changed, 53 insertions(+), 27 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index a569d24..31c4646 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -559,7 +559,7 @@ following example objects:
Usage: { 'command': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
'*returns': TYPE-NAME, '*boxed': true,
'*gen': false, '*success-response': false,
- '*allow-oob': true }
+ '*allow-oob': true, '*allowed-in-preconfig': true }
Commands are defined by using a dictionary containing several members,
where three members are most common. The 'command' member is a
@@ -683,6 +683,14 @@ OOB command handlers must satisfy the following conditions:
If in doubt, do not implement OOB execution support.
+A command may use optional 'allowed-in-preconfig' key to permit
+its execution at early runtime configuration stage (preconfig runstate).
+If not specified then a command defaults to 'allowed-in-preconfig: false'.
+
+An example of declaring preconfig enabled command:
+ { 'command': 'qmp_capabilities',
+ 'allowed-in-preconfig': true }
+
=== Events ===
Usage: { 'event': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
diff --git a/monitor.c b/monitor.c
index 0ffdf1d..e5e60dc 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1183,7 +1183,7 @@ static void monitor_init_qmp_commands(void)
qmp_register_command(&qmp_commands, "query-qmp-schema",
qmp_query_qmp_schema,
- QCO_NO_OPTIONS);
+ QCO_ALLOWED_IN_PRECONFIG);
qmp_register_command(&qmp_commands, "device_add", qmp_device_add,
QCO_NO_OPTIONS);
qmp_register_command(&qmp_commands, "netdev_add", qmp_netdev_add,
@@ -1193,7 +1193,8 @@ static void monitor_init_qmp_commands(void)
QTAILQ_INIT(&qmp_cap_negotiation_commands);
qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
- qmp_marshal_qmp_capabilities, QCO_NO_OPTIONS);
+ qmp_marshal_qmp_capabilities,
+ QCO_ALLOWED_IN_PRECONFIG);
}
static bool qmp_cap_enabled(Monitor *mon, QMPCapability cap)
diff --git a/qapi/introspect.json b/qapi/introspect.json
index c7f67b7..8036154 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -262,13 +262,16 @@
# @allow-oob: whether the command allows out-of-band execution.
# (Since: 2.12)
#
+# @allowed-in-preconfig: command can be executed in preconfig runstate,
+# default: 'false' (Since 2.13)
+#
# TODO: @success-response (currently irrelevant, because it's QGA, not QMP)
#
# Since: 2.5
##
{ 'struct': 'SchemaInfoCommand',
'data': { 'arg-type': 'str', 'ret-type': 'str',
- 'allow-oob': 'bool' } }
+ 'allow-oob': 'bool', 'allowed-in-preconfig': 'bool' } }
##
# @SchemaInfoEvent:
diff --git a/qapi/misc.json b/qapi/misc.json
index dfd390b..cbe8338 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -35,7 +35,8 @@
#
##
{ 'command': 'qmp_capabilities',
- 'data': { '*enable': [ 'QMPCapability' ] } }
+ 'data': { '*enable': [ 'QMPCapability' ] },
+ 'allowed-in-preconfig': true }
##
# @QMPCapability:
@@ -153,7 +154,8 @@
# Note: This example has been shortened as the real response is too long.
#
##
-{ 'command': 'query-commands', 'returns': ['CommandInfo'] }
+{ 'command': 'query-commands', 'returns': ['CommandInfo'],
+ 'allowed-in-preconfig': true }
##
# @LostTickPolicy:
@@ -1228,7 +1230,7 @@
# <- { "return": {} }
#
##
-{ 'command': 'exit-preconfig' }
+{ 'command': 'exit-preconfig', 'allowed-in-preconfig': true }
##
# @system_wakeup:
@@ -2636,7 +2638,8 @@
#
##
{'command': 'query-command-line-options', 'data': { '*option': 'str' },
- 'returns': ['CommandLineOptionInfo'] }
+ 'returns': ['CommandLineOptionInfo'],
+ 'allowed-in-preconfig': true }
##
# @X86CPURegister32:
diff --git a/qapi/run-state.json b/qapi/run-state.json
index 9694a9f..d6ed5d8 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -94,7 +94,8 @@
# "status": "running" } }
#
##
-{ 'command': 'query-status', 'returns': 'StatusInfo' }
+{ 'command': 'query-status', 'returns': 'StatusInfo',
+ 'allowed-in-preconfig': true }
##
# @SHUTDOWN:
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 0c5da3a..9975d95 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -193,13 +193,16 @@ out:
return ret
-def gen_register_command(name, success_response, allow_oob):
+def gen_register_command(name, success_response, allow_oob,
+ allowed_in_preconfig):
options = []
if not success_response:
options += ['QCO_NO_SUCCESS_RESP']
if allow_oob:
options += ['QCO_ALLOW_OOB']
+ if allowed_in_preconfig:
+ options += ['QCO_ALLOWED_IN_PRECONFIG']
if not options:
options = ['QCO_NO_OPTIONS']
@@ -275,8 +278,8 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
c_prefix=c_name(self._prefix, protect=False)))
genc.add(gen_registry(self._regy, self._prefix))
- def visit_command(self, name, info, arg_type, ret_type,
- gen, success_response, boxed, allow_oob):
+ def visit_command(self, name, info, arg_type, ret_type, gen,
+ success_response, boxed, allow_oob, allowed_in_preconfig):
if not gen:
return
self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type))
@@ -285,7 +288,8 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
self._genc.add(gen_marshal_output(ret_type))
self._genh.add(gen_marshal_decl(name))
self._genc.add(gen_marshal(name, arg_type, boxed, ret_type))
- self._regy += gen_register_command(name, success_response, allow_oob)
+ self._regy += gen_register_command(name, success_response, allow_oob,
+ allowed_in_preconfig)
def gen_commands(schema, output_dir, prefix):
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 3e14bc4..87273a2 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -872,7 +872,8 @@ def check_keys(expr_elem, meta, required, optional=[]):
raise QAPISemError(info,
"'%s' of %s '%s' should only use false value"
% (key, meta, name))
- if (key == 'boxed' or key == 'allow-oob') and value is not True:
+ if (key == 'boxed' or key == 'allow-oob' or
+ key == 'allowed-in-preconfig') and value is not True:
raise QAPISemError(info,
"'%s' of %s '%s' should only use true value"
% (key, meta, name))
@@ -922,7 +923,7 @@ def check_exprs(exprs):
meta = 'command'
check_keys(expr_elem, 'command', [],
['data', 'returns', 'gen', 'success-response',
- 'boxed', 'allow-oob'])
+ 'boxed', 'allow-oob', 'allowed-in-preconfig'])
elif 'event' in expr:
meta = 'event'
check_keys(expr_elem, 'event', [], ['data', 'boxed'])
@@ -1044,8 +1045,8 @@ class QAPISchemaVisitor(object):
def visit_alternate_type(self, name, info, variants):
pass
- def visit_command(self, name, info, arg_type, ret_type,
- gen, success_response, boxed, allow_oob):
+ def visit_command(self, name, info, arg_type, ret_type, gen,
+ success_response, boxed, allow_oob, allowed_in_preconfig):
pass
def visit_event(self, name, info, arg_type, boxed):
@@ -1422,7 +1423,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
class QAPISchemaCommand(QAPISchemaEntity):
def __init__(self, name, info, doc, arg_type, ret_type,
- gen, success_response, boxed, allow_oob):
+ gen, success_response, boxed, allow_oob, allowed_in_preconfig):
QAPISchemaEntity.__init__(self, name, info, doc)
assert not arg_type or isinstance(arg_type, str)
assert not ret_type or isinstance(ret_type, str)
@@ -1434,6 +1435,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
self.success_response = success_response
self.boxed = boxed
self.allow_oob = allow_oob
+ self.allowed_in_preconfig = allowed_in_preconfig
def check(self, schema):
if self._arg_type_name:
@@ -1458,7 +1460,8 @@ class QAPISchemaCommand(QAPISchemaEntity):
visitor.visit_command(self.name, self.info,
self.arg_type, self.ret_type,
self.gen, self.success_response,
- self.boxed, self.allow_oob)
+ self.boxed, self.allow_oob,
+ self.allowed_in_preconfig)
class QAPISchemaEvent(QAPISchemaEntity):
@@ -1678,6 +1681,7 @@ class QAPISchema(object):
success_response = expr.get('success-response', True)
boxed = expr.get('boxed', False)
allow_oob = expr.get('allow-oob', False)
+ allowed_in_preconfig = expr.get('allowed-in-preconfig', False)
if isinstance(data, OrderedDict):
data = self._make_implicit_object_type(
name, info, doc, 'arg', self._make_members(data, info))
@@ -1686,7 +1690,8 @@ class QAPISchema(object):
rets = self._make_array_type(rets[0], info)
self._def_entity(QAPISchemaCommand(name, info, doc, data, rets,
gen, success_response,
- boxed, allow_oob))
+ boxed, allow_oob,
+ allowed_in_preconfig))
def _def_event(self, expr, info, doc):
name = expr['event']
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 9b312b2..cb6f2e1 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -226,8 +226,8 @@ class QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
name=doc.symbol,
body=texi_entity(doc, 'Members')))
- def visit_command(self, name, info, arg_type, ret_type,
- gen, success_response, boxed, allow_oob):
+ def visit_command(self, name, info, arg_type, ret_type, gen,
+ success_response, boxed, allow_oob, allowed_in_preconfig):
doc = self.cur_doc
if boxed:
body = texi_body(doc)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index f9e67e8..5246be1 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -171,14 +171,15 @@ const QLitObject %(c_name)s = %(c_string)s;
{'members': [{'type': self._use_type(m.type)}
for m in variants.variants]})
- def visit_command(self, name, info, arg_type, ret_type,
- gen, success_response, boxed, allow_oob):
+ def visit_command(self, name, info, arg_type, ret_type, gen,
+ success_response, boxed, allow_oob, allowed_in_preconfig):
arg_type = arg_type or self._schema.the_empty_object_type
ret_type = ret_type or self._schema.the_empty_object_type
self._gen_qlit(name, 'command',
{'arg-type': self._use_type(arg_type),
'ret-type': self._use_type(ret_type),
- 'allow-oob': allow_oob})
+ 'allow-oob': allow_oob,
+ 'allowed-in-preconfig': allowed_in_preconfig})
def visit_event(self, name, info, arg_type, boxed):
arg_type = arg_type or self._schema.the_empty_object_type
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index c1a144b..73d2c77 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -41,8 +41,8 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
print('alternate %s' % name)
self._print_variants(variants)
- def visit_command(self, name, info, arg_type, ret_type,
- gen, success_response, boxed, allow_oob):
+ def visit_command(self, name, info, arg_type, ret_type, gen,
+ success_response, boxed, allow_oob, allowed_in_preconfig):
print('command %s %s -> %s' % \
(name, arg_type and arg_type.name, ret_type and ret_type.name))
print(' gen=%s success_response=%s boxed=%s oob=%s' % \
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 06/11] tests: let qapi-schema tests detect allowed-in-preconfig
2018-04-27 15:05 [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP Igor Mammedov
` (4 preceding siblings ...)
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 05/11] qapi: introduce new cmd option "allowed-in-preconfig" Igor Mammedov
@ 2018-04-27 15:05 ` Igor Mammedov
2018-04-27 22:08 ` Eric Blake
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 07/11] tests: add allowed-in-preconfig-test for qapi-schema Igor Mammedov
` (4 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Igor Mammedov @ 2018-04-27 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, ehabkost, pkrempa, armbru
use new allowed-in-preconfig parameter in tests.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/qapi-schema/doc-good.out | 4 ++--
tests/qapi-schema/ident-with-escape.out | 2 +-
tests/qapi-schema/indented-expr.out | 4 ++--
tests/qapi-schema/qapi-schema-test.out | 20 ++++++++++----------
tests/qapi-schema/test-qapi.py | 4 ++--
5 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 63058b1..9c8a483 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -28,9 +28,9 @@ object q_obj_cmd-arg
member arg2: str optional=True
member arg3: bool optional=False
command cmd q_obj_cmd-arg -> Object
- gen=True success_response=True boxed=False oob=False
+ gen=True success_response=True boxed=False oob=False preconfig=False
command cmd-boxed Object -> None
- gen=True success_response=True boxed=True oob=False
+ gen=True success_response=True boxed=True oob=False preconfig=False
doc freeform
body=
= Section
diff --git a/tests/qapi-schema/ident-with-escape.out b/tests/qapi-schema/ident-with-escape.out
index 82213aa..24c976f 100644
--- a/tests/qapi-schema/ident-with-escape.out
+++ b/tests/qapi-schema/ident-with-escape.out
@@ -5,4 +5,4 @@ module ident-with-escape.json
object q_obj_fooA-arg
member bar1: str optional=False
command fooA q_obj_fooA-arg -> None
- gen=True success_response=True boxed=False oob=False
+ gen=True success_response=True boxed=False oob=False preconfig=False
diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out
index 862678f..bd8a486 100644
--- a/tests/qapi-schema/indented-expr.out
+++ b/tests/qapi-schema/indented-expr.out
@@ -3,6 +3,6 @@ enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
prefix QTYPE
module indented-expr.json
command eins None -> None
- gen=True success_response=True boxed=False oob=False
+ gen=True success_response=True boxed=False oob=False preconfig=False
command zwei None -> None
- gen=True success_response=True boxed=False oob=False
+ gen=True success_response=True boxed=False oob=False preconfig=False
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 467577d..1f5038f 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -16,7 +16,7 @@ object Empty1
object Empty2
base Empty1
command user_def_cmd0 Empty2 -> Empty2
- gen=True success_response=True boxed=False oob=False
+ gen=True success_response=True boxed=False oob=False preconfig=False
enum QEnumTwo ['value1', 'value2']
prefix QENUM_TWO
object UserDefOne
@@ -143,31 +143,31 @@ object UserDefNativeListUnion
case sizes: q_obj_sizeList-wrapper
case any: q_obj_anyList-wrapper
command user_def_cmd None -> None
- gen=True success_response=True boxed=False oob=False
+ gen=True success_response=True boxed=False oob=False preconfig=False
object q_obj_user_def_cmd1-arg
member ud1a: UserDefOne optional=False
command user_def_cmd1 q_obj_user_def_cmd1-arg -> None
- gen=True success_response=True boxed=False oob=False
+ gen=True success_response=True boxed=False oob=False preconfig=False
object q_obj_user_def_cmd2-arg
member ud1a: UserDefOne optional=False
member ud1b: UserDefOne optional=True
command user_def_cmd2 q_obj_user_def_cmd2-arg -> UserDefTwo
- gen=True success_response=True boxed=False oob=False
+ gen=True success_response=True boxed=False oob=False preconfig=False
object q_obj_guest-get-time-arg
member a: int optional=False
member b: int optional=True
command guest-get-time q_obj_guest-get-time-arg -> int
- gen=True success_response=True boxed=False oob=False
+ gen=True success_response=True boxed=False oob=False preconfig=False
object q_obj_guest-sync-arg
member arg: any optional=False
command guest-sync q_obj_guest-sync-arg -> any
- gen=True success_response=True boxed=False oob=False
+ gen=True success_response=True boxed=False oob=False preconfig=False
command boxed-struct UserDefZero -> None
- gen=True success_response=True boxed=True oob=False
+ gen=True success_response=True boxed=True oob=False preconfig=False
command boxed-union UserDefNativeListUnion -> None
- gen=True success_response=True boxed=True oob=False
+ gen=True success_response=True boxed=True oob=False preconfig=False
command an-oob-command None -> None
- gen=True success_response=True boxed=False oob=True
+ gen=True success_response=True boxed=False oob=True preconfig=False
object UserDefOptions
member i64: intList optional=True
member u64: uint64List optional=True
@@ -231,4 +231,4 @@ object q_obj___org.qemu_x-command-arg
member c: __org.qemu_x-Union2 optional=False
member d: __org.qemu_x-Alt optional=False
command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Union1
- gen=True success_response=True boxed=False oob=False
+ gen=True success_response=True boxed=False oob=False preconfig=False
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 73d2c77..f0ff701 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -45,8 +45,8 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
success_response, boxed, allow_oob, allowed_in_preconfig):
print('command %s %s -> %s' % \
(name, arg_type and arg_type.name, ret_type and ret_type.name))
- print(' gen=%s success_response=%s boxed=%s oob=%s' % \
- (gen, success_response, boxed, allow_oob))
+ print(' gen=%s success_response=%s boxed=%s oob=%s preconfig=%s' % \
+ (gen, success_response, boxed, allow_oob, allowed_in_preconfig))
def visit_event(self, name, info, arg_type, boxed):
print('event %s %s' % (name, arg_type and arg_type.name))
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 07/11] tests: add allowed-in-preconfig-test for qapi-schema
2018-04-27 15:05 [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP Igor Mammedov
` (5 preceding siblings ...)
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 06/11] tests: let qapi-schema tests detect allowed-in-preconfig Igor Mammedov
@ 2018-04-27 15:05 ` Igor Mammedov
2018-04-27 22:11 ` Eric Blake
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 08/11] tests: extend qmp test with preconfig checks Igor Mammedov
` (3 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Igor Mammedov @ 2018-04-27 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, ehabkost, pkrempa, armbru
make sure the QAPISchema can parse allowed-in-preconfig correctly
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
tests/Makefile.include | 1 +
tests/qapi-schema/allowed-in-preconfig-test.err | 1 +
tests/qapi-schema/allowed-in-preconfig-test.exit | 1 +
tests/qapi-schema/allowed-in-preconfig-test.json | 2 ++
tests/qapi-schema/allowed-in-preconfig-test.out | 0
tests/qapi-schema/qapi-schema-test.json | 3 +++
tests/qapi-schema/qapi-schema-test.out | 2 ++
tests/test-qmp-cmds.c | 4 ++++
8 files changed, 14 insertions(+)
create mode 100644 tests/qapi-schema/allowed-in-preconfig-test.err
create mode 100644 tests/qapi-schema/allowed-in-preconfig-test.exit
create mode 100644 tests/qapi-schema/allowed-in-preconfig-test.json
create mode 100644 tests/qapi-schema/allowed-in-preconfig-test.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3b9a5e3..4d2460d 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -524,6 +524,7 @@ qapi-schema += missing-type.json
qapi-schema += nested-struct-data.json
qapi-schema += non-objects.json
qapi-schema += oob-test.json
+qapi-schema += allowed-in-preconfig-test.json
qapi-schema += pragma-doc-required-crap.json
qapi-schema += pragma-extra-junk.json
qapi-schema += pragma-name-case-whitelist-crap.json
diff --git a/tests/qapi-schema/allowed-in-preconfig-test.err b/tests/qapi-schema/allowed-in-preconfig-test.err
new file mode 100644
index 0000000..2b0511b
--- /dev/null
+++ b/tests/qapi-schema/allowed-in-preconfig-test.err
@@ -0,0 +1 @@
+tests/qapi-schema/allowed-in-preconfig-test.json:2: 'allowed-in-preconfig' of command 'allowed-in-preconfig-test' should only use true value
diff --git a/tests/qapi-schema/allowed-in-preconfig-test.exit b/tests/qapi-schema/allowed-in-preconfig-test.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/allowed-in-preconfig-test.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/allowed-in-preconfig-test.json b/tests/qapi-schema/allowed-in-preconfig-test.json
new file mode 100644
index 0000000..7e0e6cb
--- /dev/null
+++ b/tests/qapi-schema/allowed-in-preconfig-test.json
@@ -0,0 +1,2 @@
+# Check against allowed-in-preconfig illegal value
+{ 'command': 'allowed-in-preconfig-test', 'allowed-in-preconfig': 'some-string' }
diff --git a/tests/qapi-schema/allowed-in-preconfig-test.out b/tests/qapi-schema/allowed-in-preconfig-test.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 06e30f4..608ca35 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -142,6 +142,9 @@
# Smoke test on Out-Of-Band
{ 'command': 'an-oob-command', 'allow-oob': true }
+# test allowed-in-preconfig flag
+{ 'command': 'allowed-in-preconfig-test', 'allowed-in-preconfig': true }
+
# For testing integer range flattening in opts-visitor. The following schema
# corresponds to the option format:
#
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 1f5038f..c955e2d 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -168,6 +168,8 @@ command boxed-union UserDefNativeListUnion -> None
gen=True success_response=True boxed=True oob=False preconfig=False
command an-oob-command None -> None
gen=True success_response=True boxed=False oob=True preconfig=False
+command allowed-in-preconfig-test None -> None
+ gen=True success_response=True boxed=False oob=False preconfig=True
object UserDefOptions
member i64: intList optional=True
member u64: uint64List optional=True
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index db690cc..730476a 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -20,6 +20,10 @@ void qmp_an_oob_command(Error **errp)
{
}
+void qmp_allowed_in_preconfig_test(Error **errp)
+{
+}
+
Empty2 *qmp_user_def_cmd0(Error **errp)
{
return g_new0(Empty2, 1);
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 08/11] tests: extend qmp test with preconfig checks
2018-04-27 15:05 [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP Igor Mammedov
` (6 preceding siblings ...)
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 07/11] tests: add allowed-in-preconfig-test for qapi-schema Igor Mammedov
@ 2018-04-27 15:05 ` Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 09/11] qmp: permit query-hotpluggable-cpus in preconfig state Igor Mammedov
` (2 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2018-04-27 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, ehabkost, pkrempa, armbru
Add permission checks for commands at 'preconfig' stage.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v6:
* replace 'cont' with 'exit-preconfig' command
v5:
* s/-preconfig/--preconfig/
v4:
* s/is_err()/qmp_rsp_is_err()/
* return true even if 'error' doesn't contain 'desc'
(Eric Blake <eblake@redhat.com>)
---
tests/qmp-test.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 772058f..c49837a 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -392,6 +392,49 @@ static void add_query_tests(QmpSchema *schema)
}
}
+static bool qmp_rsp_is_err(QDict *rsp)
+{
+ QDict *error = qdict_get_qdict(rsp, "error");
+ QDECREF(rsp);
+ return !!error;
+}
+
+static void test_qmp_preconfig(void)
+{
+ QDict *rsp, *ret;
+ QTestState *qs = qtest_startf("%s --preconfig", common_args);
+
+ /* preconfig state */
+ /* enabled commands, no error expected */
+ g_assert(!qmp_rsp_is_err(qtest_qmp(qs, "{ 'execute': 'query-commands' }")));
+
+ /* forbidden commands, expected error */
+ g_assert(qmp_rsp_is_err(qtest_qmp(qs, "{ 'execute': 'query-cpus' }")));
+
+ /* check that query-status returns preconfig state */
+ rsp = qtest_qmp(qs, "{ 'execute': 'query-status' }");
+ ret = qdict_get_qdict(rsp, "return");
+ g_assert(ret);
+ g_assert_cmpstr(qdict_get_try_str(ret, "status"), ==, "preconfig");
+ QDECREF(rsp);
+
+ /* exit preconfig state */
+ g_assert(!qmp_rsp_is_err(qtest_qmp(qs, "{ 'execute': 'exit-preconfig' }")));
+ qtest_qmp_eventwait(qs, "RESUME");
+
+ /* check that query-status returns running state */
+ rsp = qtest_qmp(qs, "{ 'execute': 'query-status' }");
+ ret = qdict_get_qdict(rsp, "return");
+ g_assert(ret);
+ g_assert_cmpstr(qdict_get_try_str(ret, "status"), ==, "running");
+ QDECREF(rsp);
+
+ /* enabled commands, no error expected */
+ g_assert(!qmp_rsp_is_err(qtest_qmp(qs, "{ 'execute': 'query-cpus' }")));
+
+ qtest_quit(qs);
+}
+
int main(int argc, char *argv[])
{
QmpSchema schema;
@@ -403,6 +446,7 @@ int main(int argc, char *argv[])
qtest_add_func("qmp/oob", test_qmp_oob);
qmp_schema_init(&schema);
add_query_tests(&schema);
+ qtest_add_func("qmp/preconfig", test_qmp_preconfig);
ret = g_test_run();
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 09/11] qmp: permit query-hotpluggable-cpus in preconfig state
2018-04-27 15:05 [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP Igor Mammedov
` (7 preceding siblings ...)
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 08/11] tests: extend qmp test with preconfig checks Igor Mammedov
@ 2018-04-27 15:05 ` Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 10/11] qmp: add set-numa-node command Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 11/11] tests: functional tests for QMP command set-numa-node Igor Mammedov
10 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2018-04-27 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, ehabkost, pkrempa, armbru
it will allow mgmt to query possible CPUs, which depends on
used machine(version)/-smp options, without restarting
QEMU and use results to configure numa mapping or adding
CPUs with device_add* later.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
PS:
*) device_add is not allowed to run at preconfig in this series
but later it could be dealt with by injecting -device
in preconfig state and letting existing -device handling
to actually plug devices
---
qapi/misc.json | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/qapi/misc.json b/qapi/misc.json
index cbe8338..739396e 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -3250,7 +3250,8 @@
# ]}
#
##
-{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
+{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'],
+ 'allowed-in-preconfig': true }
##
# @GuidInfo:
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 10/11] qmp: add set-numa-node command
2018-04-27 15:05 [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP Igor Mammedov
` (8 preceding siblings ...)
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 09/11] qmp: permit query-hotpluggable-cpus in preconfig state Igor Mammedov
@ 2018-04-27 15:05 ` Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 11/11] tests: functional tests for QMP command set-numa-node Igor Mammedov
10 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2018-04-27 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, ehabkost, pkrempa, armbru
Command is allowed to run only in preconfig stage and
will allow to configure numa mapping for CPUs depending
on possible CPUs layout (query-hotpluggable-cpus) for
given machine instance.
Example of configuration session:
$QEMU -smp 2 --preconfig ...
QMP:
-> {'execute': 'query-hotpluggable-cpus' }
<- {'return': [
{'props': {'core-id': 0, 'thread-id': 0, 'socket-id': 1}, ... },
{'props': {'core-id': 0, 'thread-id': 0, 'socket-id': 0}, ... }
]}
-> {'execute': 'set-numa-node', 'arguments': { 'type': 'node', 'nodeid': 0 } }
<- {'return': {}}
-> {'execute': 'set-numa-node', 'arguments': { 'type': 'cpu',
'node-id': 0, 'core-id': 0, 'thread-id': 0, 'socket-id': 1, }
}
<- {'return': {}}
-> {'execute': 'set-numa-node', 'arguments': { 'type': 'node', 'nodeid': 1 } }
-> {'execute': 'set-numa-node', 'arguments': { 'type': 'cpu',
'node-id': 1, 'core-id': 0, 'thread-id': 0, 'socket-id': 0 }
}
<- {'return': {}}
-> {'execute': 'query-hotpluggable-cpus' }
<- {'return': [
{'props': {'core-id': 0, 'thread-id': 0, 'node-id': 0, 'socket-id': 1}, ... },
{'props': {'core-id': 0, 'thread-id': 0, 'node-id': 1, 'socket-id': 0}, ... }
]}
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v6:
* limit command to preconfig state only
* add example QMP command sequence
---
numa.c | 11 +++++++++++
qapi/misc.json | 14 ++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/numa.c b/numa.c
index 63e3989..8e96bcc 100644
--- a/numa.c
+++ b/numa.c
@@ -444,6 +444,17 @@ void parse_numa_opts(MachineState *ms)
}
}
+void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
+{
+ if (!runstate_check(RUN_STATE_PRECONFIG)) {
+ error_setg(errp, "The command is permitted only in '%s' state",
+ RunState_str(RUN_STATE_PRECONFIG));
+ return;
+ }
+
+ set_numa_options(MACHINE(qdev_get_machine()), cmd, errp);
+}
+
void numa_cpu_pre_plug(const CPUArchId *slot, DeviceState *dev, Error **errp)
{
int node_id = object_property_get_int(OBJECT(dev), "node-id", &error_abort);
diff --git a/qapi/misc.json b/qapi/misc.json
index 739396e..96adc27 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -3475,3 +3475,17 @@
##
{ 'command': 'x-oob-test', 'data' : { 'lock': 'bool' },
'allow-oob': true }
+
+##
+# @set-numa-node:
+#
+# Runtime equivalent of '-numa' CLI option, available at
+# preconfigure stage to configure numa mapping before initializing
+# machine.
+#
+# Since 2.13
+##
+{ 'command': 'set-numa-node', 'boxed': true,
+ 'data': 'NumaOptions',
+ 'allowed-in-preconfig': true
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v6 11/11] tests: functional tests for QMP command set-numa-node
2018-04-27 15:05 [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP Igor Mammedov
` (9 preceding siblings ...)
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 10/11] qmp: add set-numa-node command Igor Mammedov
@ 2018-04-27 15:05 ` Igor Mammedov
10 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2018-04-27 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, ehabkost, pkrempa, armbru
* start QEMU with 2 unmapped cpus,
* while in preconfig state
* add 2 numa nodes
* assign cpus to them
* exit preconfig and in running state check that cpus
are mapped correctly.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v6:
* replace 'cont' with 'exit-preconfig' command
v5:
* s/qobject_to_qdict(/qobject_to(QDict,/
* s/-preconfig/--preconfig/
v4:
* drop duplicate is_err() and reuse qmp_rsp_is_err() wich is moved
to generic file libqtest.c. (Eric Blake <eblake@redhat.com>)
FIXUP! tests: functional tests for QMP command set-numa-node
---
tests/libqtest.h | 9 ++++++++
tests/libqtest.c | 7 +++++++
tests/numa-test.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
tests/qmp-test.c | 7 -------
4 files changed, 77 insertions(+), 7 deletions(-)
diff --git a/tests/libqtest.h b/tests/libqtest.h
index cbe8df4..ac52872 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -972,4 +972,13 @@ void qtest_qmp_device_add(const char *driver, const char *id, const char *fmt,
*/
void qtest_qmp_device_del(const char *id);
+/**
+ * qmp_rsp_is_err:
+ * @rsp: QMP response to check for error
+ *
+ * Test @rsp for error and discard @rsp.
+ * Returns 'true' if there is error in @rsp and 'false' otherwise.
+ */
+bool qmp_rsp_is_err(QDict *rsp);
+
#endif
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 6f33a37..33426d5 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -1098,3 +1098,10 @@ void qtest_qmp_device_del(const char *id)
QDECREF(response1);
QDECREF(response2);
}
+
+bool qmp_rsp_is_err(QDict *rsp)
+{
+ QDict *error = qdict_get_qdict(rsp, "error");
+ QDECREF(rsp);
+ return !!error;
+}
diff --git a/tests/numa-test.c b/tests/numa-test.c
index 0f861d8..0135a0c 100644
--- a/tests/numa-test.c
+++ b/tests/numa-test.c
@@ -260,6 +260,66 @@ static void aarch64_numa_cpu(const void *data)
g_free(cli);
}
+static void pc_dynamic_cpu_cfg(const void *data)
+{
+ QObject *e;
+ QDict *resp;
+ QList *cpus;
+ QTestState *qs;
+
+ qs = qtest_startf("%s %s", data ? (char *)data : "",
+ "-nodefaults --preconfig -smp 2");
+
+ /* create 2 numa nodes */
+ g_assert(!qmp_rsp_is_err(qtest_qmp(qs, "{ 'execute': 'set-numa-node',"
+ " 'arguments': { 'type': 'node', 'nodeid': 0 } }")));
+ g_assert(!qmp_rsp_is_err(qtest_qmp(qs, "{ 'execute': 'set-numa-node',"
+ " 'arguments': { 'type': 'node', 'nodeid': 1 } }")));
+
+ /* map 2 cpus in non default reverse order
+ * i.e socket1->node0, socket0->node1
+ */
+ g_assert(!qmp_rsp_is_err(qtest_qmp(qs, "{ 'execute': 'set-numa-node',"
+ " 'arguments': { 'type': 'cpu', 'node-id': 0, 'socket-id': 1 } }")));
+ g_assert(!qmp_rsp_is_err(qtest_qmp(qs, "{ 'execute': 'set-numa-node',"
+ " 'arguments': { 'type': 'cpu', 'node-id': 1, 'socket-id': 0 } }")));
+
+ /* let machine initialization to complete and run */
+ g_assert(!qmp_rsp_is_err(qtest_qmp(qs, "{ 'execute': 'exit-preconfig' }")));
+ qtest_qmp_eventwait(qs, "RESUME");
+
+ /* check that CPUs are mapped as expected */
+ resp = qtest_qmp(qs, "{ 'execute': 'query-hotpluggable-cpus'}");
+ g_assert(qdict_haskey(resp, "return"));
+ cpus = qdict_get_qlist(resp, "return");
+ g_assert(cpus);
+ while ((e = qlist_pop(cpus))) {
+ const QDict *cpu, *props;
+ int64_t socket, node;
+
+ cpu = qobject_to(QDict, e);
+ g_assert(qdict_haskey(cpu, "props"));
+ props = qdict_get_qdict(cpu, "props");
+
+ g_assert(qdict_haskey(props, "node-id"));
+ node = qdict_get_int(props, "node-id");
+ g_assert(qdict_haskey(props, "socket-id"));
+ socket = qdict_get_int(props, "socket-id");
+
+ if (socket == 0) {
+ g_assert_cmpint(node, ==, 1);
+ } else if (socket == 1) {
+ g_assert_cmpint(node, ==, 0);
+ } else {
+ g_assert(false);
+ }
+ qobject_decref(e);
+ }
+ QDECREF(resp);
+
+ qtest_quit(qs);
+}
+
int main(int argc, char **argv)
{
const char *args = NULL;
@@ -278,6 +338,7 @@ int main(int argc, char **argv)
if (!strcmp(arch, "i386") || !strcmp(arch, "x86_64")) {
qtest_add_data_func("/numa/pc/cpu/explicit", args, pc_numa_cpu);
+ qtest_add_data_func("/numa/pc/dynamic/cpu", args, pc_dynamic_cpu_cfg);
}
if (!strcmp(arch, "ppc64")) {
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index c49837a..4d5f198 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -392,13 +392,6 @@ static void add_query_tests(QmpSchema *schema)
}
}
-static bool qmp_rsp_is_err(QDict *rsp)
-{
- QDict *error = qdict_get_qdict(rsp, "error");
- QDECREF(rsp);
- return !!error;
-}
-
static void test_qmp_preconfig(void)
{
QDict *rsp, *ret;
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 03/11] cli: add --preconfig option
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 03/11] cli: add --preconfig option Igor Mammedov
@ 2018-04-27 21:44 ` Eric Blake
2018-05-01 15:07 ` Daniel P. Berrangé
1 sibling, 0 replies; 25+ messages in thread
From: Eric Blake @ 2018-04-27 21:44 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: ehabkost, pkrempa, armbru
[-- Attachment #1: Type: text/plain, Size: 7825 bytes --]
On 04/27/2018 10:05 AM, Igor Mammedov wrote:
> This option allows pausing QEMU in the new RUN_STATE_PRECONFIG state,
> allowing the configuration of QEMU from QMP before the machine jumps
> into board initialization code of machine_run_board_init()
>
> The intent is to allow management to query machine state and additionally
> configure it using previous query results within one QEMU instance
> (i.e. eliminate the need to start QEMU twice, 1st to query board specific
> parameters and 2nd for actual VM start using query results for
> additional parameters).
>
> The new option complements -S option and could be used with or without
> it. The difference is that -S pauses QEMU when the machine is completely
> initialized with all devices wired up and ready to execute guest code
> (QEMU needs only to unpause VCPUs to let guest execute its code),
> while the "preconfig" option pauses QEMU early before board specific init
> callback (machine_run_board_init) is executed and allows the configuration
> of machine parameters which will be used by board init code.
>
> When early introspection/configuration is done, command 'exit-preconfig'
> should be used to exit RUN_STATE_PRECONFIG and transition to the next
> requested state (i.e. if -S is used then QEMU will pause the second
> time when board/device initialization is completed or start guest
> execution if -S isn't provided on CLI)
>
> PS:
> Initially 'preconfig' is planned to be used for configuring numa
> topology depending on board specified possible cpus layout.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> +++ b/include/qapi/qmp/dispatch.h
> @@ -23,6 +23,7 @@ typedef enum QmpCommandOptions
> QCO_NO_OPTIONS = 0x0,
> QCO_NO_SUCCESS_RESP = (1U << 0),
> QCO_ALLOW_OOB = (1U << 1),
> + QCO_ALLOWED_IN_PRECONFIG = (1U << 2),
> } QmpCommandOptions;
I'm thinking patch 3 and 5 should be swapped, at least for
bisectability. Otherwise, you have a window where --preconfig works
from the command line, but there is nothing registering any commands as
allowed in preconfig, which either lets qemu get stuck (if you actually
check for this flag) or lets you break things during preconfig (if you
don't check for this flag, because then ANY command can be run); I'll
see which as I read further.
> +++ b/qapi/misc.json
> @@ -1209,6 +1209,28 @@
> { 'command': 'cont' }
>
> ##
> +# @exit-preconfig:
> +#
> +# Exit from "preconfig" state
[1]
> +#
> +# Since 2.13
> +#
> +# Returns: nothing
> +#
> +# Notes: Command makes QEMU exit from preconfig state and proceeds with
> +# VM initialization using configuration data provided on command line
> +# and via QMP monitor at preconfig state. Command is available only at
> +# preconfig state (i.e. if --preconfig command line option).
Wording suggestion:
Notes: This command makes QEMU exit the preconfig state and proceed with
VM initialization using configuration data provided on the command line
and via the QMP monitor during the preconfig stat. The command is only
available during the preconfig state (i.e. when the --preconfig command
line option was in use).
Also, I wonder if you need a separate Notes: paragraph, or if this could
be used as the overall command description at [1].
> +#
> +# Example:
> +#
> +# -> { "execute": "exit-preconfig" }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'exit-preconfig' }
> +
Missing the 'allowed-in-preconfig' tag that isn't introduced until patch
5...
> @@ -101,6 +102,13 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request,
> return NULL;
> }
>
> + if (runstate_check(RUN_STATE_PRECONFIG) &&
> + !(cmd->options & QCO_ALLOWED_IN_PRECONFIG)) {
> + error_setg(errp, "The command '%s' isn't permitted in '%s' state",
> + cmd->name, RunState_str(RUN_STATE_PRECONFIG));
> + return NULL;
> + }
> +
...but you check the flag, therefore with just this patch, you can't run
exit-preconfig, and any command line using --preconfig is now hung.
> +++ b/qemu-options.hx
> @@ -3304,6 +3304,19 @@ STEXI
> Run the emulation in single step mode.
> ETEXI
>
> +DEF("preconfig", 0, QEMU_OPTION_preconfig, \
> + "--preconfig pause QEMU before machine is initialized\n",
> + QEMU_ARCH_ALL)
> +STEXI
> +@item --preconfig
> +@findex --preconfig
> +Pause QEMU for interactive configuration before the machine is created,
> +which allows querying and configuring properties that will affect
> +machine initialization. Use the QMP command 'cont' to exit the preconfig
s/cont/exit-preconfig/
> +state and move to the next state (ie. run guest if -S isn't used or
> +pause the second time if -S is used).
> +@node Managed start up options
> +@section Managed start up options
> +
> +In system mode emulation, it's possible to create VM in paused state using
s/create/create a/
s/in paused/in a paused/
s/using/using the/
> +-S command line option. In this state the machine is completely initialized
> +according to command line options and ready to execute VM code but VCPU threads
> +are not executing any code. VM state in this paused state depends on way QEMU
s/VM/The VM/
s/on way/on the way/
> +was started. It could be in:
> +@table @asis
> +@item initial state (after reset/power on state)
> +@item with direct kernel loading initial state could be amended to execute
Run-on; I'm less sure if this is what you intended, but:
s/loading initial/loading, the initial/
> +code loaded by QEMU in VM's RAM and with incoming migration
s/VM's/the VM's/
> +@item with incoming migration, initial state will by ammended with the migrated
s/ammended/amended/
> +machine state after migration completes.
> +@end table
> +
> +This paused state is typically used by users to query machine state and/or
> +additionally configure machine (hotplug devices) in runtime before allowing
s/machine (hotplug/the machine (by hotplugging/
> +VM code to run.
> +
> +However at -S pause point it's impossible to configure options that affect
s/However at -S pause point/However, at the -S pause point,/
> +initial VM creation (like: -smp/-m/-numa ...) or cold plug devices. That's
> +when -preconfig command line option should be used. It allows pausing QEMU
s/-preconfig/the --preconfig/
> +before the initial VM creation, in a new preconfig state, where additional
> +queries and configuration can be performed via QMP before moving on to
> +the resulting configuration startup. In the preconfig state, QEMU only allows
> +a limited set of commands over the QMP monitor, where the commands do not
> +depend on an initialized machine, including but not limited to:
> +@table @asis
> +@item qmp_capabilities
> +@item query-qmp-schema
> +@item query-commands
> +@item query-status
> +@item exit-preconfig
> +@end table
> +The full list of commands is in QMP schema which could be queried with
> +query-qmp-schema, where commands supported at preconfig state have option
> +'allowed-in-preconfig' set to true.
This sentence is not true until later in the series, but I'm fine with
documentation patches going in out-of-order to avoid churn if they are
correct by the end of the series (of course, you may be reordering
things anyways, based on my earlier comments).
But overall this is looking a lot better, with documentation explaining
the need, and the new command (rather than overloading 'cont') for
switching back to the rest of the command line behavior.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 04/11] hmp: disable monitor in preconfig state
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 04/11] hmp: disable monitor in preconfig state Igor Mammedov
@ 2018-04-27 21:48 ` Eric Blake
0 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2018-04-27 21:48 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: ehabkost, pkrempa, armbru
[-- Attachment #1: Type: text/plain, Size: 939 bytes --]
On 04/27/2018 10:05 AM, Igor Mammedov wrote:
> Ban it for now, if someone would need it to work early,
> one would have to implement checks if HMP command is valid
> at preconfig state.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
Reviewed-by: Eric Blake <eblake@redhat.com>
There's a bisection window between when RUN_STATE_PRECONFIG is added and
this patch where HMP can be used to screw things up, but I'm less
worried about that being triggered in practice. If you want to avoid
that window, then introduce RUN_STATE_PRECONFIG in one patch, then this
patch, then add command line --preconfig in a later patch (thus, NOTHING
can get into preconfig state, even though the code is starting to be
modified to honor it, until --preconfig is enabled on the command line).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 05/11] qapi: introduce new cmd option "allowed-in-preconfig"
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 05/11] qapi: introduce new cmd option "allowed-in-preconfig" Igor Mammedov
@ 2018-04-27 22:05 ` Eric Blake
2018-04-28 3:51 ` Peter Xu
2018-05-01 14:57 ` Igor Mammedov
2018-05-01 15:29 ` Daniel P. Berrangé
1 sibling, 2 replies; 25+ messages in thread
From: Eric Blake @ 2018-04-27 22:05 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: ehabkost, pkrempa, armbru, Peter Xu
[-- Attachment #1: Type: text/plain, Size: 6928 bytes --]
On 04/27/2018 10:05 AM, Igor Mammedov wrote:
> New option will be used to allow commands, which are prepared/need
> to run, during preconfig state. Other commands that should be able
> to run in preconfig state, should be ameded to not expect machine
s/ameded/amended/
> in initialized state or deal with it.
>
> For compatibility reasons, commands that don't use new flag
> 'allowed-in-preconfig' explicitly are not permitted to run in
> preconfig state but allowed in all other states like they used
> to be.
>
> Within this patch allow following commands in preconfig state:
> qmp_capabilities
> query-qmp-schema
> query-commands
> query-command-line-options
> query-status
> exit-preconfig
> to allow qmp connection, basic introspection and moving to the next
> state.
>
> PS:
> set-numa-node and query-hotpluggable-cpus will be enabled later in
> a separate patches.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v6:
> * exclude 'cont' command from preconfig enabled, in favor of
> exit-preconfig command
> * makr exit-preconfig with allowed-in-preconfig=true
s/makr/mark/ (not that the changelog matters to git...)
> +++ b/docs/devel/qapi-code-gen.txt
> @@ -559,7 +559,7 @@ following example objects:
> Usage: { 'command': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
> '*returns': TYPE-NAME, '*boxed': true,
> '*gen': false, '*success-response': false,
> - '*allow-oob': true }
> + '*allow-oob': true, '*allowed-in-preconfig': true }
Bikeshedding - is it worth naming this just 'allow-preconfig', for a bit
more similarity to 'allow-oob'? It's less typing, and still conveys the
same amount of information (allow preconfig to use this command).
>
> Commands are defined by using a dictionary containing several members,
> where three members are most common. The 'command' member is a
> @@ -683,6 +683,14 @@ OOB command handlers must satisfy the following conditions:
>
> If in doubt, do not implement OOB execution support.
>
> +A command may use optional 'allowed-in-preconfig' key to permit
> +its execution at early runtime configuration stage (preconfig runstate).
> +If not specified then a command defaults to 'allowed-in-preconfig: false'.
Unusual spelling for JSON:
s/'allowed-in-preconfig: false'/'allowed-in-preconfig':false/
and of course, additional tweaks here and in other patches if you like
my bikeshedding for a shorter name.
> +
> +An example of declaring preconfig enabled command:
Might read better as:
An example of declaring a command that is enabled during preconfig:
> + { 'command': 'qmp_capabilities',
> + 'allowed-in-preconfig': true }
> +
> === Events ===
>
> Usage: { 'event': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
> diff --git a/monitor.c b/monitor.c
> index 0ffdf1d..e5e60dc 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1183,7 +1183,7 @@ static void monitor_init_qmp_commands(void)
>
> qmp_register_command(&qmp_commands, "query-qmp-schema",
> qmp_query_qmp_schema,
> - QCO_NO_OPTIONS);
> + QCO_ALLOWED_IN_PRECONFIG);
I understand why query-qmp-schema is special cased (because it uses
'gen':false in QAPI), but...
> qmp_register_command(&qmp_commands, "device_add", qmp_device_add,
> QCO_NO_OPTIONS);
> qmp_register_command(&qmp_commands, "netdev_add", qmp_netdev_add,
> @@ -1193,7 +1193,8 @@ static void monitor_init_qmp_commands(void)
>
> QTAILQ_INIT(&qmp_cap_negotiation_commands);
> qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
> - qmp_marshal_qmp_capabilities, QCO_NO_OPTIONS);
> + qmp_marshal_qmp_capabilities,
> + QCO_ALLOWED_IN_PRECONFIG);
...why are we still special-casing the registration of qmp_capabilities
here...
> }
>
> static bool qmp_cap_enabled(Monitor *mon, QMPCapability cap)
> diff --git a/qapi/introspect.json b/qapi/introspect.json
> index c7f67b7..8036154 100644
> --- a/qapi/introspect.json
> +++ b/qapi/introspect.json
> @@ -262,13 +262,16 @@
> # @allow-oob: whether the command allows out-of-band execution.
> # (Since: 2.12)
> #
> +# @allowed-in-preconfig: command can be executed in preconfig runstate,
> +# default: 'false' (Since 2.13)
> +#
If we like the bikeshedding on the QAPI spelling, I think it also
applies to the introspection spelling.
> +++ b/qapi/misc.json
> @@ -35,7 +35,8 @@
> #
> ##
> { 'command': 'qmp_capabilities',
> - 'data': { '*enable': [ 'QMPCapability' ] } }
> + 'data': { '*enable': [ 'QMPCapability' ] },
> + 'allowed-in-preconfig': true }
...should't this be good enough to get qmp_capabilities registered? Hmm
- maybe that's an independent cleanup (and it might be missed fallout
from Peter Xu's OOB work).
> +++ b/scripts/qapi/common.py
> @@ -1422,7 +1423,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
>
> class QAPISchemaCommand(QAPISchemaEntity):
> def __init__(self, name, info, doc, arg_type, ret_type,
> - gen, success_response, boxed, allow_oob):
> + gen, success_response, boxed, allow_oob, allowed_in_preconfig):
Borderline long line; I'd wrap it (except that if you use the shorter
name allow_preconfig, then there's not a long line issue).
> +++ b/scripts/qapi/doc.py
> @@ -226,8 +226,8 @@ class QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
> name=doc.symbol,
> body=texi_entity(doc, 'Members')))
>
> - def visit_command(self, name, info, arg_type, ret_type,
> - gen, success_response, boxed, allow_oob):
> + def visit_command(self, name, info, arg_type, ret_type, gen,
> + success_response, boxed, allow_oob, allowed_in_preconfig):
> doc = self.cur_doc
> if boxed:
> body = texi_body(doc)
It's nice that introspection covers whether a command is allowed during
preconfig; but wouldn't it also be nice if the documentation also
mentioned this attribute? (Hmm, partly my fault for missing during
review of OOB that the same can be said about documentation of allow_oob
- so you were just copying existing practice)
Overall looks reasonable to me, but see my notes elsewhere in the series
about hoisting this earlier in the series while still keeping things
compiling (perhaps by splitting the introduction of
QCO_ALLOWED_IN_PRECONFIG from the introduction of --preconfig on the
command line).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 06/11] tests: let qapi-schema tests detect allowed-in-preconfig
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 06/11] tests: let qapi-schema tests detect allowed-in-preconfig Igor Mammedov
@ 2018-04-27 22:08 ` Eric Blake
2018-04-27 22:12 ` Eric Blake
0 siblings, 1 reply; 25+ messages in thread
From: Eric Blake @ 2018-04-27 22:08 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: ehabkost, pkrempa, armbru
[-- Attachment #1: Type: text/plain, Size: 1289 bytes --]
On 04/27/2018 10:05 AM, Igor Mammedov wrote:
> use new allowed-in-preconfig parameter in tests.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/qapi-schema/doc-good.out | 4 ++--
> tests/qapi-schema/ident-with-escape.out | 2 +-
> tests/qapi-schema/indented-expr.out | 4 ++--
> tests/qapi-schema/qapi-schema-test.out | 20 ++++++++++----------
> tests/qapi-schema/test-qapi.py | 4 ++--
> 5 files changed, 17 insertions(+), 17 deletions(-)
Might also be worth adding a negative test, similar to oob-test.json, as
well as tweaking qapi-schema.json to have at least one command with the
property set to true...
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -16,7 +16,7 @@ object Empty1
> object Empty2
> base Empty1
> command user_def_cmd0 Empty2 -> Empty2
> - gen=True success_response=True boxed=False oob=False
> + gen=True success_response=True boxed=False oob=False preconfig=False
> enum QEnumTwo ['value1', 'value2']
> prefix QENUM_TWO
> object UserDefOne
...so that this file tests as much as possible independently from our
normal QMP schema.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 07/11] tests: add allowed-in-preconfig-test for qapi-schema
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 07/11] tests: add allowed-in-preconfig-test for qapi-schema Igor Mammedov
@ 2018-04-27 22:11 ` Eric Blake
0 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2018-04-27 22:11 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: ehabkost, pkrempa, armbru
[-- Attachment #1: Type: text/plain, Size: 2039 bytes --]
On 04/27/2018 10:05 AM, Igor Mammedov wrote:
> make sure the QAPISchema can parse allowed-in-preconfig correctly
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> tests/Makefile.include | 1 +
> tests/qapi-schema/allowed-in-preconfig-test.err | 1 +
> tests/qapi-schema/allowed-in-preconfig-test.exit | 1 +
> tests/qapi-schema/allowed-in-preconfig-test.json | 2 ++
> tests/qapi-schema/allowed-in-preconfig-test.out | 0
> tests/qapi-schema/qapi-schema-test.json | 3 +++
> tests/qapi-schema/qapi-schema-test.out | 2 ++
> tests/test-qmp-cmds.c | 4 ++++
Aha. I'd squash this with patch 6, since it supplies the additional
tests I was asking about there. (They were separate on the OOB series
more because of the time crunch of getting it in before rc0; but now
that we aren't under a crunch, I'd rather just have one patch for all
new tests).
> 8 files changed, 14 insertions(+)
> create mode 100644 tests/qapi-schema/allowed-in-preconfig-test.err
> create mode 100644 tests/qapi-schema/allowed-in-preconfig-test.exit
> create mode 100644 tests/qapi-schema/allowed-in-preconfig-test.json
> create mode 100644 tests/qapi-schema/allowed-in-preconfig-test.out
>
If you like my bikeshedding for a shorter 'allow-preconfig' name, don't
forget to rename the files as well as their contents.
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -142,6 +142,9 @@
> # Smoke test on Out-Of-Band
> { 'command': 'an-oob-command', 'allow-oob': true }
>
> +# test allowed-in-preconfig flag
> +{ 'command': 'allowed-in-preconfig-test', 'allowed-in-preconfig': true }
Why not just have a single command that sets both allow-oob and
allowed-in-preconfig on the same command? Of course, rename it to
something other than 'an-oob-command', if you merge things into one command.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 06/11] tests: let qapi-schema tests detect allowed-in-preconfig
2018-04-27 22:08 ` Eric Blake
@ 2018-04-27 22:12 ` Eric Blake
0 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2018-04-27 22:12 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel; +Cc: pkrempa, ehabkost, armbru
[-- Attachment #1: Type: text/plain, Size: 499 bytes --]
On 04/27/2018 05:08 PM, Eric Blake wrote:
> On 04/27/2018 10:05 AM, Igor Mammedov wrote:
>> use new allowed-in-preconfig parameter in tests.
>>
>
> Might also be worth adding a negative test, similar to oob-test.json, as
> well as tweaking qapi-schema.json to have at least one command with the
> property set to true...
Answering myself - see patch 7.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 05/11] qapi: introduce new cmd option "allowed-in-preconfig"
2018-04-27 22:05 ` Eric Blake
@ 2018-04-28 3:51 ` Peter Xu
2018-04-30 14:49 ` Eric Blake
2018-05-01 14:57 ` Igor Mammedov
1 sibling, 1 reply; 25+ messages in thread
From: Peter Xu @ 2018-04-28 3:51 UTC (permalink / raw)
To: Eric Blake; +Cc: Igor Mammedov, qemu-devel, ehabkost, pkrempa, armbru
On Fri, Apr 27, 2018 at 05:05:30PM -0500, Eric Blake wrote:
[...]
> > diff --git a/monitor.c b/monitor.c
> > index 0ffdf1d..e5e60dc 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -1183,7 +1183,7 @@ static void monitor_init_qmp_commands(void)
> >
> > qmp_register_command(&qmp_commands, "query-qmp-schema",
> > qmp_query_qmp_schema,
> > - QCO_NO_OPTIONS);
> > + QCO_ALLOWED_IN_PRECONFIG);
>
> I understand why query-qmp-schema is special cased (because it uses
> 'gen':false in QAPI), but...
>
> > qmp_register_command(&qmp_commands, "device_add", qmp_device_add,
> > QCO_NO_OPTIONS);
> > qmp_register_command(&qmp_commands, "netdev_add", qmp_netdev_add,
> > @@ -1193,7 +1193,8 @@ static void monitor_init_qmp_commands(void)
> >
> > QTAILQ_INIT(&qmp_cap_negotiation_commands);
> > qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
> > - qmp_marshal_qmp_capabilities, QCO_NO_OPTIONS);
> > + qmp_marshal_qmp_capabilities,
> > + QCO_ALLOWED_IN_PRECONFIG);
>
> ...why are we still special-casing the registration of qmp_capabilities
> here...
[1]
>
> > }
> >
> > static bool qmp_cap_enabled(Monitor *mon, QMPCapability cap)
> > diff --git a/qapi/introspect.json b/qapi/introspect.json
> > index c7f67b7..8036154 100644
> > --- a/qapi/introspect.json
> > +++ b/qapi/introspect.json
> > @@ -262,13 +262,16 @@
> > # @allow-oob: whether the command allows out-of-band execution.
> > # (Since: 2.12)
> > #
> > +# @allowed-in-preconfig: command can be executed in preconfig runstate,
> > +# default: 'false' (Since 2.13)
> > +#
>
> If we like the bikeshedding on the QAPI spelling, I think it also
> applies to the introspection spelling.
>
>
> > +++ b/qapi/misc.json
> > @@ -35,7 +35,8 @@
> > #
> > ##
> > { 'command': 'qmp_capabilities',
> > - 'data': { '*enable': [ 'QMPCapability' ] } }
> > + 'data': { '*enable': [ 'QMPCapability' ] },
> > + 'allowed-in-preconfig': true }
>
> ...should't this be good enough to get qmp_capabilities registered? Hmm
> - maybe that's an independent cleanup (and it might be missed fallout
> from Peter Xu's OOB work).
My understanding is that we have two lists:
QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
And here it only registers with qmp_commands list via:
qmp_init_marshal(&qmp_commands);
But not for the other one, which is explicitly registered at [1]. So
it seems that [1] is still needed?
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 05/11] qapi: introduce new cmd option "allowed-in-preconfig"
2018-04-28 3:51 ` Peter Xu
@ 2018-04-30 14:49 ` Eric Blake
0 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2018-04-30 14:49 UTC (permalink / raw)
To: Peter Xu; +Cc: Igor Mammedov, qemu-devel, ehabkost, pkrempa, armbru
On 04/27/2018 10:51 PM, Peter Xu wrote:
>>> QTAILQ_INIT(&qmp_cap_negotiation_commands);
>>> qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
>>> - qmp_marshal_qmp_capabilities, QCO_NO_OPTIONS);
>>> + qmp_marshal_qmp_capabilities,
>>> + QCO_ALLOWED_IN_PRECONFIG);
>>
>> ...why are we still special-casing the registration of qmp_capabilities
>> here...
>
> My understanding is that we have two lists:
>
> QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
>
> And here it only registers with qmp_commands list via:
>
> qmp_init_marshal(&qmp_commands);
>
> But not for the other one, which is explicitly registered at [1]. So
> it seems that [1] is still needed?
Ah, that makes sense. I overlooked the difference in list name in the
first parameter to qmp_register_command(), since that line was not
changed in the diff.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 05/11] qapi: introduce new cmd option "allowed-in-preconfig"
2018-04-27 22:05 ` Eric Blake
2018-04-28 3:51 ` Peter Xu
@ 2018-05-01 14:57 ` Igor Mammedov
1 sibling, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2018-05-01 14:57 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, pkrempa, ehabkost, Peter Xu, armbru
On Fri, 27 Apr 2018 17:05:30 -0500
Eric Blake <eblake@redhat.com> wrote:
> On 04/27/2018 10:05 AM, Igor Mammedov wrote:
> > New option will be used to allow commands, which are prepared/need
> > to run, during preconfig state. Other commands that should be able
> > to run in preconfig state, should be ameded to not expect machine
>
> s/ameded/amended/
>
> > in initialized state or deal with it.
> >
> > For compatibility reasons, commands that don't use new flag
> > 'allowed-in-preconfig' explicitly are not permitted to run in
> > preconfig state but allowed in all other states like they used
> > to be.
> >
> > Within this patch allow following commands in preconfig state:
> > qmp_capabilities
> > query-qmp-schema
> > query-commands
> > query-command-line-options
> > query-status
> > exit-preconfig
> > to allow qmp connection, basic introspection and moving to the next
> > state.
> >
> > PS:
> > set-numa-node and query-hotpluggable-cpus will be enabled later in
> > a separate patches.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v6:
> > * exclude 'cont' command from preconfig enabled, in favor of
> > exit-preconfig command
> > * makr exit-preconfig with allowed-in-preconfig=true
>
> s/makr/mark/ (not that the changelog matters to git...)
>
>
> > +++ b/docs/devel/qapi-code-gen.txt
> > @@ -559,7 +559,7 @@ following example objects:
> > Usage: { 'command': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
> > '*returns': TYPE-NAME, '*boxed': true,
> > '*gen': false, '*success-response': false,
> > - '*allow-oob': true }
> > + '*allow-oob': true, '*allowed-in-preconfig': true }
>
> Bikeshedding - is it worth naming this just 'allow-preconfig', for a bit
> more similarity to 'allow-oob'? It's less typing, and still conveys the
> same amount of information (allow preconfig to use this command).
Since I'll need to respin anyways, lets got with shorter name
>
> >
> > Commands are defined by using a dictionary containing several members,
> > where three members are most common. The 'command' member is a
> > @@ -683,6 +683,14 @@ OOB command handlers must satisfy the following conditions:
> >
> > If in doubt, do not implement OOB execution support.
> >
> > +A command may use optional 'allowed-in-preconfig' key to permit
> > +its execution at early runtime configuration stage (preconfig runstate).
> > +If not specified then a command defaults to 'allowed-in-preconfig: false'.
>
> Unusual spelling for JSON:
> s/'allowed-in-preconfig: false'/'allowed-in-preconfig':false/
>
> and of course, additional tweaks here and in other patches if you like
> my bikeshedding for a shorter name.
>
> > +
> > +An example of declaring preconfig enabled command:
>
> Might read better as:
> An example of declaring a command that is enabled during preconfig:
>
> > + { 'command': 'qmp_capabilities',
> > + 'allowed-in-preconfig': true }
> > +
> > === Events ===
> >
> > Usage: { 'event': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
> > diff --git a/monitor.c b/monitor.c
> > index 0ffdf1d..e5e60dc 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -1183,7 +1183,7 @@ static void monitor_init_qmp_commands(void)
> >
> > qmp_register_command(&qmp_commands, "query-qmp-schema",
> > qmp_query_qmp_schema,
> > - QCO_NO_OPTIONS);
> > + QCO_ALLOWED_IN_PRECONFIG);
>
> I understand why query-qmp-schema is special cased (because it uses
> 'gen':false in QAPI), but...
>
> > qmp_register_command(&qmp_commands, "device_add", qmp_device_add,
> > QCO_NO_OPTIONS);
> > qmp_register_command(&qmp_commands, "netdev_add", qmp_netdev_add,
> > @@ -1193,7 +1193,8 @@ static void monitor_init_qmp_commands(void)
> >
> > QTAILQ_INIT(&qmp_cap_negotiation_commands);
> > qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
> > - qmp_marshal_qmp_capabilities, QCO_NO_OPTIONS);
> > + qmp_marshal_qmp_capabilities,
> > + QCO_ALLOWED_IN_PRECONFIG);
>
> ...why are we still special-casing the registration of qmp_capabilities
> here...
>
> > }
> >
> > static bool qmp_cap_enabled(Monitor *mon, QMPCapability cap)
> > diff --git a/qapi/introspect.json b/qapi/introspect.json
> > index c7f67b7..8036154 100644
> > --- a/qapi/introspect.json
> > +++ b/qapi/introspect.json
> > @@ -262,13 +262,16 @@
> > # @allow-oob: whether the command allows out-of-band execution.
> > # (Since: 2.12)
> > #
> > +# @allowed-in-preconfig: command can be executed in preconfig runstate,
> > +# default: 'false' (Since 2.13)
> > +#
>
> If we like the bikeshedding on the QAPI spelling, I think it also
> applies to the introspection spelling.
>
>
> > +++ b/qapi/misc.json
> > @@ -35,7 +35,8 @@
> > #
> > ##
> > { 'command': 'qmp_capabilities',
> > - 'data': { '*enable': [ 'QMPCapability' ] } }
> > + 'data': { '*enable': [ 'QMPCapability' ] },
> > + 'allowed-in-preconfig': true }
>
> ...should't this be good enough to get qmp_capabilities registered? Hmm
> - maybe that's an independent cleanup (and it might be missed fallout
> from Peter Xu's OOB work).
>
>
> > +++ b/scripts/qapi/common.py
>
> > @@ -1422,7 +1423,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
> >
> > class QAPISchemaCommand(QAPISchemaEntity):
> > def __init__(self, name, info, doc, arg_type, ret_type,
> > - gen, success_response, boxed, allow_oob):
> > + gen, success_response, boxed, allow_oob, allowed_in_preconfig):
>
> Borderline long line; I'd wrap it (except that if you use the shorter
> name allow_preconfig, then there's not a long line issue).
>
> > +++ b/scripts/qapi/doc.py
> > @@ -226,8 +226,8 @@ class QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
> > name=doc.symbol,
> > body=texi_entity(doc, 'Members')))
> >
> > - def visit_command(self, name, info, arg_type, ret_type,
> > - gen, success_response, boxed, allow_oob):
> > + def visit_command(self, name, info, arg_type, ret_type, gen,
> > + success_response, boxed, allow_oob, allowed_in_preconfig):
> > doc = self.cur_doc
> > if boxed:
> > body = texi_body(doc)
>
> It's nice that introspection covers whether a command is allowed during
> preconfig; but wouldn't it also be nice if the documentation also
> mentioned this attribute? (Hmm, partly my fault for missing during
> review of OOB that the same can be said about documentation of allow_oob
> - so you were just copying existing practice)
yep, it's pretty much copypast.
Could you point to a place/example commit that adds documentation?
> Overall looks reasonable to me, but see my notes elsewhere in the series
> about hoisting this earlier in the series while still keeping things
> compiling (perhaps by splitting the introduction of
> QCO_ALLOWED_IN_PRECONFIG from the introduction of --preconfig on the
> command line).
I'll fix the rest of comments and reshuffle series as you suggest here
and in previous review comments.
Thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 03/11] cli: add --preconfig option
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 03/11] cli: add --preconfig option Igor Mammedov
2018-04-27 21:44 ` Eric Blake
@ 2018-05-01 15:07 ` Daniel P. Berrangé
2018-05-03 11:35 ` Igor Mammedov
1 sibling, 1 reply; 25+ messages in thread
From: Daniel P. Berrangé @ 2018-05-01 15:07 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, pkrempa, ehabkost, armbru
On Fri, Apr 27, 2018 at 05:05:15PM +0200, Igor Mammedov wrote:
> This option allows pausing QEMU in the new RUN_STATE_PRECONFIG state,
> allowing the configuration of QEMU from QMP before the machine jumps
> into board initialization code of machine_run_board_init()
>
> The intent is to allow management to query machine state and additionally
> configure it using previous query results within one QEMU instance
> (i.e. eliminate the need to start QEMU twice, 1st to query board specific
> parameters and 2nd for actual VM start using query results for
> additional parameters).
>
> The new option complements -S option and could be used with or without
> it. The difference is that -S pauses QEMU when the machine is completely
> initialized with all devices wired up and ready to execute guest code
> (QEMU needs only to unpause VCPUs to let guest execute its code),
> while the "preconfig" option pauses QEMU early before board specific init
> callback (machine_run_board_init) is executed and allows the configuration
> of machine parameters which will be used by board init code.
> When early introspection/configuration is done, command 'exit-preconfig'
> should be used to exit RUN_STATE_PRECONFIG and transition to the next
> requested state (i.e. if -S is used then QEMU will pause the second
> time when board/device initialization is completed or start guest
> execution if -S isn't provided on CLI)
If we're going to introduce this --preconfig facility I would very much
like us to ensure that we do *not* need to continue to also use -S. IOW
it would be nice to essentially consider --preconfig a better impl of
-S and stop using -S, perhaps even deprecating it for removal.
What logical things does QEMU do after exiting pre-config stage, but
before starting CPUs that might be dependency for mgmt apps that would
neccessitate using both --preconfig and -S ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 05/11] qapi: introduce new cmd option "allowed-in-preconfig"
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 05/11] qapi: introduce new cmd option "allowed-in-preconfig" Igor Mammedov
2018-04-27 22:05 ` Eric Blake
@ 2018-05-01 15:29 ` Daniel P. Berrangé
2018-05-03 11:22 ` Igor Mammedov
1 sibling, 1 reply; 25+ messages in thread
From: Daniel P. Berrangé @ 2018-05-01 15:29 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, pkrempa, ehabkost, armbru
On Fri, Apr 27, 2018 at 05:05:17PM +0200, Igor Mammedov wrote:
> New option will be used to allow commands, which are prepared/need
> to run, during preconfig state. Other commands that should be able
> to run in preconfig state, should be ameded to not expect machine
> in initialized state or deal with it.
>
> For compatibility reasons, commands that don't use new flag
> 'allowed-in-preconfig' explicitly are not permitted to run in
> preconfig state but allowed in all other states like they used
> to be.
>
> Within this patch allow following commands in preconfig state:
> qmp_capabilities
> query-qmp-schema
> query-commands
> query-command-line-options
> query-status
> exit-preconfig
> to allow qmp connection, basic introspection and moving to the next
> state.
Looking at what libvirt sends to QEMU before 'cont', when -S is used,
we have a lot of differences. In order, libvirt does something like
this on guest startup:
"qmp_capabilities"
"query-migrate-capabilities"
"migrate-set-capabilities"
"query-chardev"
"qom-list"
"qom-get"
"qom-get"
"query-hotpluggable-cpus"
"query-cpus"
"query-iothreads"
"balloon"
"query-balloon"
"cont"
So --preconfig looks like it cannot be used for anything libvirt currently
does during guest startup. This would force us to use the new set-numa-node
command right at the start, even though this is possibly not a time when
libvirt is actually ready to set the numa node in QEMU. This kind of code
ordering constraint QEMU will put on libvirt is why I really don't like
the idea of having multiple startup phases where you can only use certain
commands in certain phases.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 05/11] qapi: introduce new cmd option "allowed-in-preconfig"
2018-05-01 15:29 ` Daniel P. Berrangé
@ 2018-05-03 11:22 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2018-05-03 11:22 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, pkrempa, ehabkost, armbru
On Tue, 1 May 2018 16:29:10 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Fri, Apr 27, 2018 at 05:05:17PM +0200, Igor Mammedov wrote:
> > New option will be used to allow commands, which are prepared/need
> > to run, during preconfig state. Other commands that should be able
> > to run in preconfig state, should be ameded to not expect machine
> > in initialized state or deal with it.
> >
> > For compatibility reasons, commands that don't use new flag
> > 'allowed-in-preconfig' explicitly are not permitted to run in
> > preconfig state but allowed in all other states like they used
> > to be.
> >
> > Within this patch allow following commands in preconfig state:
> > qmp_capabilities
> > query-qmp-schema
> > query-commands
> > query-command-line-options
> > query-status
> > exit-preconfig
> > to allow qmp connection, basic introspection and moving to the next
> > state.
>
> Looking at what libvirt sends to QEMU before 'cont', when -S is used,
> we have a lot of differences. In order, libvirt does something like
> this on guest startup:
I took a quick look at these commands in QEMU
> "qmp_capabilities"
this is preconfig enabled within this series
> "query-migrate-capabilities"
> "migrate-set-capabilities"
it could be possible to make it work at preconfig time,
but we probably need to fix -incoming mess first
(as David pointed out it's possible to postpone switching to
inmigrate runstate but needs some work done to make it
actually happen)
> "query-chardev"
could be enabled at preconfig with some behavioral change,
namely, frontend-open field would report false at preconfig time
> "qom-list"
> "qom-get"
> "qom-get"
Above depends on whether being queried object exits at
preconfig time, so possibility of using commands should be
evaluated separately for each use case.
Could you point out to relevant objects that libvirt
uses with these commands and why it's needed?
> "query-hotpluggable-cpus"
preconfig enabled within this series, so that
libvirt could get cpus layout for configuring numa
(it should work for x86 and spapr, and later with AR
once CPU hotplug is enabled there)
> "query-iothreads"
both -object and monitor_init_globals() are done before machine_init
so it should be possible make it work at preconfig
> "balloon"
> "query-balloon"
These are depend on initialized machine and -device balloon processed
What's the reasons for libvirt to use it at -S pause instead of
using -device on CLI for adding balloon device?
> "query-cpus"
That one probably won't be possible as it needs initialized machine
or at least 'machine_init() + -device' processed first.
Why libvirt does need it to start VM?
Above 3 commands and possibly others could be made usable
with --preconfig in case we implement steps between
machine_run_board_init() ... qdev_machine_creation_done()
as qmp commands like: qmp_initialize_board(), enable device_add,
etc. and move earlier the steps from there that don't really
depend on being created machine.
> "cont"
>
> So --preconfig looks like it cannot be used for anything libvirt currently
> does during guest startup. This would force us to use the new set-numa-node
> command right at the start, even though this is possibly not a time when
> libvirt is actually ready to set the numa node in QEMU.
set_numa_node at preconfig time was made with libvirt in mind
so this should work, Peter is just waiting on it being merged
before he would coding libvirt support for it.
> This kind of code
> ordering constraint QEMU will put on libvirt is why I really don't like
> the idea of having multiple startup phases where you can only use certain
> commands in certain phases.
I've even implemented fixing up built machine at -S pause point
for the sake of experiment [1] at he cost of an extra machine reset
per every set-numa-node command, but even if it would work for numa,
is really not good approach, rather fragile and won't scale, we would
be just piling one hack on another.
Hopefully we would be able to make enough commands preconfig enabled
to start VM without need for -S.
> Regards,
> Daniel
[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg502794.html
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v6 03/11] cli: add --preconfig option
2018-05-01 15:07 ` Daniel P. Berrangé
@ 2018-05-03 11:35 ` Igor Mammedov
0 siblings, 0 replies; 25+ messages in thread
From: Igor Mammedov @ 2018-05-03 11:35 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, pkrempa, ehabkost, armbru
On Tue, 1 May 2018 16:07:23 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Fri, Apr 27, 2018 at 05:05:15PM +0200, Igor Mammedov wrote:
> > This option allows pausing QEMU in the new RUN_STATE_PRECONFIG state,
> > allowing the configuration of QEMU from QMP before the machine jumps
> > into board initialization code of machine_run_board_init()
> >
> > The intent is to allow management to query machine state and additionally
> > configure it using previous query results within one QEMU instance
> > (i.e. eliminate the need to start QEMU twice, 1st to query board specific
> > parameters and 2nd for actual VM start using query results for
> > additional parameters).
> >
> > The new option complements -S option and could be used with or without
> > it. The difference is that -S pauses QEMU when the machine is completely
> > initialized with all devices wired up and ready to execute guest code
> > (QEMU needs only to unpause VCPUs to let guest execute its code),
> > while the "preconfig" option pauses QEMU early before board specific init
> > callback (machine_run_board_init) is executed and allows the configuration
> > of machine parameters which will be used by board init code.
>
> > When early introspection/configuration is done, command 'exit-preconfig'
> > should be used to exit RUN_STATE_PRECONFIG and transition to the next
> > requested state (i.e. if -S is used then QEMU will pause the second
> > time when board/device initialization is completed or start guest
> > execution if -S isn't provided on CLI)
>
> If we're going to introduce this --preconfig facility I would very much
> like us to ensure that we do *not* need to continue to also use -S. IOW
> it would be nice to essentially consider --preconfig a better impl of
> -S and stop using -S, perhaps even deprecating it for removal.
That's how I view this as well,
modulo removal of -S option as it still might be used for hotplugging and
for early VM startup dubugging and other thing that really need ready
to run machine.
> What logical things does QEMU do after exiting pre-config stage, but
> before starting CPUs that might be dependency for mgmt apps that would
> neccessitate using both --preconfig and -S ?
I've replied to your other email listing them, so lets figure
out the plan of what needs to be fixed so that libvirt could
switch from -S to --preconfig only.
PS:
(Since I've started this whole thing, I'm willing to work on
gradually improving it)
> Regards,
> Daniel
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2018-05-03 11:35 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-27 15:05 [Qemu-devel] [PATCH v6 00/11] enable numa configuration before machine_init() from QMP Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 01/11] numa: postpone options post-processing till machine_run_board_init() Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 02/11] numa: split out NumaOptions parsing into set_numa_options() Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 03/11] cli: add --preconfig option Igor Mammedov
2018-04-27 21:44 ` Eric Blake
2018-05-01 15:07 ` Daniel P. Berrangé
2018-05-03 11:35 ` Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 04/11] hmp: disable monitor in preconfig state Igor Mammedov
2018-04-27 21:48 ` Eric Blake
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 05/11] qapi: introduce new cmd option "allowed-in-preconfig" Igor Mammedov
2018-04-27 22:05 ` Eric Blake
2018-04-28 3:51 ` Peter Xu
2018-04-30 14:49 ` Eric Blake
2018-05-01 14:57 ` Igor Mammedov
2018-05-01 15:29 ` Daniel P. Berrangé
2018-05-03 11:22 ` Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 06/11] tests: let qapi-schema tests detect allowed-in-preconfig Igor Mammedov
2018-04-27 22:08 ` Eric Blake
2018-04-27 22:12 ` Eric Blake
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 07/11] tests: add allowed-in-preconfig-test for qapi-schema Igor Mammedov
2018-04-27 22:11 ` Eric Blake
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 08/11] tests: extend qmp test with preconfig checks Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 09/11] qmp: permit query-hotpluggable-cpus in preconfig state Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 10/11] qmp: add set-numa-node command Igor Mammedov
2018-04-27 15:05 ` [Qemu-devel] [PATCH v6 11/11] tests: functional tests for QMP command set-numa-node Igor Mammedov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.