Re: [Qemu-devel] [PATCH v6 00/15] Add qapi-to-JSON visitor

[Marc-André Lureau]

Hi

On Mon, Oct 10, 2016 at 5:29 PM Eric Blake <address@hidden> 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 visitor'
> 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é Lureau <address@hidden>



>
>  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é Lureau