From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52954) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGUAM-0000Zm-9z for qemu-devel@nongnu.org; Tue, 05 Jan 2016 11:08:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aGUAH-0004JA-Ak for qemu-devel@nongnu.org; Tue, 05 Jan 2016 11:08:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36407) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGUAH-0004J5-3K for qemu-devel@nongnu.org; Tue, 05 Jan 2016 11:08:53 -0500 References: <1450717720-9627-1-git-send-email-eblake@redhat.com> <1450717720-9627-23-git-send-email-eblake@redhat.com> From: Eric Blake Message-ID: <568BEA8F.1050900@redhat.com> Date: Tue, 5 Jan 2016 09:08:47 -0700 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="4B0BIFWxdF5sURdKWCR3fGkmVteHRkDFi" Subject: Re: [Qemu-devel] [PATCH v8 22/35] qapi: Add visit_type_null() visitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= Cc: Michael Roth , QEMU , Markus Armbruster This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --4B0BIFWxdF5sURdKWCR3fGkmVteHRkDFi Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 01/05/2016 07:05 AM, Marc-Andr=C3=A9 Lureau wrote: > Hi >=20 > On Mon, Dec 21, 2015 at 6:08 PM, Eric Blake wrote: >> Right now, qmp-output-visitor happens to produce a QNull result >> if nothing is actually visited between the creation of the visitor >> and the request for the resulting QObject. A stronger protocol >> would require that a QMP output visit MUST visit something. But >> to still be able to produce a JSON 'null' output, we need a new >> visitor function that states our intentions. >> >> This patch introduces the new visit_type_null() interface, and >> a later patch will then wire it up into the qmp output visitor. >> >> Signed-off-by: Eric Blake >> >=20 > overall looks good to me: > Reviewed-by: Marc-Andr=C3=A9 Lureau >=20 > Just a small remark below, >=20 >> + /* Must be provided to visit explicit null values (right now, onl= y the >> + * dealloc visitor supports this). */ >> + void (*type_null)(Visitor *v, const char *name, Error **errp); >> >=20 > It's not clear to me what you mean by "Must be provided" if only one > visitor implements it. >=20 >> +void visit_type_null(Visitor *v, const char *name, Error **errp) >> +{ >> + v->type_null(v, name, errp); >> +} >=20 > Shouldn't it be optional then and a if (v->type_null) be added? No one else (yet) uses a visitor that explicitly visits JSON 'null'. If someone adds the use of such a visitor, this code will crash at the attempt to use the missing callback, which will tell the developer to add the callback. Adding a conditional would paper over the issue and make it harder to find. At any rate, it matches existing pattern of 'must be provided if you plan to visit X' vs. visitors that lack the callback - see visit_type_any= (). Maybe I should write a followup patch that implements it for qmp-input-visitor, as well as a testsuite that proves we can round-trip a null literal from JSON to QMP back to JSON. And when it comes to blockdev commands for reopening a device (such as changing its throttling options), we've toyed with the idea of being able to explicitly call out to leave an option unchanged (omit 'option' altogether) vs. reset the option to its default (doable with 'option':null, but requires that we allow parsing explicit null). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --4B0BIFWxdF5sURdKWCR3fGkmVteHRkDFi 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/ iQEcBAEBCAAGBQJWi+qPAAoJEKeha0olJ0NqcEQH+wamUyKdsm/hLypMwbNCl5Eg 2nkD09c3JqOrPSnpEW+Eciw6qd04YoiWEC3PgjTaORH0xCnJI4uraX5k9oYswfBt xWymGPp7bFQ/3Mijqh5MDW8D21nv57To/92UhaSr/ap+MWSWy1QmdkztnNPsUJaV ummkNWxWarHIB/GHc8L2kZznGgpdMf9pCVKuDC+CDbBbXRqwbyoeg9jaZwfCG0qD 085z8YqhKV4qwQu9SggOLtwEz4tTKx6GZ2y4EYYtjoTjBbgmMkCqK/z+tk7P8XTv eZi5HvZEFEtenblawM29sHfVTaVJ9zvG7+1YQisyhUnW1UwekArFqD+PglgHuT0= =dHMS -----END PGP SIGNATURE----- --4B0BIFWxdF5sURdKWCR3fGkmVteHRkDFi--