From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48940) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aYLqH-0007tt-Pd for qemu-devel@nongnu.org; Tue, 23 Feb 2016 17:54:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aYLqE-0001xj-F5 for qemu-devel@nongnu.org; Tue, 23 Feb 2016 17:54:05 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45000) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aYLqE-0001xf-71 for qemu-devel@nongnu.org; Tue, 23 Feb 2016 17:54:02 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id D5E0565400 for ; Tue, 23 Feb 2016 22:54:00 +0000 (UTC) References: <1456224706-1591-1-git-send-email-berrange@redhat.com> From: John Snow Message-ID: <56CCE307.5070206@redhat.com> Date: Tue, 23 Feb 2016 17:53:59 -0500 MIME-Version: 1.0 In-Reply-To: <1456224706-1591-1-git-send-email-berrange@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] qmp-shell: fix pretty printing of JSON responses List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Markus Armbruster , Luiz Capitulino On 02/23/2016 05:51 AM, 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 > 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. > Mea Culpa. > 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'}]} > > 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" > } > ] > } > That's good news as far as I am concerned. > Signed-off-by: Daniel P. Berrange > --- > scripts/qmp/qmp-shell | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell > index 7a402ed..0373b24 100755 > --- a/scripts/qmp/qmp-shell > +++ b/scripts/qmp/qmp-shell > @@ -70,7 +70,6 @@ import json > import ast > import readline > import sys > -import pprint > > class QMPCompleter(list): > def complete(self, text, state): > @@ -103,11 +102,11 @@ class FuzzyJSON(ast.NodeTransformer): > # TODO: QMPShell's interface is a bit ugly (eg. _fill_completion() and > # _execute_cmd()). Let's design a better one. ^ Heh ^ > class QMPShell(qmp.QEMUMonitorProtocol): > - def __init__(self, address, pp=None): > + def __init__(self, address, pretty=False): > qmp.QEMUMonitorProtocol.__init__(self, self.__get_address(address)) > self._greeting = None > self._completer = None > - self._pp = pp > + self._pretty = pretty > self._transmode = False > self._actions = list() > > @@ -231,11 +230,11 @@ class QMPShell(qmp.QEMUMonitorProtocol): > return qmpcmd > > def _print(self, qmp): > - jsobj = json.dumps(qmp) > - if self._pp is not None: > - self._pp.pprint(jsobj) > - else: > - print str(jsobj) > + indent = None > + if self._pretty: > + indent = 4 > + jsobj = json.dumps(qmp, indent=indent) > + print str(jsobj) > > def _execute_cmd(self, cmdline): > try: > @@ -377,7 +376,7 @@ def main(): > addr = '' > qemu = None > hmp = False > - pp = None > + pretty = False > verbose = False > > try: > @@ -387,9 +386,7 @@ def main(): > fail_cmdline(arg) > hmp = True > elif arg == "-p": > - if pp is not None: > - fail_cmdline(arg) > - pp = pprint.PrettyPrinter(indent=4) > + pretty = True We now accept any number of -p arguments. That's probably fine. > elif arg == "-v": > verbose = True > else: > @@ -398,7 +395,7 @@ def main(): > if hmp: > qemu = HMPShell(arg) > else: > - qemu = QMPShell(arg, pp) > + qemu = QMPShell(arg, pretty) > addr = arg > > if qemu is None: > Reviewed-by: John Snow