marcandre.lur...@redhat.com writes:
> From: Marc-André Lureau <marcandre.lur...@redhat.com>
>
> Stop using the so-called 'middle' mode. Instead, use qmp_find_command()
> from generated qapi commands registry.
>
> Note: this commit requires a 'make clean' prior to make, since the
> generated files do not depend on Makefile (due to a cyclic rule
> introduced in 4115852bb0).
We generally say "commit 4115852bb0".
Sounds like we had a cyclic dependency. Do you mean "they don't depend
on Makefile, because that would be a cyclic dependency"?
Paolo, any smart ideas on how to avoid "requires a 'make clean'"?
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> monitor.c | 15 ++++--
> Makefile | 2 +-
> qmp-commands.hx | 143
> --------------------------------------------------------
> vl.c | 1 +
> 4 files changed, 13 insertions(+), 148 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 3a28b43..5bbe4bb 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3934,6 +3934,7 @@ static void handle_qmp_command(JSONMessageParser
> *parser, GQueue *tokens)
> QObject *obj, *data;
> QDict *input, *args;
> const mon_cmd_t *cmd;
> + QmpCommand *qcmd;
> const char *cmd_name;
> Monitor *mon = cur_mon;
>
> @@ -3959,7 +3960,8 @@ static void handle_qmp_command(JSONMessageParser
> *parser, GQueue *tokens)
> cmd_name = qdict_get_str(input, "execute");
> trace_handle_qmp_command(mon, cmd_name);
> cmd = qmp_find_cmd(cmd_name);
> - if (!cmd) {
> + qcmd = qmp_find_command(cmd_name);
> + if (!qcmd || !cmd) {
Looks awkward, but it's temporary. Makes sense.
> error_set(&local_err, ERROR_CLASS_COMMAND_NOT_FOUND,
> "The command %s has not been found", cmd_name);
> goto err_out;
> @@ -3981,7 +3983,7 @@ static void handle_qmp_command(JSONMessageParser
> *parser, GQueue *tokens)
> goto err_out;
> }
>
> - cmd->mhandler.cmd_new(args, &data, &local_err);
> + qcmd->fn(args, &data, &local_err);
>
> err_out:
> monitor_protocol_emitter(mon, data, local_err);
> @@ -4050,10 +4052,15 @@ void monitor_resume(Monitor *mon)
>
> static QObject *get_qmp_greeting(void)
> {
> + QmpCommand *cmd;
> QObject *ver = NULL;
>
> - qmp_marshal_query_version(NULL, &ver, NULL);
> - return qobject_from_jsonf("{'QMP':{'version': %p,'capabilities':
> []}}",ver);
> + cmd = qmp_find_command("query-version");
> + assert(cmd && cmd->fn);
> + cmd->fn(NULL, &ver, NULL);
> +
> + return qobject_from_jsonf("{'QMP':{'version': %p, 'capabilities': []}}",
> + ver);
Meh.
The generator makes the generated qmp_marshal_FOOs() static unless
middle mode. Middle mode is going away (good riddance). But replacing
the linker's work by qmp_find_command() just so we can keep the
qmp_marshal_FOOs() static isn't an improvement. Especially since it
adds another run-time failure mode. Let's change the generator instead.
> }
>
> static void monitor_qmp_event(void *opaque, int event)
> diff --git a/Makefile b/Makefile
> index 0d7647f..fcdc192 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -311,7 +311,7 @@ $(qapi-modules) $(SRC_PATH)/scripts/qapi-event.py
> $(qapi-py)
> qmp-commands.h qmp-marshal.c :\
> $(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
> - $(gen-out-type) -o "." -m $<, \
> + $(gen-out-type) -o "." $<, \
> " GEN $@")
> qmp-introspect.h qmp-introspect.c :\
> $(qapi-modules) $(SRC_PATH)/scripts/qapi-introspect.py $(qapi-py)
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 13707ac..1ad8dda 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -63,7 +63,6 @@ EQMP
> {
> .name = "quit",
> .args_type = "",
> - .mhandler.cmd_new = qmp_marshal_quit,
> },
>
> SQMP
[More of the same snipped...]
> diff --git a/vl.c b/vl.c
> index a455947..a819e05 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2971,6 +2971,7 @@ int main(int argc, char **argv, char **envp)
> qemu_init_exec_dir(argv[0]);
>
> module_call_init(MODULE_INIT_QOM);
> + module_call_init(MODULE_INIT_QAPI);
>
> qemu_add_opts(&qemu_drive_opts);
> qemu_add_drive_opts(&qemu_legacy_drive_opts);
So the code added by PATCH 03 doesn't actually run without this, right?
Okay with me, but let's mention it in the commit message of PATCH 03.
