From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49656) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQOGy-0005E1-0I for qemu-devel@nongnu.org; Mon, 01 Feb 2016 18:52:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aQOGt-0004cu-WB for qemu-devel@nongnu.org; Mon, 01 Feb 2016 18:52:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40597) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQOGt-0004ch-Ns for qemu-devel@nongnu.org; Mon, 01 Feb 2016 18:52:39 -0500 References: <1453219845-30939-1-git-send-email-eblake@redhat.com> <1453219845-30939-24-git-send-email-eblake@redhat.com> <87a8nxilmt.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <56AFEFC4.5020202@redhat.com> Date: Mon, 1 Feb 2016 16:52:36 -0700 MIME-Version: 1.0 In-Reply-To: <87a8nxilmt.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="WR3CAxKqfl7nKpjMJRWNUccmHgXGNcWLq" Subject: Re: [Qemu-devel] [PATCH v9 23/37] qmp: Support explicit null during input visit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: marcandre.lureau@redhat.com, qemu-devel@nongnu.org, Michael Roth This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --WR3CAxKqfl7nKpjMJRWNUccmHgXGNcWLq Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 01/22/2016 10:12 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> Implement the new type_null() callback for the qmp input visitor. >> While we don't yet have a use for this in qapi (the generator >> will need some tweaks first), one usage is already envisioned: >> when changing blockdev parameters, it would be nice to have a >> difference between leaving a tuning parameter unchanged (omit >> that parameter from the struct) and to explicitly reset the >> parameter to its default without having to know what the default >> value is (specify the parameter with an explicit null value, >> which will require us to allow a qapi alternate that chooses >> between the normal value and an explicit null). >> >> At any rate, we can test this without the use of generated qapi >> by manually using visit_start_struct()/visit_end_struct(). >=20 > Well, we test by calling visit_type_null() manually. We choose to wrap= > it in a visit_start_struct() ... visit_end_struct() pair, but that's > detail. Actually, we do an unwrapped root visit first, and then a > struct-wrapped visit. >=20 > Suggest "by calling visit_type_null() manually." >=20 >> Signed-off-by: Eric Blake >> >> --- >> +static void qmp_input_type_null(Visitor *v, const char *name, Error *= *errp) >> +{ >> + QmpInputVisitor *qiv =3D to_qiv(v); >> + QObject *qobj =3D qmp_input_get_object(qiv, name, true); >> + >> + if (qobject_type(qobj) =3D=3D QTYPE_QNULL) { >> + return; >> + } >> + >> + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null= ", >> + "null"); >=20 > Recommend to put the error in the conditional: >=20 > if (qobject_type(qobj) !=3D QTYPE_QNULL) { > error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "nu= ll", > "null"); > } Sure, I can reflow the logic. >> + /* Check that qnull reference counting is sane: >> + * 1 for global use, 1 for our qnull() use, and 1 still owned by = 'v' >> + * until it is torn down */ >> + null =3D qnull(); >> + g_assert(null->refcnt =3D=3D 3); >> + visitor_input_teardown(data, NULL); >> + g_assert(null->refcnt =3D=3D 2); >> + qobject_decref(null); >=20 > For other kinds of QObject, we leave the testing of reference counting > to the check-qKIND.c, and don't bother with it when testing the > visitors. Any particular reason to do null differently? Well, 19/37 added reference counting checks to test-qmp-output-visitor.c, and we don't have a check-qnull.c test yet. That, and the thing being checked here is that the visitor doesn't over- or under-reference the static qnull object (just checking qnull() without a visitor doesn't tell you if the visitor has any reference counting bugs). But maybe it is indeed worth writing a check-qnull.c file that does this work. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --WR3CAxKqfl7nKpjMJRWNUccmHgXGNcWLq 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/ iQEcBAEBCAAGBQJWr+/EAAoJEKeha0olJ0NqxFEH/jZhC+3BfsW+HIUaEhuL2+z4 pcAeOZoY9r1guCQURhz4szFkDHDMe+aDlegNxLvio8pPy1kMQyKQh3D8KrQrQguX 4alL4XA69ddqQR/u7k6IMEfenGe2SzE+JNZiKP0/whfmjmQlf9jCrrOHLVgKoqF1 vNlXJcUW2N28/yN5QADTeya1zvPxD9KGZs9H0d3lvff1RBng8TjxKSmSbV5XwUjH OxmfnaPOONhFluFA/SO4h1qHv/DOQJB2oZRqfaw6gLyjre7Mnh7OBlp/0kCgr14O Hz4a9T4XDeYctbV9JKx/6TroOYWwRWJCVhXzhxe3ZGO+ZSCsDtrPqg/iu+GXODA= =fDSQ -----END PGP SIGNATURE----- --WR3CAxKqfl7nKpjMJRWNUccmHgXGNcWLq--