From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48941) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btvxZ-0003Vn-SL for qemu-devel@nongnu.org; Tue, 11 Oct 2016 08:15:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1btvxX-0005Y0-9F for qemu-devel@nongnu.org; Tue, 11 Oct 2016 08:15:04 -0400 Received: from mail-lf0-x241.google.com ([2a00:1450:4010:c07::241]:36438) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btvxW-0005XV-Sn for qemu-devel@nongnu.org; Tue, 11 Oct 2016 08:15:03 -0400 Received: by mail-lf0-x241.google.com with SMTP id b75so3394515lfg.3 for ; Tue, 11 Oct 2016 05:15:02 -0700 (PDT) MIME-Version: 1.0 References: <1476105837-9861-1-git-send-email-eblake@redhat.com> In-Reply-To: <1476105837-9861-1-git-send-email-eblake@redhat.com> From: =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= Date: Tue, 11 Oct 2016 12:14:50 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v6 00/15] Add qapi-to-JSON visitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: armbru@redhat.com Hi On Mon, Oct 10, 2016 at 5:29 PM Eric Blake wrote: > [5 months later...] > Going from qapi to QObject to JSON is wasteful compared to going > straight from qapi to JSON. What's more, having QObject in the > middle means that dict keys are shuffled according to hash ordering, > while going straight from qapi means we match the .json QAPI file > declaration order. Factoring out JSON visits to a common central > file will make it easier to make any further tweaks, such as > patching floating point output to be unambiguous. > > Based on Markus' qapi-next branch, but should apply to current > qemu.git master. > > Diff from v4, which was at: > https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg03220.html > (v4 was both JSON and clone visitors; v5 split out > just the clone visitor, leaving just the JSON visitor and hence > calling this v6): > > - assert up front that we are no longer willing to receive or > pass non-finite numbers through QMP > - drop experiment related to string-izing nonfinite numbers > - rebase to recent changes > - address other minor review comments from Markus > > Available as a tag at: > git fetch git://repo.or.cz/qemu/ericb.git qapi-jsonv6 > > 001/15:[down] 'qapi: Visitor documentation tweak' > 002/15:[down] 'qapi: Assert finite use of 'number'' > 003/15:[0013] [FC] 'qapi: Factor out JSON string escaping' > 004/15:[0022] [FC] 'qapi: Factor out JSON number formatting' > 005/15:[----] [--] 'qapi: Add qstring_append_printf()' > 006/15:[----] [--] 'qapi: Use qstring_append_chr() where appropriate' > 007/15:[----] [--] 'qstring: Add qstring_consume_str()' > 008/15:[0022] [FC] 'qstring: Add qstring_wrap_str()' > 009/15:[----] [-C] 'qobject: Consolidate qobject_to_json() calls' > 010/15:[----] [--] 'tests: Test qobject_to_json() pretty formatting' > 011/15:[0013] [FC] 'qapi: Add JSON output visitor' > 012/15:[0012] [FC] 'qapi: Support pretty printing in JSON output visitor' > 013/15:[0041] [FC] 'qobject: Implement qobject_to_json() atop JSON visito= r' > 014/15:[----] [--] 'qapi: Add 'any' support to JSON output' > 015/15:[0002] [FC] 'qemu-img: Use new JSON output formatter' > > > Eric Blake (15): > qapi: Visitor documentation tweak > qapi: Assert finite use of 'number' > qapi: Factor out JSON string escaping > qapi: Factor out JSON number formatting > qapi: Add qstring_append_printf() > qapi: Use qstring_append_chr() where appropriate > qstring: Add qstring_consume_str() > qstring: Add qstring_wrap_str() > qobject: Consolidate qobject_to_json() calls > tests: Test qobject_to_json() pretty formatting > qapi: Add JSON output visitor > qapi: Support pretty printing in JSON output visitor > qobject: Implement qobject_to_json() atop JSON visitor > qapi: Add 'any' support to JSON output > qemu-img: Use new JSON output formatter > Except the minor comments I left in patch 8 and 12, the whole series is good to me: Reviewed-by: Marc-Andr=C3=A9 Lureau > > include/qapi/visitor.h | 33 +-- > include/qapi/json-output-visitor.h | 29 +++ > include/qapi/qmp/qjson.h | 16 +- > include/qapi/qmp/qstring.h | 9 +- > block.c | 5 +- > block/accounting.c | 6 +- > block/archipelago.c | 6 +- > blockdev.c | 3 +- > migration/migration.c | 4 + > migration/qjson.c | 12 +- > monitor.c | 10 +- > qapi/json-output-visitor.c | 211 ++++++++++++++++ > qemu-img.c | 46 ++-- > qga/main.c | 5 +- > qobject/json-parser.c | 25 +- > qobject/qjson.c | 301 ++++++++++------------- > qobject/qstring.c | 69 +++++- > qom/object.c | 3 +- > tests/check-qjson.c | 93 ++++++-- > tests/check-qstring.c | 46 +++- > tests/libqtest.c | 2 +- > tests/test-json-output-visitor.c | 477 > +++++++++++++++++++++++++++++++++++++ > tests/test-qmp-output-visitor.c | 5 + > tests/test-visitor-serialization.c | 2 +- > qapi/Makefile.objs | 2 +- > tests/.gitignore | 1 + > tests/Makefile.include | 5 +- > tests/qemu-iotests/043.out | 22 +- > 28 files changed, 1137 insertions(+), 311 deletions(-) > create mode 100644 include/qapi/json-output-visitor.h > create mode 100644 qapi/json-output-visitor.c > create mode 100644 tests/test-json-output-visitor.c > > -- > 2.7.4 > > > -- Marc-Andr=C3=A9 Lureau