From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39265) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bGImQ-0005YU-Ra for qemu-devel@nongnu.org; Fri, 24 Jun 2016 00:31:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bGImM-0003Kh-Jj for qemu-devel@nongnu.org; Fri, 24 Jun 2016 00:31:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47827) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bGImM-0003Kc-BQ for qemu-devel@nongnu.org; Fri, 24 Jun 2016 00:31:42 -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 A7108C0828C6 for ; Fri, 24 Jun 2016 04:31:41 +0000 (UTC) References: <20160623000809.4522-1-marcandre.lureau@redhat.com> <20160623000809.4522-6-marcandre.lureau@redhat.com> From: Eric Blake Message-ID: <576CB7AC.90602@redhat.com> Date: Thu, 23 Jun 2016 22:31:40 -0600 MIME-Version: 1.0 In-Reply-To: <20160623000809.4522-6-marcandre.lureau@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="DWGnkPM9Fu27bNDhVSmelIbHdV2DWJH0t" Subject: Re: [Qemu-devel] [PATCH 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, qemu-devel@nongnu.org Cc: armbru@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --DWGnkPM9Fu27bNDhVSmelIbHdV2DWJH0t From: Eric Blake To: marcandre.lureau@redhat.com, qemu-devel@nongnu.org Cc: armbru@redhat.com Message-ID: <576CB7AC.90602@redhat.com> Subject: Re: [PATCH 05/12] monitor: register the qapi generated commands References: <20160623000809.4522-1-marcandre.lureau@redhat.com> <20160623000809.4522-6-marcandre.lureau@redhat.com> In-Reply-To: <20160623000809.4522-6-marcandre.lureau@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 06/22/2016 06:08 PM, marcandre.lureau@redhat.com wrote: > From: Marc-Andr=C3=A9 Lureau >=20 > Stop using the so-called 'middle' mode. Instead, use qmp_find_command()= > from generated qapi commands registry. >=20 > 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). I'm not as well-versed as Paolo on Makefile issues, so I won't comment on that part of the patch. >=20 > Signed-off-by: Marc-Andr=C3=A9 Lureau > --- > Makefile | 2 +- > monitor.c | 12 +++-- > qmp-commands.hx | 143 ------------------------------------------------= -------- > vl.c | 1 + > 4 files changed, 11 insertions(+), 147 deletions(-) >=20 > +++ b/monitor.c > @@ -3884,6 +3884,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 =3D cur_mon; > =20 > @@ -3909,7 +3910,8 @@ static void handle_qmp_command(JSONMessageParser = *parser, 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); Is it worth creating a single type that contains both mon_cmd_t and QmpCommand information, so that we don't need two similarly-named functions to look up two related pieces of information? Not necessarily in this patch. > + if (!qcmd || !cmd) { > error_set(&local_err, ERROR_CLASS_COMMAND_NOT_FOUND, > "The command %s has not been found", cmd_name); > goto err_out; > @@ -3931,7 +3933,7 @@ static void handle_qmp_command(JSONMessageParser = *parser, GQueue *tokens) > goto err_out; > } > =20 > - cmd->mhandler.cmd_new(args, &data, &local_err); > + qcmd->fn(args, &data, &local_err); > =20 > err_out: > monitor_protocol_emitter(mon, data, local_err); > @@ -4000,9 +4002,13 @@ void monitor_resume(Monitor *mon) > =20 > static QObject *get_qmp_greeting(void) > { > + QmpCommand *cmd; > QObject *ver =3D NULL; > =20 > - qmp_marshal_query_version(NULL, &ver, NULL); > + 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); Worth fixing the missing space after ',' while touching near here? > } > =20 > diff --git a/qmp-commands.hx b/qmp-commands.hx > index ee88e48..95c1e7d 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, At one point, I posted an RFC patch for removing .args_type on most QMP command listings in this file. Maybe you still do that later in your series, but as my work definitely conflicts with yours, and your series is older, I don't mind getting through your series first. Overall, I like the general idea of automating things rather than having to duplicate information in qmp-commands.hx. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --DWGnkPM9Fu27bNDhVSmelIbHdV2DWJH0t Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXbLesAAoJEKeha0olJ0NqXTUH/1fOxrbdBMWnHUVdCo2gV1Vm NE57vAu0OS2HA/CxZJOEF0G+dmDDPEF0EctMDS8J/BPCQl9cN5AsqUPL/yqKgH3Y Qlv9iYjbqzpVIhC+/Q2IXYd3YoVtMI1/KM2QTWXKepMJruq61IjICBmERz5RJFMv FVXd6n8ERSbSPTlkvkzzeszSQxqWa9/E3JVhs4GOZBHgupw7xLDA5j5t7DByWrqg 3dxe19kYBJtb0oug7U8HsbKJSMoPUjI8dBUtVeuyOJC55T66GX6/1Pbj2pVP1rQH e4kkEym8cb6JfP8YFKg1mTKfDgKBUOJWQ5e4VfH21IfxHxhzUCsLpz6auS0zNK4= =35YW -----END PGP SIGNATURE----- --DWGnkPM9Fu27bNDhVSmelIbHdV2DWJH0t--