From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33607) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YBNxl-0005Bd-Ji for qemu-devel@nongnu.org; Wed, 14 Jan 2015 08:26:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YBNxi-0006Fp-QD for qemu-devel@nongnu.org; Wed, 14 Jan 2015 08:26:21 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46011) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YBNxi-0006Fe-IZ for qemu-devel@nongnu.org; Wed, 14 Jan 2015 08:26:18 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0EDQHMu023564 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 14 Jan 2015 08:26:18 -0500 Message-ID: <54B66E78.40103@redhat.com> Date: Wed, 14 Jan 2015 06:26:16 -0700 From: Eric Blake MIME-Version: 1.0 References: <1421171432-8733-1-git-send-email-armbru@redhat.com> <1421171432-8733-5-git-send-email-armbru@redhat.com> In-Reply-To: <1421171432-8733-5-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="57E7q1pmea16dq9iHxXJUmmA630MT0tcd" Subject: Re: [Qemu-devel] [PATCH 4/9] qmp: Clean up qmp_query_spice() #ifndef !CONFIG_SPICE dummy List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: kraxel@redhat.com, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --57E7q1pmea16dq9iHxXJUmmA630MT0tcd Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 01/13/2015 10:50 AM, Markus Armbruster wrote: > QMP command query-spice exists only #ifdef CONFIG_SPICE. Due to QAPI > limitations, we need a dummy function anyway, but it's unreachable. >=20 > Our current dummy function goes out of its way to produce the exact > same error as the QMP core does for unknown commands. Cute, but both > unclean and unnecessary. Replace by straight abort(). >=20 > Signed-off-by: Markus Armbruster > --- > + * qmp-commands.hx ensures that QMP command query-spice exists only > + * #ifdef CONFIG_SPICE. Necessary for an accurate query-commands > + * result. However, the QAPI schema is blissfully unaware of that, > + * and the QAPI code generator happily generates a dead > + * qmp_marshal_input_query_spice() that calls qmp_query_spice(). > + * Provide it one, or else linking fails. > + * FIXME Educate the QAPI schema on CONFIG_SPICE. There's probably several commands that are only conditionally compiled in qmp-commands.hx, but where the qapi has no way to express that they are conditional on how the binary was compiled. I'm not sure if it would help the user to document that a command listed in the .json might not actually exist, but if enhancing the qapi code generator to understand conditionals made for less maintenance, then that alone would be worth cleaning up this FIXME. In the meantime, your patch is just fine. Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --57E7q1pmea16dq9iHxXJUmmA630MT0tcd Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJUtm54AAoJEKeha0olJ0NqigQH/1aTgOhH7bZ1tSoetihsZRxp Tiz7BofmRUKjxfakYUClBwstZ5f5e279Xu8kJmUqnYytfEDJxkwcAk0n6+06gb83 7fnir1vNforSFlF+/0fPCPk9tkcZgwGzxwdET3ReEoKWNKb3Bf9TmQl5tXGkowPZ sdNRQpH4IEIbfeqwuwmUrhc81I6o5rNmw+kQqwtyIXySOinZoIW31hUMX51in+QP ajjwhZj1llGjX0uwDkoe/EjWk+OvrNuGHuDSVP0+uUgjgePMH02VVaApcyVAwIG6 4xuWJfcT8dCzCb28Q5u9dtugPXMuowXO4k65HZX8sOZU5g7xE0njSV0V2ZjLeh0= =o99G -----END PGP SIGNATURE----- --57E7q1pmea16dq9iHxXJUmmA630MT0tcd--