From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45103) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YkyIR-0003Ob-4Q for qemu-devel@nongnu.org; Wed, 22 Apr 2015 13:18:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YkyIN-0002cJ-QA for qemu-devel@nongnu.org; Wed, 22 Apr 2015 13:18:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34391) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YkyIN-0002c6-I5 for qemu-devel@nongnu.org; Wed, 22 Apr 2015 13:18:43 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3MHIhK2015708 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 22 Apr 2015 13:18:43 -0400 Message-ID: <5537D7F1.1090700@redhat.com> Date: Wed, 22 Apr 2015 11:18:41 -0600 From: Eric Blake MIME-Version: 1.0 References: <1429719709-880-1-git-send-email-jsnow@redhat.com> <1429719709-880-3-git-send-email-jsnow@redhat.com> In-Reply-To: <1429719709-880-3-git-send-email-jsnow@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="EMsN2J8q0h38vSFgaAqoHHDuAVO5RPjID" Subject: Re: [Qemu-devel] [PATCH v2 2/4] scripts: qmp-shell: Expand support for QMP expressions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org Cc: lcapitulino@redhat.com, kchamart@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --EMsN2J8q0h38vSFgaAqoHHDuAVO5RPjID Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 04/22/2015 10:21 AM, John Snow wrote: > This includes support for [] expressions, single-quotes in > QMP expressions (which is not strictly a part of JSON), and > the ability to use "True" or "False" literals instead of > JSON's lowercased 'true' and 'false' literals. >=20 > qmp-shell currently allows you to describe values as > JSON expressions: > key=3D{"key":{"key2":"val"}} >=20 > But it does not currently support arrays, which are needed > for serializing and deserializing transactions: > key=3D[{"type":"drive-backup","data":{...}}] >=20 > qmp-shell also only currently accepts doubly quoted strings > as-per JSON spec, but QMP allows single quotes. >=20 > Lastly, python allows you to utilize "True" or "False" as > boolean literals, but JSON expects "true" or "false". Expand > qmp-shell to allow the user to type either, converting to the > correct type. Well, when easy to do. See below for more discussion... >=20 > As a consequence of the above, the key=3Dval parsing is also improved > to give better error messages if a key=3Dval token is not provided. >=20 > CAVEAT: The parser is still extremely rudimentary and does not > expect to find spaces in {} nor [] expressions. This patch does > not improve this functionality. If someone cares about this, they can fix it later. As I said in the v1 review, qmp-shell is mostly for debugging and testsuites, and we can live with the caveats for now. >=20 > Signed-off-by: John Snow > --- > scripts/qmp/qmp-shell | 44 ++++++++++++++++++++++++++++---------------= - > 1 file changed, 28 insertions(+), 16 deletions(-) As far as I can tell, this makes qmp-shell strictly more useful, so I have no qualms giving: Reviewed-by: Eric Blake >=20 > diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell > index a9632ec..4cdcb6c 100755 > --- a/scripts/qmp/qmp-shell > +++ b/scripts/qmp/qmp-shell > @@ -88,23 +88,35 @@ class QMPShell(qmp.QEMUMonitorProtocol): > # clearing everything as it doesn't seem to matter > readline.set_completer_delims('') > =20 > + def __parse_value(self, val): > + try: > + return int(val) > + except ValueError: > + pass > + > + if val.lower() =3D=3D 'true': > + return True > + if val.lower() =3D=3D 'false': > + return False =2E..as promised earlier, This allows param=3DtRuE (neither JSON nor Python), but that's a minor price to pay to get both param=3Dtrue (param=3DJSON style) and param=3DTr= ue (param=3DPython style) to work. Meanwhile... > + if val.startswith(('{', '[')): > + try: > + return json.loads(val) This statement accepts param=3DJSON style, as in: param=3D{ "foo":true } but it rejects (as invalid JSON): param=3D{ 'foo':true } param=3D{ "foo":True } param=3D{ 'foo':True } > + except ValueError: > + pass > + try: > + return ast.literal_eval(val) And this statement accepts param=3DPython style, as in: param=3D{ "foo":True } param=3D{ 'foo':True } but it rejects (as invalid Python): param=3D{ "foo":true } param=3D{ 'foo':true } Of those four combinations, QMP accepts: param=3D{ "foo":true } param=3D{ 'foo':true } while rejecting: param=3D{ "foo":True } param=3D{ 'foo':True } So among the four combinations of single/double quoting vs upper/lower casing of boolean literals, qmp-shell now accepts three variants, but we have a case that QMP accepts but qmp-shell rejects (single quotes mixed with lower-case true). Not the end of the world (again, if someone wants all four cases to work in qmp-shell and/or QMP proper, they can provide a patch later to further enhance things). But maybe worth expanding that CAVEAT in the commit message to acknowledge the issues. At any rate, doesn't affect my R-b given above. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --EMsN2J8q0h38vSFgaAqoHHDuAVO5RPjID 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/ iQEcBAEBCAAGBQJVN9fxAAoJEKeha0olJ0NqrhEH+QGU42P7ssRYYbE2dzHm7qrk yUW96Via+g4lJXD3Vyb0CtxtU6GfxCDaQmVzWi578Qjbv6kkyPDC4njbAGtSDk1V cJtVjMovHOWUjvYcflthmI/xn8bUzdT23yNNHbPNW94obojNr1DJ7ZjucqUp7j3h YG6dawxgSqyTkMWuE+kbqOd3ynrglxSrEs8iLFAQJSB90U44NaujFJHOmJU3XtSd v/vPYGYjeYredvQuFxwjyxA2xF9PN8TB5kNhL9wtmVDB59FlvpQS+rJMJDUnwchW SYjKrIaXyJNcV5ctFt6EbgGWXIMrOUWq0rZHE850H3WbjKvj7tJUegxObNyXY6Q= =JQHH -----END PGP SIGNATURE----- --EMsN2J8q0h38vSFgaAqoHHDuAVO5RPjID--