From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57549) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1Dfz-0004mb-O2 for qemu-devel@nongnu.org; Tue, 24 Nov 2015 08:30:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1Dfy-0005Pa-JW for qemu-devel@nongnu.org; Tue, 24 Nov 2015 08:30:31 -0500 Date: Tue, 24 Nov 2015 21:30:22 +0800 From: Fam Zheng Message-ID: <20151124133022.GA8887@ad.usersys.redhat.com> References: <1448342531-16407-1-git-send-email-famz@redhat.com> <1448342531-16407-13-git-send-email-famz@redhat.com> <56544F43.3090603@redhat.com> <87y4dn36rm.fsf@blackfin.pond.sub.org> <20151124132541.GA16060@ad.usersys.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151124132541.GA16060@ad.usersys.redhat.com> Subject: Re: [Qemu-devel] [PATCH for-2.6 12/14] qemu-img: In 'map', use QDict to generate JSON output List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Kevin Wolf , qemu-block@nongnu.org, Jeff Cody , Peter Lieven , qemu-devel@nongnu.org, Stefan Hajnoczi , Paolo Bonzini On Tue, 11/24 21:25, Fam Zheng wrote: > On Tue, 11/24 13:45, Markus Armbruster wrote: > > Paolo Bonzini writes: > > > > > On 24/11/2015 06:22, Fam Zheng wrote: > > >> case OFORMAT_JSON: > > >> - printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64", \"depth\": %d," > > >> - " \"zero\": %s, \"data\": %s", > > >> - (e->start == 0 ? "[" : ",\n"), > > >> - e->start, e->length, e->depth, > > >> - (e->flags & BDRV_BLOCK_ZERO) ? "true" : "false", > > >> - (e->flags & BDRV_BLOCK_DATA) ? "true" : "false"); > > >> + if (e->start == 0) { > > >> + printf("["); > > >> + } else { > > >> + printf(","); > > >> + } > > >> + > > >> + dict = qdict_new(); > > >> + qdict_put(dict, "start", qint_from_int(e->start)); > > >> + qdict_put(dict, "length", qint_from_int(e->length)); > > >> + qdict_put(dict, "depth", qint_from_int(e->depth)); > > >> + qdict_put(dict, "zero", qbool_from_bool(e->flags & BDRV_BLOCK_ZERO)); > > >> + qdict_put(dict, "data", qbool_from_bool(e->flags & BDRV_BLOCK_DATA)); > > >> if (e->flags & BDRV_BLOCK_OFFSET_VALID) { > > >> - printf(", \"offset\": %"PRId64"", e->offset); > > >> + qdict_put(dict, "offset", qint_from_int(e->offset)); > > >> } > > >> - putchar('}'); > > >> + str = qobject_to_json(QOBJECT(dict)); > > >> + printf("%s\n", qstring_get_str(str)); > > > > > > I think it's better if you use QAPI for this. You can make MapEntry a > > > QAPI struct and generate the QObject through a QMP visitor. > > > > > > The reason is that we could add JSON visitors that let us parse or > > > produce JSON without going through the expensive QObject creation. Even > > > though that is far away, the least explicit QObject manipulation we > > > have, the better. > > > > I concur. > > > > Manual messing with QDict is of course fine when a higher-level > > interface doesn't fit. But here, we serialize a struct to JSON, and > > that's something QAPI is meant to do. Having to define a QAPI type may > > be a bit of a bother, but once that's done, the "serialize this struct > > to JSON" code becomes less tedious. qemu-img.c already has a few > > examples; search for qmp_output_visitor_new(). > > Can we do streaming with QAPI? The size of "MapEntry array" can be huge. Or you mean we don't have to serialize the list in one go, instead we can serialize one MapEntry a time with QAPI like what we do now? That sounds doable. Fam