qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kashyap Chamarthy <kchamart@redhat.com>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>,
	qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2] qmp-shell: fix pretty printing of JSON responses
Date: Tue, 23 Feb 2016 12:39:39 +0100	[thread overview]
Message-ID: <20160223113939.GA5450@tesla.redhat.com> (raw)
In-Reply-To: <1456224706-1591-1-git-send-email-berrange@redhat.com>

On Tue, Feb 23, 2016 at 10:51:46AM +0000, Daniel P. Berrange wrote:
> Pretty printing of JSON responses is important to be able to understand
> large responses from query commands in particular. Unfortunately this
> was broken during the addition of the verbose flag in
> 
>   commit 1ceca07e48ead0dd2e41576c81d40e6a91cafefd
>   Author: John Snow <jsnow@redhat.com>
>   Date:   Wed Apr 29 15:14:04 2015 -0400
> 
>     scripts: qmp-shell: Add verbose flag
> 
> This is because that change turned the python data structure into a
> formatted JSON string before the pretty print was given it. So we're
> just pretty printing a string, which is a no-op.
> 
> The original pretty printer would output python objects.
> 
> (QEMU) query-chardev
> {   u'return': [   {   u'filename': u'vc',
>                        u'frontend-open': False,
>                        u'label': u'parallel0'},
>                    {   u'filename': u'vc',
>                        u'frontend-open': True,
>                        u'label': u'serial0'},
>                    {   u'filename': u'unix:/tmp/qemp,server',
>                        u'frontend-open': True,
>                        u'label': u'compat_monitor0'}]}


For me, without your patch (I was testing from 'v2.5.0-1307-g80b5d6b')
the output doesn't even look like the above -- when `qmp-shell -p
/path/to/socket` is invoked, and I type in any of the query commands,
what I see is long unterminated JSON output.

> This fixes the problem by switching to outputting pretty formatted JSON
> text instead. This has the added benefit that the pretty printed output
> is now valid JSON text. Due to the way the verbose flag was handled, the
> pretty printing now applies to the command sent, as well as its response:
> 
> (QEMU) query-chardev
> {
>     "execute": "query-chardev",
>     "arguments": {}
> }
> {
>     "return": [
>         {
>             "frontend-open": false,
>             "label": "parallel0",
>             "filename": "vc"
>         },
>         {
>             "frontend-open": true,
>             "label": "serial0",
>             "filename": "vc"
>         },
>         {
>             "frontend-open": true,
>             "label": "compat_monitor0",
>             "filename": "unix:/tmp/qmp,server"
>         }
>     ]
> }


Works nice here.  I ran for a bunch fo query-commands:

$ git describe
pull-io-next-2016-02-16-1-221-ga043f9c

[Fire up QEMU with '-qmp unix:./qmp-sock,server'

$ ./qmp-shell -p /var/tmp/qmp-sock
[...]
(QEMU) query-named-block-nodes
{
    "return": [
        {
            "bps_rd": 0, 
            "ro": false, 
            "backing_file_depth": 1, 
            "encrypted": false, 
            "image": {
                "cluster-size": 65536, 
                "backing-image": {
                    "cluster-size": 65536, 
                    "format": "qcow2", 
                    "filename": "/export/tests//cirros-0.3.3.img", 
                    "virtual-size": 41126400, 
                    "dirty-flag": false, 
                    "format-specific": {
                        "data": {
                            "compat": "0.10", 
                            "refcount-bits": 16
                        }, 
                        "type": "qcow2"
                    }, 
                    "actual-size": 14446592
                }, 
                "format": "qcow2", 
                "full-backing-filename": "/export/tests//cirros-0.3.3.img", 
                "backing-filename-format": "qcow2", 
                "filename": "./ext-snap1.qcow2", 
                "dirty-flag": false, 
                "virtual-size": 41126400, 
                "backing-filename": "/export/tests//cirros-0.3.3.img", 
                "format-specific": {
                    "data": {
                        "compat": "1.1", 
                        "refcount-bits": 16, 
                        "corrupt": false, 
                        "lazy-refcounts": false
                    }, 
                    "type": "qcow2"
                }, 
                "actual-size": 200704
            },
            [...]

> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  scripts/qmp/qmp-shell | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)

FWIW:

Tested-by: Kashyap Chamarthy <kchamart@redhat.com>

[...]

-- 
/kashyap

  reply	other threads:[~2016-02-23 11:39 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-23 10:51 [Qemu-devel] [PATCH v2] qmp-shell: fix pretty printing of JSON responses Daniel P. Berrange
2016-02-23 11:39 ` Kashyap Chamarthy [this message]
2016-02-23 22:53 ` John Snow
2016-02-24 10:28   ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160223113939.GA5450@tesla.redhat.com \
    --to=kchamart@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).