From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34753) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a79PA-0007e5-7H for qemu-devel@nongnu.org; Thu, 10 Dec 2015 17:09:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a79P5-00087W-7l for qemu-devel@nongnu.org; Thu, 10 Dec 2015 17:09:40 -0500 Received: from resqmta-po-08v.sys.comcast.net ([96.114.154.167]:35506) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a79P4-00087O-VA for qemu-devel@nongnu.org; Thu, 10 Dec 2015 17:09:35 -0500 References: <1448514335-15988-1-git-send-email-famz@redhat.com> <1448514335-15988-15-git-send-email-famz@redhat.com> From: Eric Blake Message-ID: <5669F813.2080105@redhat.com> Date: Thu, 10 Dec 2015 15:09:23 -0700 MIME-Version: 1.0 In-Reply-To: <1448514335-15988-15-git-send-email-famz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="lo4hpPpSHgfp5qAtjvP3Fp1N5EirWsC7S" Subject: Re: [Qemu-devel] [PATCH v3 14/15] qemu-img: Use QAPI visitor to generate JSON List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , qemu-block@nongnu.org, Jeff Cody , Peter Lieven , Markus Armbruster , Stefan Hajnoczi , Paolo Bonzini This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --lo4hpPpSHgfp5qAtjvP3Fp1N5EirWsC7S Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/25/2015 10:05 PM, Fam Zheng wrote: > A visible improvement is that "filename" is now included in the output > if it's valid. >=20 > Reviewed-by: Eric Blake > Signed-off-by: Fam Zheng > --- > qemu-img.c | 34 ++++++++++------ > tests/qemu-iotests/122.out | 96 ++++++++++++++++++++++++++------------= -------- > 2 files changed, 77 insertions(+), 53 deletions(-) >=20 > @@ -2157,20 +2163,26 @@ static void dump_map_entry(OutputFormat output_= format, MapEntry *e, > } > break; > case OFORMAT_JSON: > - printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64"," The space after '{' here is interesting below [1] > - " \"depth\": %"PRId64", \"zero\": %s, \"data\": %s", > - (e->start =3D=3D 0 ? "[" : ",\n"), > - e->start, e->length, e->depth, > - e->zero ? "true" : "false", > - e->data ? "true" : "false"); > - if (e->has_offset) { > - printf(", \"offset\": %"PRId64"", e->offset); > - } > - putchar('}'); > + ov =3D qmp_output_visitor_new(); > + visit_type_MapEntry(qmp_output_get_visitor(ov), > + &e, NULL, &error_abort); > + obj =3D qmp_output_get_qobject(ov); > + str =3D qobject_to_json(obj); > + assert(str !=3D NULL); For what it's worth, I have a patch series on my local tree that adds json_output_visitor_new(), and which would not only bypass the wasted QObject by doing a multi-step qapi=3D>QObject=3D>JSON, but it also has th= e benefit of outputting JSON that is in qapi order (under your control) rather than QDict hash order (deterministic and still valid JSON, but hard to predict and not always the nicest results for human readers). > +++ b/tests/qemu-iotests/122.out > @@ -49,12 +49,13 @@ read 65536/65536 bytes at offset 4194304 > 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > read 65536/65536 bytes at offset 8388608 > 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > -[{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": tru= e}, > -{ "start": 65536, "length": 4128768, "depth": 0, "zero": true, "data":= false}, > -{ "start": 4194304, "length": 65536, "depth": 0, "zero": false, "data"= : true}, > -{ "start": 4259840, "length": 4128768, "depth": 0, "zero": true, "data= ": false}, > -{ "start": 8388608, "length": 65536, "depth": 0, "zero": false, "data"= : true}, > -{ "start": 8454144, "length": 4128768, "depth": 0, "zero": true, "data= ": false}] > +[{"length": 65536, "start": 0, "zero": false, "depth": 0, "data": true= } Thus, my series conflicts with your patch, and one of us will have to rebase on the other. But with my series, we'd have yet more churn here, looking like: > {"start": 65536, "length": 4128768, "data": true, "zero": true, "depth"= : 0}, unless we change patch 13/15 to lay out the qapi struct in the same order as the existing output. But some churn is inevitable; both the QObject formatter and my pending patches output a leading "{KEY", rather than "{ KEY" (back to point [1] above), regardless of which key gets displayed first. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --lo4hpPpSHgfp5qAtjvP3Fp1N5EirWsC7S 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/ iQEcBAEBCAAGBQJWafgUAAoJEKeha0olJ0NqxO8H/25SavG4Z9/xr++H3hHkY+WL Ke8T5VnazZD5VFcCFSwF1+rpCh+J3YMTa/sBV1buwFPoVPYwDXyTAISlg+cYgywt quvxL22GDQQ6Mad20pP+6ymnSpIcJYl7vLDzCP6myGigB/QmlJbc6TBqiHp7l4Sa sU9sanlR7lcAw1kQXn8SchXrI2p1WTqEbT3LgBWafwIVi1pW0Z96gKI2X83+fLd7 jQrzK5uygQ/YMvVE1z+7iOJOYULIbdv5gzyU+TKDlFr3SYWzRWBkHYc6vlsw0Ec2 E629xwFqY/3bt55KwyY3+/WPBecltTsoJ74UKLqo235zPziUYLa9Ya+lZ6jUr6g= =jlgb -----END PGP SIGNATURE----- --lo4hpPpSHgfp5qAtjvP3Fp1N5EirWsC7S--