* Igor Mammedov (imamm...@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 <imamm...@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 / dgilb...@redhat.com / Manchester, UK
