From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34076) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bVeSE-0003hZ-2D for qemu-devel@nongnu.org; Fri, 05 Aug 2016 08:42:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bVeSA-0007IU-QV for qemu-devel@nongnu.org; Fri, 05 Aug 2016 08:42:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38128) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bVeSA-0007IF-IH for qemu-devel@nongnu.org; Fri, 05 Aug 2016 08:42:18 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6C95B63700 for ; Fri, 5 Aug 2016 12:42:17 +0000 (UTC) From: Markus Armbruster References: <20160721140030.28383-1-marcandre.lureau@redhat.com> <20160721140030.28383-6-marcandre.lureau@redhat.com> Date: Fri, 05 Aug 2016 14:42:15 +0200 In-Reply-To: <20160721140030.28383-6-marcandre.lureau@redhat.com> (marcandre lureau's message of "Thu, 21 Jul 2016 18:00:23 +0400") Message-ID: <87invfv13s.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 05/12] monitor: register the qapi generated commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcandre.lureau@redhat.com Cc: qemu-devel@nongnu.org, Paolo Bonzini marcandre.lureau@redhat.com writes: > From: Marc-Andr=C3=A9 Lureau > > 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=C3=A9 Lureau > --- > 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 *p= arser, GQueue *tokens) > QObject *obj, *data; > QDict *input, *args; > const mon_cmd_t *cmd; > + QmpCommand *qcmd; > const char *cmd_name; > Monitor *mon =3D cur_mon; >=20=20 > @@ -3959,7 +3960,8 @@ static void handle_qmp_command(JSONMessageParser *p= arser, GQueue *tokens) > cmd_name =3D qdict_get_str(input, "execute"); > trace_handle_qmp_command(mon, cmd_name); > cmd =3D qmp_find_cmd(cmd_name); > - if (!cmd) { > + qcmd =3D 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 *p= arser, GQueue *tokens) > goto err_out; > } >=20=20 > - cmd->mhandler.cmd_new(args, &data, &local_err); > + qcmd->fn(args, &data, &local_err); >=20=20 > err_out: > monitor_protocol_emitter(mon, data, local_err); > @@ -4050,10 +4052,15 @@ void monitor_resume(Monitor *mon) >=20=20 > static QObject *get_qmp_greeting(void) > { > + QmpCommand *cmd; > QObject *ver =3D NULL; >=20=20 > - qmp_marshal_query_version(NULL, &ver, NULL); > - return qobject_from_jsonf("{'QMP':{'version': %p,'capabilities': []}= }",ver); > + cmd =3D 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. > } >=20=20 > 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 $(q= api-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 =3D "quit", > .args_type =3D "", > - .mhandler.cmd_new =3D qmp_marshal_quit, > }, >=20=20 > 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]); >=20=20 > module_call_init(MODULE_INIT_QOM); > + module_call_init(MODULE_INIT_QAPI); >=20=20 > 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.