From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55385) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bO5xQ-0005jY-MM for qemu-devel@nongnu.org; Fri, 15 Jul 2016 12:27:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bO5xL-0003mi-Gu for qemu-devel@nongnu.org; Fri, 15 Jul 2016 12:27:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50771) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bO5xL-0003mQ-8B for qemu-devel@nongnu.org; Fri, 15 Jul 2016 12:27:15 -0400 References: <1468505019-18154-1-git-send-email-berrange@redhat.com> <1468505019-18154-4-git-send-email-berrange@redhat.com> <87eg6udibp.fsf@dusky.pond.sub.org> From: Eric Blake Message-ID: <57890EE1.4060901@redhat.com> Date: Fri, 15 Jul 2016 10:27:13 -0600 MIME-Version: 1.0 In-Reply-To: <87eg6udibp.fsf@dusky.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="2kRjoKDsXErkgVRolbnVra31hpljAUO9R" Subject: Re: [Qemu-devel] [PATCH v8 3/7] qapi: add a QmpInputVisitor that does string conversion List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , "Daniel P. Berrange" Cc: Paolo Bonzini , =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , qemu-devel@nongnu.org, =?UTF-8?Q?Andreas_F=c3=a4rber?= , Max Reitz This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --2kRjoKDsXErkgVRolbnVra31hpljAUO9R From: Eric Blake To: Markus Armbruster , "Daniel P. Berrange" Cc: Paolo Bonzini , =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , qemu-devel@nongnu.org, =?UTF-8?Q?Andreas_F=c3=a4rber?= , Max Reitz Message-ID: <57890EE1.4060901@redhat.com> Subject: Re: [Qemu-devel] [PATCH v8 3/7] qapi: add a QmpInputVisitor that does string conversion References: <1468505019-18154-1-git-send-email-berrange@redhat.com> <1468505019-18154-4-git-send-email-berrange@redhat.com> <87eg6udibp.fsf@dusky.pond.sub.org> In-Reply-To: <87eg6udibp.fsf@dusky.pond.sub.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/15/2016 09:36 AM, Markus Armbruster wrote: > "Daniel P. Berrange" writes: >=20 >> Currently the QmpInputVisitor assumes that all scalar >> values are directly represented as their final types. >> ie it assumes an 'int' is using QInt, and a 'bool' is >> using QBool. >> >> This adds an alternative constructor for QmpInputVisitor >> that will set it up such that it expects a QString for >> all scalar types instead. >> >> This makes it possible to use QmpInputVisitor with a >> QDict produced from QemuOpts, where everything is in >> string format. >> >> Reviewed-by: Marc-Andr=C3=A9 Lureau >> Signed-off-by: Daniel P. Berrange >> --- >> +Visitor *qmp_string_input_visitor_new(QObject *obj, bool strict); >=20 > We have a QMP input visitor that isn't really about QMP, and a string > visitor. You're adding a QMP string input visitor that's even less > about QMP (using it with QMP would be wrong, in fact), and that is > related to the string visitor only insofar as both parse strings. > Differently, of course; this is QEMU. >=20 > Can't think of a better name, and I'm running out of Friday. >=20 Sounds like we might want a prereq patch that just does a global rename of s/qmp-\(input-visitor\|output-visitor\)/qobj-\1/ through all affected files. Since it really is a QObject visitor, the rename will make it easier to give the new visitor a nicer name. > Should we specify how strings are parsed? >=20 > Do you actually need both strict =3D true and strict =3D false here? I'd be fine if we just enforced that the new QObject-string parser is always strict. Non-strict parsers should only be used where we are worried about back-compat for hand-written code that knows how to deal with unparsed keys (such as device_add, where we MUST accept arguments not part of the QAPI schema, so long as we don't have a schema that fully describes it). >> static void qmp_input_optional(Visitor *v, const char *name, bool *pr= esent) >> { >> QmpInputVisitor *qiv =3D to_qiv(v); >=20 > Separate callbacks result in cleaner code than what I reviewed last. > Cleaner semantics, too: either all the scalars have sane types, or all > are strings. In other words, it forces us to strongly be tied to the input source, where command line is strongly-typed to strings, and QMP is strongly-typed to native types. It doesn't solve the problem of netdev_add previously being loose and taking both types, but as we've mentioned in other threads, we may not care (it may be sufficient to state that the undocumented looseness of netdev_add is not worth fixing); and Dan did have the nice followup proposal of adding yet another callback with peek semantics that can then delegate to the correct strongly-typed callback, rather than my approach that munged it all into a single callback. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --2kRjoKDsXErkgVRolbnVra31hpljAUO9R 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/ iQEcBAEBCAAGBQJXiQ7hAAoJEKeha0olJ0NqfJgH/RTJuWymQRgexFPZ1ZoNqqG5 P0cqIPQyWrRrBiJyO7qQtpYFqe4fUHY3b1vfyxDLXO+OWuXFx/lOXi+c7ExlkBWv aVDx4RYZM6qvWLV5fBWRK0VajDmZoA1BEVMwnfhcd8B3pfRCnTvCyZwCz3o0bcea 4k5HiOJH6JVEcEsw7PATUDhdsPzpce0ERDRdJ4OPRiMsmCPVFUCfG+mib9+T6xg7 ZqOmJZVhezeQV1UM/8tc0ZukuEEF8lIWIrxgNPe/spHb2ED7ZxB54iPAqEDE5BAl 8KxcL2VXBOMEO5aMrEzpfNwE7DhlGzQo09SbXz+gpAOXktSXRuShyT86p0cSpPo= =VvAj -----END PGP SIGNATURE----- --2kRjoKDsXErkgVRolbnVra31hpljAUO9R--