Re: [Qemu-devel] [RFC PATCH 05/11] qapi: Add visitor for implicit structs

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]

On 07/09/2013 03:53 AM, Kevin Wolf wrote:
> These can be used when an embedded struct is parsed and members not
> belonging to the struct may be present in the input (parsing flat
> namespect QMP union with discriminator)

namespect? Not sure if you meant 'namespaced'?

Again, a comment demonstrating in sample QMP on what this is attempting
would be helpful in the commit message.  Without looking at later
patches yet, I'm guessing from your description that your intent is to
reach a point where:

{ 'type': 'Extra1',
  'data': { 'foo': 'str' } }
{ 'type': 'Extra2',
  'data': { 'bar': 'str' } }
{ 'union': 'Type',
  'data': { 'variant1': 'Extra1',
            'variant2': 'Extra2' } }
{ 'command': 'test',
  'data': { 'arg': 'Type' } }

which currently requires this over the wire:

{ "execute": "test",
  "arguments": { "arg": { "type": "variant1",
       "data": { "foo": "hello" } } } }

can be shortened to:

{ "execute": "test",
  "arguments": { "arg": { "type": "variant1", "foo": "hello" } } }

by allowing the struct passed as "arg" to inline the contents of the
union rather than require another nesting level through "data".  This
patch sets up the entry points so that a future patch can parse the
struct of arg as an implicit struct, and not choke over the key "foo",
instead of the current visitor of a union type that expects exactly the
two keys "type" and "data".

> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  include/qapi/visitor-impl.h |  4 ++++
>  include/qapi/visitor.h      |  3 +++
>  qapi/qapi-visit-core.c      | 16 ++++++++++++++++
>  qapi/qmp-input-visitor.c    | 14 ++++++++++++++
>  4 files changed, 37 insertions(+)

Although the commit message left me floundering, the code itself seems
reasonable - you are just adding two new entry points to allow a new
type of parse.  It remains to later patches to see how the new parsers
are useful, but that doesn't stop me from giving:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]