Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6 12/14] qemu-img: In 'map', use QDict to generate JSON output

[Fam Zheng]

On Tue, 11/24 21:25, Fam Zheng wrote:
> On Tue, 11/24 13:45, Markus Armbruster wrote:
> > Paolo Bonzini <address@hidden> 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