From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:57490)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1a1C8L-0004A2-1d
	for qemu-devel@nongnu.org; Tue, 24 Nov 2015 06:51:41 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1a1C8K-0003LR-5r
	for qemu-devel@nongnu.org; Tue, 24 Nov 2015 06:51:40 -0500
References: <1448342531-16407-1-git-send-email-famz@redhat.com>
	<1448342531-16407-13-git-send-email-famz@redhat.com>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <56544F43.3090603@redhat.com>
Date: Tue, 24 Nov 2015 12:51:31 +0100
MIME-Version: 1.0
In-Reply-To: <1448342531-16407-13-git-send-email-famz@redhat.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH for-2.6 12/14] qemu-img: In 'map',
 use QDict to generate JSON output
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Fam Zheng <famz@redhat.com>, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Jeff Cody <jcody@redhat.com>, Peter Lieven <pl@kamp.de>, qemu-block@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>



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.

Paolo