| Message ID | 1518784641-43151-6-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | enable numa configuration before machine_init() from QMP | expand |
On 02/16/2018 06:37 AM, Igor Mammedov wrote: > Add optional 'runstates' parameter in QAPI command definition, > which will permit to specify RunState variations in which > a command could be exectuted via QMP monitor. s/exectuted/executed/ > > For compatibility reasons, commands, that don't use s/commands,/commands/ > 'runstates' explicitly, are not permited to run in s/explicitly,/explicitly/ s/permited/permitted/ > preconfig state but allowed in all other states. > > New option will be used to allow commands, which are > prepared/need to run this early, to run in preconfig state. > It will include query-hotpluggable-cpus and new set-numa-node > commands. Other commands that should be able to run in > preconfig state, should be ammeded to not expect machine s/ammeded/amended/ > in initialized state. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > include/qapi/qmp/dispatch.h | 5 +++- > monitor.c | 28 +++++++++++++++++--- > qapi-schema.json | 12 +++++++-- > qapi/qmp-dispatch.c | 39 ++++++++++++++++++++++++++++ > qapi/qmp-registry.c | 4 ++- > qapi/run-state.json | 6 ++++- > scripts/qapi-commands.py | 46 ++++++++++++++++++++++++++------- > scripts/qapi-introspect.py | 2 +- > scripts/qapi.py | 15 +++++++---- > scripts/qapi2texi.py | 2 +- > 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 | 18 ++++++------- > tests/qapi-schema/test-qapi.py | 6 ++--- > 15 files changed, 151 insertions(+), 42 deletions(-) Missing mention in docs/; among other things, see how the OOB series adds a similar per-command witness during QMP on whether the command can be used in certain contexts: https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg05789.html including edits to docs/devel/qapi-code-gen.txt (definitely needed here) and docs/interop/qmp-spec.txt (may or may not be needed here). > > diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h > index 1e694b5..f821781 100644 > --- a/include/qapi/qmp/dispatch.h > +++ b/include/qapi/qmp/dispatch.h > @@ -15,6 +15,7 @@ > #define QAPI_QMP_DISPATCH_H > > #include "qemu/queue.h" > +#include "qapi-types.h" Probably conflict with the pending work from Markus to reorganize the QAPI header files to be more modular. > +++ b/qapi-schema.json > @@ -219,7 +219,11 @@ > # Note: This example has been shortened as the real response is too long. > # > ## > -{ 'command': 'query-commands', 'returns': ['CommandInfo'] } > +{ 'command': 'query-commands', 'returns': ['CommandInfo'], > + 'runstates': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', > + 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', > + 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog', > + 'guest-panicked', 'colo', 'preconfig' ] } Wow, that's going to be a lot of states to list for every command that is interested in the non-default state. Would a simple bool flag be any easier than a list of states, since it looks like preconfig is the only special state? > > ## > # @LostTickPolicy: > @@ -1146,7 +1150,11 @@ > # <- { "return": {} } > # > ## > -{ 'command': 'cont' } > +{ 'command': 'cont', > + 'runstates': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', > + 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', > + 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog', > + 'guest-panicked', 'colo', 'preconfig' ] } Should 'stop' also be permitted in the preconfig state, to get to the state that used to be provided by 'qemu -S'? > @@ -65,6 +66,40 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) > return dict; > } > > +static bool is_cmd_permited(const QmpCommand *cmd, Error **errp) s/permited/permitted/g > +{ > + int i; > + char *list = NULL; > + > + /* Old commands that don't have explicit runstate in schema > + * are permited to run except of at PRECONFIG stage including in the comments > + */ > + if (!cmd->valid_runstates) { > + if (runstate_check(RUN_STATE_PRECONFIG)) { > + const char *state = RunState_str(RUN_STATE_PRECONFIG); > + error_setg(errp, "The command '%s' isn't valid in '%s'", > + cmd->name, state); > + return false; > + } > + return true; > + } > + > + for (i = 0; cmd->valid_runstates[i] != RUN_STATE__MAX; i++) { > + if (runstate_check(cmd->valid_runstates[i])) { > + return true; > + } > + } > + > + for (i = 0; cmd->valid_runstates[i] != RUN_STATE__MAX; i++) { > + const char *state = RunState_str(cmd->valid_runstates[i]); > + list = g_strjoin(", ", state, list, NULL); This is rather complex compared to a simple bool of whether a command can be run in preconfig; do we anticipate ever making any other commands fine-grained by state where the length of this message is worthwhile? > +++ b/scripts/qapi-commands.py > @@ -192,17 +192,45 @@ out: > return ret > > > -def gen_register_command(name, success_response): > +def gen_register_command(name, success_response, runstates): > options = 'QCO_NO_OPTIONS' > if not success_response: > options = 'QCO_NO_SUCCESS_RESP' > > - ret = mcgen(''' > - qmp_register_command(cmds, "%(name)s", > - qmp_marshal_%(c_name)s, %(opts)s); > -''', > - name=name, c_name=c_name(name), > - opts=options) > + if runstates is None: > + ret = mcgen(''' > + qmp_register_command(cmds, "%(name)s", This is changing the indentation of generated output; everything within the mcgen() should be indented according to the output level, not the current Python nesting of the source file. > + qmp_marshal_%(c_name)s, %(opts)s, > + NULL); > + ''', > + name=name, c_name=c_name(name), > + opts=options) > + else: > + ret = mcgen(''' > + static const RunState qmp_valid_states_%(c_name)s[] = { > +''' > + , c_name=c_name(name)) Unusual placement of the , between args to mcgen(). > + > + for value in runstates: > + ret += mcgen(''' > + %(c_enum)s, > +''' > + , c_enum=c_enum_const('RunState', value)) > + > + ret += mcgen(''' > + %(c_enum)s, > + }; > +''' > + , c_enum=c_enum_const('RunState', "_MAX")) > + > + ret += mcgen(''' > + qmp_register_command(cmds, "%(name)s", > + qmp_marshal_%(c_name)s, %(opts)s, > + qmp_valid_states_%(c_name)s); > + ''', > + name=name, c_name=c_name(name), > + opts=options) > + > return ret > > > @@ -241,7 +269,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor): > self._visited_ret_types = None > > def visit_command(self, name, info, arg_type, ret_type, > - gen, success_response, boxed): > + gen, success_response, boxed, runstates): > if not gen: > return > self.decl += gen_command_decl(name, arg_type, boxed, ret_type) > @@ -250,7 +278,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor): > self.defn += gen_marshal_output(ret_type) > self.decl += gen_marshal_decl(name) > self.defn += gen_marshal(name, arg_type, boxed, ret_type) > - self._regy += gen_register_command(name, success_response) > + self._regy += gen_register_command(name, success_response, runstates) Yeah, we'll definitely need to rebase this on top of Markus' work to modularize QAPI output files. > +++ b/scripts/qapi.py > @@ -919,7 +919,8 @@ def check_exprs(exprs): > elif 'command' in expr: > meta = 'command' > check_keys(expr_elem, 'command', [], > - ['data', 'returns', 'gen', 'success-response', 'boxed']) > + ['data', 'returns', 'gen', 'success-response', 'boxed', > + 'runstates']) 'runstates' is specific to QMP, and doesn't really apply to QGA, right? That makes me wonder if it is really the best interface to be exposing to the QAPI generator. Certainly, a single bool that states whether a command is allowed in preconfig is a bit more extensible to both QGA and QMP, when compared to a list of QMP-specific states. > @@ -1639,6 +1642,7 @@ class QAPISchema(object): > gen = expr.get('gen', True) > success_response = expr.get('success-response', True) > boxed = expr.get('boxed', False) > + runstates = expr.get('runstates') > if isinstance(data, OrderedDict): > data = self._make_implicit_object_type( > name, info, doc, 'arg', self._make_members(data, info)) > @@ -1646,7 +1650,8 @@ class QAPISchema(object): > assert len(rets) == 1 > rets = self._make_array_type(rets[0], info) > self._def_entity(QAPISchemaCommand(name, info, doc, data, rets, > - gen, success_response, boxed)) > + gen, success_response, boxed, > + runstates)) Where do we validate that the list of states passed in the QAPI file actually makes sense (and that the user didn't typo anything)? That's another argument for a bool flag rather than an array of states, as it is easier to validate that a bool was set correctly rather than that a list doesn't introduce bogus states. > +++ b/tests/qapi-schema/doc-good.out > @@ -18,9 +18,9 @@ object Variant1 > member var1: str optional=False > object Variant2 > command cmd q_obj_cmd-arg -> Object > - gen=True success_response=True boxed=False > + gen=True success_response=True boxed=False, runstates=None Inconsistent on whether output arguments are separated by comma... > +++ b/tests/qapi-schema/test-qapi.py > @@ -37,11 +37,11 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): > self._print_variants(variants) > > def visit_command(self, name, info, arg_type, ret_type, > - gen, success_response, boxed): > + gen, success_response, boxed, runstates): > 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' % \ > - (gen, success_response, boxed)) > + print(' gen=%s success_response=%s boxed=%s, runstates=%s' % \ ...here Also, while you did add coverage of the new information in the successful tests, you didn't add any negative tests to check diagnosis messages emitted when the field is present in the QAPI schema but doesn't make sense, and you didn't modify any of the test-only QAPI commands to use non-default states (which means only the live QMP commands test the new feature). I would have expected at least a change in tests/qapi-schema/qapi-schema-test.json. Overall, this is adding a lot of complexity to QMP; are we sure this is the interface libvirt wants to use for early NUMA configuration? Can we simplify it to just a new bool, similar to 'allow-oob'?
On Tue, 27 Feb 2018 16:10:31 -0600 Eric Blake <eblake@redhat.com> wrote: > On 02/16/2018 06:37 AM, Igor Mammedov wrote: > > Add optional 'runstates' parameter in QAPI command definition, > > which will permit to specify RunState variations in which > > a command could be exectuted via QMP monitor. > > s/exectuted/executed/ > > > > > For compatibility reasons, commands, that don't use > > s/commands,/commands/ > > > 'runstates' explicitly, are not permited to run in > > s/explicitly,/explicitly/ > s/permited/permitted/ > > > preconfig state but allowed in all other states. > > > > New option will be used to allow commands, which are > > prepared/need to run this early, to run in preconfig state. > > It will include query-hotpluggable-cpus and new set-numa-node > > commands. Other commands that should be able to run in > > preconfig state, should be ammeded to not expect machine > > s/ammeded/amended/ > > > in initialized state. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > include/qapi/qmp/dispatch.h | 5 +++- > > monitor.c | 28 +++++++++++++++++--- > > qapi-schema.json | 12 +++++++-- > > qapi/qmp-dispatch.c | 39 ++++++++++++++++++++++++++++ > > qapi/qmp-registry.c | 4 ++- > > qapi/run-state.json | 6 ++++- > > scripts/qapi-commands.py | 46 ++++++++++++++++++++++++++------- > > scripts/qapi-introspect.py | 2 +- > > scripts/qapi.py | 15 +++++++---- > > scripts/qapi2texi.py | 2 +- > > 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 | 18 ++++++------- > > tests/qapi-schema/test-qapi.py | 6 ++--- > > 15 files changed, 151 insertions(+), 42 deletions(-) > [...] > Overall, this is adding a lot of complexity to QMP; are we sure this is > the interface libvirt wants to use for early NUMA configuration? Can we > simplify it to just a new bool, similar to 'allow-oob'? Being unsure if preconfig only specific knob would fly, I was trying to introduce a generic mechanism that could be used to limit any command to certain runstates. I'd gladly to scrape it off and implement something like 'allow-oob' if that's acceptable. It could be "preconfig-enabled" which defaults to false and necessary commands would be explicitly marked as preconfig enabled. Then I could just build into 'set-numa-node' C callback an extra check to limit it only to preconfig state, like we typically do in other handlers instead of going for more complex generic approach. That would significantly simplify series.
* Igor Mammedov (imammedo@redhat.com) wrote: > Add optional 'runstates' parameter in QAPI command definition, > which will permit to specify RunState variations in which > a command could be exectuted via QMP monitor. > > For compatibility reasons, commands, that don't use > 'runstates' explicitly, are not permited to run in > preconfig state but allowed in all other states. > > New option will be used to allow commands, which are > prepared/need to run this early, to run in preconfig state. > It will include query-hotpluggable-cpus and new set-numa-node > commands. Other commands that should be able to run in > preconfig state, should be ammeded to not expect machine > in initialized state. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > include/qapi/qmp/dispatch.h | 5 +++- > monitor.c | 28 +++++++++++++++++--- > qapi-schema.json | 12 +++++++-- > qapi/qmp-dispatch.c | 39 ++++++++++++++++++++++++++++ > qapi/qmp-registry.c | 4 ++- > qapi/run-state.json | 6 ++++- > scripts/qapi-commands.py | 46 ++++++++++++++++++++++++++------- > scripts/qapi-introspect.py | 2 +- > scripts/qapi.py | 15 +++++++---- > scripts/qapi2texi.py | 2 +- > 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 | 18 ++++++------- > tests/qapi-schema/test-qapi.py | 6 ++--- > 15 files changed, 151 insertions(+), 42 deletions(-) > > diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h > index 1e694b5..f821781 100644 > --- a/include/qapi/qmp/dispatch.h > +++ b/include/qapi/qmp/dispatch.h > @@ -15,6 +15,7 @@ > #define QAPI_QMP_DISPATCH_H > > #include "qemu/queue.h" > +#include "qapi-types.h" > > typedef void (QmpCommandFunc)(QDict *, QObject **, Error **); > > @@ -31,12 +32,14 @@ typedef struct QmpCommand > QmpCommandOptions options; > QTAILQ_ENTRY(QmpCommand) node; > bool enabled; > + const RunState *valid_runstates; > } QmpCommand; > > typedef QTAILQ_HEAD(QmpCommandList, QmpCommand) QmpCommandList; > > void qmp_register_command(QmpCommandList *cmds, const char *name, > - QmpCommandFunc *fn, QmpCommandOptions options); > + QmpCommandFunc *fn, QmpCommandOptions options, > + const RunState valid_runstates[]); > void qmp_unregister_command(QmpCommandList *cmds, const char *name); > QmpCommand *qmp_find_command(QmpCommandList *cmds, const char *name); > QObject *qmp_dispatch(QmpCommandList *cmds, QObject *request); > diff --git a/monitor.c b/monitor.c > index fcb5386..2edc96c 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -224,6 +224,25 @@ static mon_cmd_t mon_cmds[]; > static mon_cmd_t info_cmds[]; > > QmpCommandList qmp_commands, qmp_cap_negotiation_commands; > +const RunState cap_negotiation_valid_runstates[] = { > + RUN_STATE_DEBUG, > + RUN_STATE_INMIGRATE, > + RUN_STATE_INTERNAL_ERROR, > + RUN_STATE_IO_ERROR, > + RUN_STATE_PAUSED, > + RUN_STATE_POSTMIGRATE, > + RUN_STATE_PRELAUNCH, > + RUN_STATE_FINISH_MIGRATE, > + RUN_STATE_RESTORE_VM, > + RUN_STATE_RUNNING, > + RUN_STATE_SAVE_VM, > + RUN_STATE_SHUTDOWN, > + RUN_STATE_SUSPENDED, > + RUN_STATE_WATCHDOG, > + RUN_STATE_GUEST_PANICKED, > + RUN_STATE_COLO, > + RUN_STATE_PRECONFIG, > +}; It's a shame this is such a manual copy. While you're taking a big hammer to HMP in the preconfig case, it's worth noting this more specific code is only protecting QMP commands. It's a shame in all the uses below we can't be working with bitmasks of run-state's; it would feel a lot easier to have a default and mask out or or in extra states. Dave > Monitor *cur_mon; > > @@ -1016,17 +1035,18 @@ void monitor_init_qmp_commands(void) > > qmp_register_command(&qmp_commands, "query-qmp-schema", > qmp_query_qmp_schema, > - QCO_NO_OPTIONS); > + QCO_NO_OPTIONS, NULL); > qmp_register_command(&qmp_commands, "device_add", qmp_device_add, > - QCO_NO_OPTIONS); > + QCO_NO_OPTIONS, NULL); > qmp_register_command(&qmp_commands, "netdev_add", qmp_netdev_add, > - QCO_NO_OPTIONS); > + QCO_NO_OPTIONS, NULL); > > qmp_unregister_commands_hack(); > > 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_NO_OPTIONS, > + cap_negotiation_valid_runstates); > } > > void qmp_qmp_capabilities(Error **errp) > diff --git a/qapi-schema.json b/qapi-schema.json > index 0262b9f..ee1053d 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -219,7 +219,11 @@ > # Note: This example has been shortened as the real response is too long. > # > ## > -{ 'command': 'query-commands', 'returns': ['CommandInfo'] } > +{ 'command': 'query-commands', 'returns': ['CommandInfo'], > + 'runstates': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', > + 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', > + 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog', > + 'guest-panicked', 'colo', 'preconfig' ] } That's going to get to be a pain to update as we add more states. > ## > # @LostTickPolicy: > @@ -1146,7 +1150,11 @@ > # <- { "return": {} } > # > ## > -{ 'command': 'cont' } > +{ 'command': 'cont', > + 'runstates': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', > + 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', > + 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog', > + 'guest-panicked', 'colo', 'preconfig' ] } > > ## > # @system_wakeup: > diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c > index e31ac4b..26cba19 100644 > --- a/qapi/qmp-dispatch.c > +++ b/qapi/qmp-dispatch.c > @@ -17,6 +17,7 @@ > #include "qapi/qmp/json-parser.h" > #include "qapi/qmp/qdict.h" > #include "qapi/qmp/qjson.h" > +#include "sysemu/sysemu.h" > > static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) > { > @@ -65,6 +66,40 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) > return dict; > } > > +static bool is_cmd_permited(const QmpCommand *cmd, Error **errp) > +{ > + int i; > + char *list = NULL; > + > + /* Old commands that don't have explicit runstate in schema > + * are permited to run except of at PRECONFIG stage > + */ > + if (!cmd->valid_runstates) { > + if (runstate_check(RUN_STATE_PRECONFIG)) { > + const char *state = RunState_str(RUN_STATE_PRECONFIG); > + error_setg(errp, "The command '%s' isn't valid in '%s'", > + cmd->name, state); > + return false; > + } > + return true; > + } > + > + for (i = 0; cmd->valid_runstates[i] != RUN_STATE__MAX; i++) { > + if (runstate_check(cmd->valid_runstates[i])) { > + return true; > + } > + } > + > + for (i = 0; cmd->valid_runstates[i] != RUN_STATE__MAX; i++) { > + const char *state = RunState_str(cmd->valid_runstates[i]); > + list = g_strjoin(", ", state, list, NULL); > + } > + error_setg(errp, "The command '%s' is valid only in '%s'", cmd->name, list); > + g_free(list); > + > + return false; > +} > + > static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request, > Error **errp) > { > @@ -92,6 +127,10 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request, > return NULL; > } > > + if (!is_cmd_permited(cmd, errp)) { > + return NULL; > + } > + > if (!qdict_haskey(dict, "arguments")) { > args = qdict_new(); > } else { > diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c > index 5af484c..59a5b85 100644 > --- a/qapi/qmp-registry.c > +++ b/qapi/qmp-registry.c > @@ -16,7 +16,8 @@ > #include "qapi/qmp/dispatch.h" > > void qmp_register_command(QmpCommandList *cmds, const char *name, > - QmpCommandFunc *fn, QmpCommandOptions options) > + QmpCommandFunc *fn, QmpCommandOptions options, > + const RunState valid_runstates[]) > { > QmpCommand *cmd = g_malloc0(sizeof(*cmd)); > > @@ -24,6 +25,7 @@ void qmp_register_command(QmpCommandList *cmds, const char *name, > cmd->fn = fn; > cmd->enabled = true; > cmd->options = options; > + cmd->valid_runstates = valid_runstates; > QTAILQ_INSERT_TAIL(cmds, cmd, node); > } > > diff --git a/qapi/run-state.json b/qapi/run-state.json > index d24a4e8..1c1c9b8 100644 > --- a/qapi/run-state.json > +++ b/qapi/run-state.json > @@ -92,7 +92,11 @@ > # "status": "running" } } > # > ## > -{ 'command': 'query-status', 'returns': 'StatusInfo' } > +{ 'command': 'query-status', 'returns': 'StatusInfo', > + 'runstates': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', > + 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', > + 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog', > + 'guest-panicked', 'colo', 'preconfig' ] } > > ## > # @SHUTDOWN: > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > index f89d748..659e167 100644 > --- a/scripts/qapi-commands.py > +++ b/scripts/qapi-commands.py > @@ -192,17 +192,45 @@ out: > return ret > > > -def gen_register_command(name, success_response): > +def gen_register_command(name, success_response, runstates): > options = 'QCO_NO_OPTIONS' > if not success_response: > options = 'QCO_NO_SUCCESS_RESP' > > - ret = mcgen(''' > - qmp_register_command(cmds, "%(name)s", > - qmp_marshal_%(c_name)s, %(opts)s); > -''', > - name=name, c_name=c_name(name), > - opts=options) > + if runstates is None: > + ret = mcgen(''' > + qmp_register_command(cmds, "%(name)s", > + qmp_marshal_%(c_name)s, %(opts)s, > + NULL); > + ''', > + name=name, c_name=c_name(name), > + opts=options) > + else: > + ret = mcgen(''' > + static const RunState qmp_valid_states_%(c_name)s[] = { > +''' > + , c_name=c_name(name)) > + > + for value in runstates: > + ret += mcgen(''' > + %(c_enum)s, > +''' > + , c_enum=c_enum_const('RunState', value)) > + > + ret += mcgen(''' > + %(c_enum)s, > + }; > +''' > + , c_enum=c_enum_const('RunState', "_MAX")) > + > + ret += mcgen(''' > + qmp_register_command(cmds, "%(name)s", > + qmp_marshal_%(c_name)s, %(opts)s, > + qmp_valid_states_%(c_name)s); > + ''', > + name=name, c_name=c_name(name), > + opts=options) > + > return ret > > > @@ -241,7 +269,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor): > self._visited_ret_types = None > > def visit_command(self, name, info, arg_type, ret_type, > - gen, success_response, boxed): > + gen, success_response, boxed, runstates): > if not gen: > return > self.decl += gen_command_decl(name, arg_type, boxed, ret_type) > @@ -250,7 +278,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor): > self.defn += gen_marshal_output(ret_type) > self.decl += gen_marshal_decl(name) > self.defn += gen_marshal(name, arg_type, boxed, ret_type) > - self._regy += gen_register_command(name, success_response) > + self._regy += gen_register_command(name, success_response, runstates) > > > (input_file, output_dir, do_c, do_h, prefix, opts) = parse_command_line() > diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py > index 032bcea..ede86ac 100644 > --- a/scripts/qapi-introspect.py > +++ b/scripts/qapi-introspect.py > @@ -154,7 +154,7 @@ const char %(c_name)s[] = %(c_string)s; > for m in variants.variants]}) > > def visit_command(self, name, info, arg_type, ret_type, > - gen, success_response, boxed): > + gen, success_response, boxed, runstates): > arg_type = arg_type or self._schema.the_empty_object_type > ret_type = ret_type or self._schema.the_empty_object_type > self._gen_json(name, 'command', > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 58f995b..5c5037e 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -919,7 +919,8 @@ def check_exprs(exprs): > elif 'command' in expr: > meta = 'command' > check_keys(expr_elem, 'command', [], > - ['data', 'returns', 'gen', 'success-response', 'boxed']) > + ['data', 'returns', 'gen', 'success-response', 'boxed', > + 'runstates']) > elif 'event' in expr: > meta = 'event' > check_keys(expr_elem, 'event', [], ['data', 'boxed']) > @@ -1030,7 +1031,7 @@ class QAPISchemaVisitor(object): > pass > > def visit_command(self, name, info, arg_type, ret_type, > - gen, success_response, boxed): > + gen, success_response, boxed, runstates): > pass > > def visit_event(self, name, info, arg_type, boxed): > @@ -1397,7 +1398,7 @@ class QAPISchemaAlternateType(QAPISchemaType): > > class QAPISchemaCommand(QAPISchemaEntity): > def __init__(self, name, info, doc, arg_type, ret_type, > - gen, success_response, boxed): > + gen, success_response, boxed, runstates): > QAPISchemaEntity.__init__(self, name, info, doc) > assert not arg_type or isinstance(arg_type, str) > assert not ret_type or isinstance(ret_type, str) > @@ -1408,6 +1409,7 @@ class QAPISchemaCommand(QAPISchemaEntity): > self.gen = gen > self.success_response = success_response > self.boxed = boxed > + self.runstates = runstates > > def check(self, schema): > if self._arg_type_name: > @@ -1431,7 +1433,8 @@ class QAPISchemaCommand(QAPISchemaEntity): > def visit(self, visitor): > visitor.visit_command(self.name, self.info, > self.arg_type, self.ret_type, > - self.gen, self.success_response, self.boxed) > + self.gen, self.success_response, self.boxed, > + self.runstates) > > > class QAPISchemaEvent(QAPISchemaEntity): > @@ -1639,6 +1642,7 @@ class QAPISchema(object): > gen = expr.get('gen', True) > success_response = expr.get('success-response', True) > boxed = expr.get('boxed', False) > + runstates = expr.get('runstates') > if isinstance(data, OrderedDict): > data = self._make_implicit_object_type( > name, info, doc, 'arg', self._make_members(data, info)) > @@ -1646,7 +1650,8 @@ class QAPISchema(object): > assert len(rets) == 1 > rets = self._make_array_type(rets[0], info) > self._def_entity(QAPISchemaCommand(name, info, doc, data, rets, > - gen, success_response, boxed)) > + gen, success_response, boxed, > + runstates)) > > def _def_event(self, expr, info, doc): > name = expr['event'] > diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py > index bf1c57b..d28594c 100755 > --- a/scripts/qapi2texi.py > +++ b/scripts/qapi2texi.py > @@ -227,7 +227,7 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor): > body=texi_entity(doc, 'Members')) > > def visit_command(self, name, info, arg_type, ret_type, > - gen, success_response, boxed): > + gen, success_response, boxed, runstates): > doc = self.cur_doc > if boxed: > body = texi_body(doc) > diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out > index 1d2c250..a47d6f6 100644 > --- a/tests/qapi-schema/doc-good.out > +++ b/tests/qapi-schema/doc-good.out > @@ -18,9 +18,9 @@ object Variant1 > member var1: str optional=False > object Variant2 > command cmd q_obj_cmd-arg -> Object > - gen=True success_response=True boxed=False > + gen=True success_response=True boxed=False, runstates=None > command cmd-boxed Object -> None > - gen=True success_response=True boxed=True > + gen=True success_response=True boxed=True, runstates=None > object q_empty > object q_obj_Variant1-wrapper > member data: Variant1 optional=False > diff --git a/tests/qapi-schema/ident-with-escape.out b/tests/qapi-schema/ident-with-escape.out > index b5637cb..d9a272b 100644 > --- a/tests/qapi-schema/ident-with-escape.out > +++ b/tests/qapi-schema/ident-with-escape.out > @@ -1,7 +1,7 @@ > enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool'] > prefix QTYPE > command fooA q_obj_fooA-arg -> None > - gen=True success_response=True boxed=False > + gen=True success_response=True boxed=False, runstates=None > object q_empty > object q_obj_fooA-arg > member bar1: str optional=False > diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out > index 586795f..4b128d5 100644 > --- a/tests/qapi-schema/indented-expr.out > +++ b/tests/qapi-schema/indented-expr.out > @@ -1,7 +1,7 @@ > enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool'] > prefix QTYPE > command eins None -> None > - gen=True success_response=True boxed=False > + gen=True success_response=True boxed=False, runstates=None > object q_empty > command zwei None -> None > - gen=True success_response=True boxed=False > + gen=True success_response=True boxed=False, runstates=None > diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out > index 3b1e908..787c228 100644 > --- a/tests/qapi-schema/qapi-schema-test.out > +++ b/tests/qapi-schema/qapi-schema-test.out > @@ -153,15 +153,15 @@ object __org.qemu_x-Union2 > tag __org.qemu_x-member1 > case __org.qemu_x-value: __org.qemu_x-Struct2 > command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Union1 > - gen=True success_response=True boxed=False > + gen=True success_response=True boxed=False, runstates=None > command boxed-struct UserDefZero -> None > - gen=True success_response=True boxed=True > + gen=True success_response=True boxed=True, runstates=None > command boxed-union UserDefNativeListUnion -> None > - gen=True success_response=True boxed=True > + gen=True success_response=True boxed=True, runstates=None > command guest-get-time q_obj_guest-get-time-arg -> int > - gen=True success_response=True boxed=False > + gen=True success_response=True boxed=False, runstates=None > command guest-sync q_obj_guest-sync-arg -> any > - gen=True success_response=True boxed=False > + gen=True success_response=True boxed=False, runstates=None > object q_empty > object q_obj_EVENT_C-arg > member a: int optional=True > @@ -222,10 +222,10 @@ object q_obj_user_def_cmd2-arg > member ud1a: UserDefOne optional=False > member ud1b: UserDefOne optional=True > command user_def_cmd None -> None > - gen=True success_response=True boxed=False > + gen=True success_response=True boxed=False, runstates=None > command user_def_cmd0 Empty2 -> Empty2 > - gen=True success_response=True boxed=False > + gen=True success_response=True boxed=False, runstates=None > command user_def_cmd1 q_obj_user_def_cmd1-arg -> None > - gen=True success_response=True boxed=False > + gen=True success_response=True boxed=False, runstates=None > command user_def_cmd2 q_obj_user_def_cmd2-arg -> UserDefTwo > - gen=True success_response=True boxed=False > + gen=True success_response=True boxed=False, runstates=None > diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py > index ac43d34..958576d 100644 > --- a/tests/qapi-schema/test-qapi.py > +++ b/tests/qapi-schema/test-qapi.py > @@ -37,11 +37,11 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): > self._print_variants(variants) > > def visit_command(self, name, info, arg_type, ret_type, > - gen, success_response, boxed): > + gen, success_response, boxed, runstates): > 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' % \ > - (gen, success_response, boxed)) > + print(' gen=%s success_response=%s boxed=%s, runstates=%s' % \ > + (gen, success_response, boxed, runstates)) > > def visit_event(self, name, info, arg_type, boxed): > print('event %s %s' % (name, arg_type and arg_type.name)) > -- > 2.7.4 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Wed, 7 Mar 2018 14:16:50 +0000 "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Igor Mammedov (imammedo@redhat.com) wrote: > > Add optional 'runstates' parameter in QAPI command definition, > > which will permit to specify RunState variations in which > > a command could be exectuted via QMP monitor. > > > > For compatibility reasons, commands, that don't use > > 'runstates' explicitly, are not permited to run in > > preconfig state but allowed in all other states. > > > > New option will be used to allow commands, which are > > prepared/need to run this early, to run in preconfig state. > > It will include query-hotpluggable-cpus and new set-numa-node > > commands. Other commands that should be able to run in > > preconfig state, should be ammeded to not expect machine > > in initialized state. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- [...] > > @@ -224,6 +224,25 @@ static mon_cmd_t mon_cmds[]; > > static mon_cmd_t info_cmds[]; > > > > QmpCommandList qmp_commands, qmp_cap_negotiation_commands; > > +const RunState cap_negotiation_valid_runstates[] = { > > + RUN_STATE_DEBUG, > > + RUN_STATE_INMIGRATE, > > + RUN_STATE_INTERNAL_ERROR, > > + RUN_STATE_IO_ERROR, > > + RUN_STATE_PAUSED, > > + RUN_STATE_POSTMIGRATE, > > + RUN_STATE_PRELAUNCH, > > + RUN_STATE_FINISH_MIGRATE, > > + RUN_STATE_RESTORE_VM, > > + RUN_STATE_RUNNING, > > + RUN_STATE_SAVE_VM, > > + RUN_STATE_SHUTDOWN, > > + RUN_STATE_SUSPENDED, > > + RUN_STATE_WATCHDOG, > > + RUN_STATE_GUEST_PANICKED, > > + RUN_STATE_COLO, > > + RUN_STATE_PRECONFIG, > > +}; > > It's a shame this is such a manual copy. > > While you're taking a big hammer to HMP in the preconfig case, > it's worth noting this more specific code is only protecting QMP > commands. > > > It's a shame in all the uses below we can't be working with bitmasks > of run-state's; it would feel a lot easier to have a default and mask > out or or in extra states. > > Dave [...] > > @@ -219,7 +219,11 @@ > > # Note: This example has been shortened as the real response is too long. > > # > > ## > > -{ 'command': 'query-commands', 'returns': ['CommandInfo'] } > > +{ 'command': 'query-commands', 'returns': ['CommandInfo'], > > + 'runstates': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', > > + 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', > > + 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog', > > + 'guest-panicked', 'colo', 'preconfig' ] } > > That's going to get to be a pain to update as we add more states. [...] Yep it would be a pain so, In v4 this approach/patch is replaced by a simpler one that Eric's suggested. It will provide preconfig specific flag in command, similar to what allow-oob patches on list do. So it would look like following: ------------------------ docs/devel/qapi-code-gen.txt ------------------------- index 25b7180..170f15f 100644 @@ -556,7 +556,8 @@ following example objects: Usage: { 'command': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT, '*returns': TYPE-NAME, '*boxed': true, - '*gen': false, '*success-response': false } + '*gen': false, '*success-response': false, + '*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 @@ -636,6 +637,13 @@ possible, the command expression should include the optional key 'success-response' with boolean value false. So far, only QGA makes use of this member. +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 }
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h index 1e694b5..f821781 100644 --- a/include/qapi/qmp/dispatch.h +++ b/include/qapi/qmp/dispatch.h @@ -15,6 +15,7 @@ #define QAPI_QMP_DISPATCH_H #include "qemu/queue.h" +#include "qapi-types.h" typedef void (QmpCommandFunc)(QDict *, QObject **, Error **); @@ -31,12 +32,14 @@ typedef struct QmpCommand QmpCommandOptions options; QTAILQ_ENTRY(QmpCommand) node; bool enabled; + const RunState *valid_runstates; } QmpCommand; typedef QTAILQ_HEAD(QmpCommandList, QmpCommand) QmpCommandList; void qmp_register_command(QmpCommandList *cmds, const char *name, - QmpCommandFunc *fn, QmpCommandOptions options); + QmpCommandFunc *fn, QmpCommandOptions options, + const RunState valid_runstates[]); void qmp_unregister_command(QmpCommandList *cmds, const char *name); QmpCommand *qmp_find_command(QmpCommandList *cmds, const char *name); QObject *qmp_dispatch(QmpCommandList *cmds, QObject *request); diff --git a/monitor.c b/monitor.c index fcb5386..2edc96c 100644 --- a/monitor.c +++ b/monitor.c @@ -224,6 +224,25 @@ static mon_cmd_t mon_cmds[]; static mon_cmd_t info_cmds[]; QmpCommandList qmp_commands, qmp_cap_negotiation_commands; +const RunState cap_negotiation_valid_runstates[] = { + RUN_STATE_DEBUG, + RUN_STATE_INMIGRATE, + RUN_STATE_INTERNAL_ERROR, + RUN_STATE_IO_ERROR, + RUN_STATE_PAUSED, + RUN_STATE_POSTMIGRATE, + RUN_STATE_PRELAUNCH, + RUN_STATE_FINISH_MIGRATE, + RUN_STATE_RESTORE_VM, + RUN_STATE_RUNNING, + RUN_STATE_SAVE_VM, + RUN_STATE_SHUTDOWN, + RUN_STATE_SUSPENDED, + RUN_STATE_WATCHDOG, + RUN_STATE_GUEST_PANICKED, + RUN_STATE_COLO, + RUN_STATE_PRECONFIG, +}; Monitor *cur_mon; @@ -1016,17 +1035,18 @@ void monitor_init_qmp_commands(void) qmp_register_command(&qmp_commands, "query-qmp-schema", qmp_query_qmp_schema, - QCO_NO_OPTIONS); + QCO_NO_OPTIONS, NULL); qmp_register_command(&qmp_commands, "device_add", qmp_device_add, - QCO_NO_OPTIONS); + QCO_NO_OPTIONS, NULL); qmp_register_command(&qmp_commands, "netdev_add", qmp_netdev_add, - QCO_NO_OPTIONS); + QCO_NO_OPTIONS, NULL); qmp_unregister_commands_hack(); 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_NO_OPTIONS, + cap_negotiation_valid_runstates); } void qmp_qmp_capabilities(Error **errp) diff --git a/qapi-schema.json b/qapi-schema.json index 0262b9f..ee1053d 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -219,7 +219,11 @@ # Note: This example has been shortened as the real response is too long. # ## -{ 'command': 'query-commands', 'returns': ['CommandInfo'] } +{ 'command': 'query-commands', 'returns': ['CommandInfo'], + 'runstates': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', + 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', + 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog', + 'guest-panicked', 'colo', 'preconfig' ] } ## # @LostTickPolicy: @@ -1146,7 +1150,11 @@ # <- { "return": {} } # ## -{ 'command': 'cont' } +{ 'command': 'cont', + 'runstates': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', + 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', + 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog', + 'guest-panicked', 'colo', 'preconfig' ] } ## # @system_wakeup: diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index e31ac4b..26cba19 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -17,6 +17,7 @@ #include "qapi/qmp/json-parser.h" #include "qapi/qmp/qdict.h" #include "qapi/qmp/qjson.h" +#include "sysemu/sysemu.h" static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) { @@ -65,6 +66,40 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) return dict; } +static bool is_cmd_permited(const QmpCommand *cmd, Error **errp) +{ + int i; + char *list = NULL; + + /* Old commands that don't have explicit runstate in schema + * are permited to run except of at PRECONFIG stage + */ + if (!cmd->valid_runstates) { + if (runstate_check(RUN_STATE_PRECONFIG)) { + const char *state = RunState_str(RUN_STATE_PRECONFIG); + error_setg(errp, "The command '%s' isn't valid in '%s'", + cmd->name, state); + return false; + } + return true; + } + + for (i = 0; cmd->valid_runstates[i] != RUN_STATE__MAX; i++) { + if (runstate_check(cmd->valid_runstates[i])) { + return true; + } + } + + for (i = 0; cmd->valid_runstates[i] != RUN_STATE__MAX; i++) { + const char *state = RunState_str(cmd->valid_runstates[i]); + list = g_strjoin(", ", state, list, NULL); + } + error_setg(errp, "The command '%s' is valid only in '%s'", cmd->name, list); + g_free(list); + + return false; +} + static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request, Error **errp) { @@ -92,6 +127,10 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request, return NULL; } + if (!is_cmd_permited(cmd, errp)) { + return NULL; + } + if (!qdict_haskey(dict, "arguments")) { args = qdict_new(); } else { diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c index 5af484c..59a5b85 100644 --- a/qapi/qmp-registry.c +++ b/qapi/qmp-registry.c @@ -16,7 +16,8 @@ #include "qapi/qmp/dispatch.h" void qmp_register_command(QmpCommandList *cmds, const char *name, - QmpCommandFunc *fn, QmpCommandOptions options) + QmpCommandFunc *fn, QmpCommandOptions options, + const RunState valid_runstates[]) { QmpCommand *cmd = g_malloc0(sizeof(*cmd)); @@ -24,6 +25,7 @@ void qmp_register_command(QmpCommandList *cmds, const char *name, cmd->fn = fn; cmd->enabled = true; cmd->options = options; + cmd->valid_runstates = valid_runstates; QTAILQ_INSERT_TAIL(cmds, cmd, node); } diff --git a/qapi/run-state.json b/qapi/run-state.json index d24a4e8..1c1c9b8 100644 --- a/qapi/run-state.json +++ b/qapi/run-state.json @@ -92,7 +92,11 @@ # "status": "running" } } # ## -{ 'command': 'query-status', 'returns': 'StatusInfo' } +{ 'command': 'query-status', 'returns': 'StatusInfo', + 'runstates': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', + 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', + 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog', + 'guest-panicked', 'colo', 'preconfig' ] } ## # @SHUTDOWN: diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index f89d748..659e167 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -192,17 +192,45 @@ out: return ret -def gen_register_command(name, success_response): +def gen_register_command(name, success_response, runstates): options = 'QCO_NO_OPTIONS' if not success_response: options = 'QCO_NO_SUCCESS_RESP' - ret = mcgen(''' - qmp_register_command(cmds, "%(name)s", - qmp_marshal_%(c_name)s, %(opts)s); -''', - name=name, c_name=c_name(name), - opts=options) + if runstates is None: + ret = mcgen(''' + qmp_register_command(cmds, "%(name)s", + qmp_marshal_%(c_name)s, %(opts)s, + NULL); + ''', + name=name, c_name=c_name(name), + opts=options) + else: + ret = mcgen(''' + static const RunState qmp_valid_states_%(c_name)s[] = { +''' + , c_name=c_name(name)) + + for value in runstates: + ret += mcgen(''' + %(c_enum)s, +''' + , c_enum=c_enum_const('RunState', value)) + + ret += mcgen(''' + %(c_enum)s, + }; +''' + , c_enum=c_enum_const('RunState', "_MAX")) + + ret += mcgen(''' + qmp_register_command(cmds, "%(name)s", + qmp_marshal_%(c_name)s, %(opts)s, + qmp_valid_states_%(c_name)s); + ''', + name=name, c_name=c_name(name), + opts=options) + return ret @@ -241,7 +269,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor): self._visited_ret_types = None def visit_command(self, name, info, arg_type, ret_type, - gen, success_response, boxed): + gen, success_response, boxed, runstates): if not gen: return self.decl += gen_command_decl(name, arg_type, boxed, ret_type) @@ -250,7 +278,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor): self.defn += gen_marshal_output(ret_type) self.decl += gen_marshal_decl(name) self.defn += gen_marshal(name, arg_type, boxed, ret_type) - self._regy += gen_register_command(name, success_response) + self._regy += gen_register_command(name, success_response, runstates) (input_file, output_dir, do_c, do_h, prefix, opts) = parse_command_line() diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py index 032bcea..ede86ac 100644 --- a/scripts/qapi-introspect.py +++ b/scripts/qapi-introspect.py @@ -154,7 +154,7 @@ const char %(c_name)s[] = %(c_string)s; for m in variants.variants]}) def visit_command(self, name, info, arg_type, ret_type, - gen, success_response, boxed): + gen, success_response, boxed, runstates): arg_type = arg_type or self._schema.the_empty_object_type ret_type = ret_type or self._schema.the_empty_object_type self._gen_json(name, 'command', diff --git a/scripts/qapi.py b/scripts/qapi.py index 58f995b..5c5037e 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -919,7 +919,8 @@ def check_exprs(exprs): elif 'command' in expr: meta = 'command' check_keys(expr_elem, 'command', [], - ['data', 'returns', 'gen', 'success-response', 'boxed']) + ['data', 'returns', 'gen', 'success-response', 'boxed', + 'runstates']) elif 'event' in expr: meta = 'event' check_keys(expr_elem, 'event', [], ['data', 'boxed']) @@ -1030,7 +1031,7 @@ class QAPISchemaVisitor(object): pass def visit_command(self, name, info, arg_type, ret_type, - gen, success_response, boxed): + gen, success_response, boxed, runstates): pass def visit_event(self, name, info, arg_type, boxed): @@ -1397,7 +1398,7 @@ class QAPISchemaAlternateType(QAPISchemaType): class QAPISchemaCommand(QAPISchemaEntity): def __init__(self, name, info, doc, arg_type, ret_type, - gen, success_response, boxed): + gen, success_response, boxed, runstates): QAPISchemaEntity.__init__(self, name, info, doc) assert not arg_type or isinstance(arg_type, str) assert not ret_type or isinstance(ret_type, str) @@ -1408,6 +1409,7 @@ class QAPISchemaCommand(QAPISchemaEntity): self.gen = gen self.success_response = success_response self.boxed = boxed + self.runstates = runstates def check(self, schema): if self._arg_type_name: @@ -1431,7 +1433,8 @@ class QAPISchemaCommand(QAPISchemaEntity): def visit(self, visitor): visitor.visit_command(self.name, self.info, self.arg_type, self.ret_type, - self.gen, self.success_response, self.boxed) + self.gen, self.success_response, self.boxed, + self.runstates) class QAPISchemaEvent(QAPISchemaEntity): @@ -1639,6 +1642,7 @@ class QAPISchema(object): gen = expr.get('gen', True) success_response = expr.get('success-response', True) boxed = expr.get('boxed', False) + runstates = expr.get('runstates') if isinstance(data, OrderedDict): data = self._make_implicit_object_type( name, info, doc, 'arg', self._make_members(data, info)) @@ -1646,7 +1650,8 @@ class QAPISchema(object): assert len(rets) == 1 rets = self._make_array_type(rets[0], info) self._def_entity(QAPISchemaCommand(name, info, doc, data, rets, - gen, success_response, boxed)) + gen, success_response, boxed, + runstates)) def _def_event(self, expr, info, doc): name = expr['event'] diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py index bf1c57b..d28594c 100755 --- a/scripts/qapi2texi.py +++ b/scripts/qapi2texi.py @@ -227,7 +227,7 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor): body=texi_entity(doc, 'Members')) def visit_command(self, name, info, arg_type, ret_type, - gen, success_response, boxed): + gen, success_response, boxed, runstates): doc = self.cur_doc if boxed: body = texi_body(doc) diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index 1d2c250..a47d6f6 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -18,9 +18,9 @@ object Variant1 member var1: str optional=False object Variant2 command cmd q_obj_cmd-arg -> Object - gen=True success_response=True boxed=False + gen=True success_response=True boxed=False, runstates=None command cmd-boxed Object -> None - gen=True success_response=True boxed=True + gen=True success_response=True boxed=True, runstates=None object q_empty object q_obj_Variant1-wrapper member data: Variant1 optional=False diff --git a/tests/qapi-schema/ident-with-escape.out b/tests/qapi-schema/ident-with-escape.out index b5637cb..d9a272b 100644 --- a/tests/qapi-schema/ident-with-escape.out +++ b/tests/qapi-schema/ident-with-escape.out @@ -1,7 +1,7 @@ enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool'] prefix QTYPE command fooA q_obj_fooA-arg -> None - gen=True success_response=True boxed=False + gen=True success_response=True boxed=False, runstates=None object q_empty object q_obj_fooA-arg member bar1: str optional=False diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out index 586795f..4b128d5 100644 --- a/tests/qapi-schema/indented-expr.out +++ b/tests/qapi-schema/indented-expr.out @@ -1,7 +1,7 @@ enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool'] prefix QTYPE command eins None -> None - gen=True success_response=True boxed=False + gen=True success_response=True boxed=False, runstates=None object q_empty command zwei None -> None - gen=True success_response=True boxed=False + gen=True success_response=True boxed=False, runstates=None diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 3b1e908..787c228 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -153,15 +153,15 @@ object __org.qemu_x-Union2 tag __org.qemu_x-member1 case __org.qemu_x-value: __org.qemu_x-Struct2 command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Union1 - gen=True success_response=True boxed=False + gen=True success_response=True boxed=False, runstates=None command boxed-struct UserDefZero -> None - gen=True success_response=True boxed=True + gen=True success_response=True boxed=True, runstates=None command boxed-union UserDefNativeListUnion -> None - gen=True success_response=True boxed=True + gen=True success_response=True boxed=True, runstates=None command guest-get-time q_obj_guest-get-time-arg -> int - gen=True success_response=True boxed=False + gen=True success_response=True boxed=False, runstates=None command guest-sync q_obj_guest-sync-arg -> any - gen=True success_response=True boxed=False + gen=True success_response=True boxed=False, runstates=None object q_empty object q_obj_EVENT_C-arg member a: int optional=True @@ -222,10 +222,10 @@ object q_obj_user_def_cmd2-arg member ud1a: UserDefOne optional=False member ud1b: UserDefOne optional=True command user_def_cmd None -> None - gen=True success_response=True boxed=False + gen=True success_response=True boxed=False, runstates=None command user_def_cmd0 Empty2 -> Empty2 - gen=True success_response=True boxed=False + gen=True success_response=True boxed=False, runstates=None command user_def_cmd1 q_obj_user_def_cmd1-arg -> None - gen=True success_response=True boxed=False + gen=True success_response=True boxed=False, runstates=None command user_def_cmd2 q_obj_user_def_cmd2-arg -> UserDefTwo - gen=True success_response=True boxed=False + gen=True success_response=True boxed=False, runstates=None diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index ac43d34..958576d 100644 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -37,11 +37,11 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): self._print_variants(variants) def visit_command(self, name, info, arg_type, ret_type, - gen, success_response, boxed): + gen, success_response, boxed, runstates): 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' % \ - (gen, success_response, boxed)) + print(' gen=%s success_response=%s boxed=%s, runstates=%s' % \ + (gen, success_response, boxed, runstates)) def visit_event(self, name, info, arg_type, boxed): print('event %s %s' % (name, arg_type and arg_type.name))
Add optional 'runstates' parameter in QAPI command definition, which will permit to specify RunState variations in which a command could be exectuted via QMP monitor. For compatibility reasons, commands, that don't use 'runstates' explicitly, are not permited to run in preconfig state but allowed in all other states. New option will be used to allow commands, which are prepared/need to run this early, to run in preconfig state. It will include query-hotpluggable-cpus and new set-numa-node commands. Other commands that should be able to run in preconfig state, should be ammeded to not expect machine in initialized state. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- include/qapi/qmp/dispatch.h | 5 +++- monitor.c | 28 +++++++++++++++++--- qapi-schema.json | 12 +++++++-- qapi/qmp-dispatch.c | 39 ++++++++++++++++++++++++++++ qapi/qmp-registry.c | 4 ++- qapi/run-state.json | 6 ++++- scripts/qapi-commands.py | 46 ++++++++++++++++++++++++++------- scripts/qapi-introspect.py | 2 +- scripts/qapi.py | 15 +++++++---- scripts/qapi2texi.py | 2 +- 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 | 18 ++++++------- tests/qapi-schema/test-qapi.py | 6 ++--- 15 files changed, 151 insertions(+), 42 deletions(-)