Re: [Qemu-devel] [PATCH v4 1/5] qapi: introduce DataObject to describe dynamic structs

[Eric Blake <eblake@redhat.com>]

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

On 01/23/2014 07:46 AM, Amos Kong wrote:
> This patch introduced a DataObject union in qapi-schema.json,
> we use it to describe dynamic data structs.
> 
> We will use it in following patches to support to QMP full
> introspection. We have many kinds of schema in json file,
> they all can be described by DataObject.
> 
> This patch also added a doc: qmp-full-introspection.txt,
> QMP introspection releated document will be added into it.

s/releated/related/

> It helps to use the new query command and understand the
> abstract method in describing the dynamic struct.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  docs/qmp-full-introspection.txt |  44 +++++++++++++
>  qapi-schema.json                | 141 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 185 insertions(+)
>  create mode 100644 docs/qmp-full-introspection.txt
> 
> --- /dev/null
> +++ b/docs/qmp-full-introspection.txt
> @@ -0,0 +1,44 @@
> += Full introspection support for QMP =

Existing practice on doc files is lousy, but it might be nice to make a
copyright/license statement explicit.

> +
> +
> +== Purpose ==
> +
> +Add a new monitor command for management to  query QMP schema

accidental two spaces

> +information, it returns a range of schema structs, which contain the

s/information, it/information. It/

> +useful metadata to help management to check supported features, QMP
> +commands detail, etc.
> +
> +== 'DataObject' union ==
> +
> +{ 'union': 'DataObject',
> +  'base': 'DataObjectBase',
> +  'discriminator': 'type',
> +  'data': {
> +    'anonymous-struct': 'DataObjectAnonymousStruct',
> +    'command': 'DataObjectCommand',
> +    'enumeration': 'DataObjectEnumeration',
> +    'reference-type': 'String',
> +    'type': 'DataObjectType',
> +    'unionobj': 'DataObjectUnion' } }
> +
> +Currently we have schema difinitions for type, command, enumeration,

s/difinitions/definitions/

> +union. Some arbitrary structs (dictionary, list or string) and native
> +types are also used in the body of definitions.
> +
> +Here we use "DataObject" union to abstract all above schema. We want
> +to provide more useful metadata, and used some enum/unions to indicate
> +the dynamic type. In the output, some simple data is processed too
> +unwieldy. In another side, some complex data is described clearly.
> +It's also caused by some limitation of QAPI infrastructure.
> +
> +So we define 'DataObject' to be an union, it always has an object name
> +except anonymous struct.
> +
> +'command', 'enumeration', 'type', 'unionobj' are common schema type,
> +'union' is a build-in type, so I used unionobj here.

s/build-in/built-in/

> +
> +'reference-type' will be used to describe native types and unextended
> +types.
> +
> +'anonymous-struct' will be used to describe arbitrary structs
> +(dictionary, list or string).

Giving some examples from the qapi-schema.json file, and how they are
represented in JSON, would be nice.

> diff --git a/qapi-schema.json b/qapi-schema.json
> index f27c48a..c63f0ca 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4270,3 +4270,144 @@
>  # Since: 1.7
>  ##
>  { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
> +
> +##
> +# @DataObjectBase
> +#
> +# Base attributes of @DataObject
> +#
> +# @name: #optional @DataObject name
> +# @type: @DataObject type
> +# @recursive: #optional key to indicate if it's extended

I'm not quite sure from the documentation file what an "extended" object is.

> +#
> +# Since: 1.8

2.0 (we aren't doing a 1.8 release)

> +##
> +{ 'type': 'DataObjectBase',
> +  'data': { '*name': 'str', 'type': 'str', '*recursive': 'bool' } }
> +
> +##
> +# @DataObjectMemberType
> +#
> +# Type of @DabaObjectMember

s/Daba/Data/

> +#
> +# @reference: reference string
> +# @anonymous: arbitrary struct
> +# @extend: the @DataObjectMember
> +#
> +# Since: 1.8

2.0

> +##
> +{ 'union': 'DataObjectMemberType',
> +  'discriminator': {},
> +  'data': { 'reference': 'str',
> +            'anonymous': 'DataObject',
> +            'extend': 'DataObject' } }

I'm not sure I follow the difference between anonymous and extend here.

> +
> +##
> +# @DataObjectMember
> +#
> +# General member of @DataObject
> +#
> +# @type: type of @DataObjectMember
> +# @name: #optional name
> +# @optional: #optional key to indicate if the @DataObjectMember is optional
> +# @recursive: #optional key to indicate if it's defined recursively
> +#
> +# Since: 1.8

2.0

> +##
> +{ 'type': 'DataObjectMember',
> +  'data': { 'type': 'DataObjectMemberType', '*name': 'str',
> +            '*optional': 'bool', '*recursive': 'bool' } }
> +
> +##
> +# @DataObjectAnonymousStruct
> +#
> +# Arbitrary struct, it can be dictionary, list or string
> +#
> +# @data: content of arbitrary struct
> +#
> +# Since: 1.8

2.0

> +##
> +{ 'type': 'DataObjectAnonymousStruct',
> +  'data': { 'data': [ 'DataObjectMember' ] } }
> +
> +##
> +# @DataObjectCommand
> +#
> +# QMP Command schema
> +#
> +# @data: QMP command content

#optional

> +# @returns: returns of executing command

@returns: #optional type of return value after executing command

> +# @gen: a key to suppress code generation

I'm not sure we want to expose @gen through QAPI.  It's better to omit
it for now, and add it later if we find that it matters, than it is to
expose it now, then wish we hadn't when trying to refactor internal code
generation later.  That is, knowing whether QAPI code resulted in
automatic-generated code instead of hand-written code in qemu won't
affect how libvirt is able to use the interface.

> +#
> +# Since: 1.8

2.0

> +##
> +{ 'type': 'DataObjectCommand',
> +  'data': { '*data': [ 'DataObjectMember' ],
> +            '*returns': 'DataObject',
> +            '*gen': 'bool' } }
> +
> +##
> +# @DataObjectEnumeration
> +#
> +# Enumeration schema
> +#
> +# @data: enumeration content, it's a string list

s/it's/as/

May be worth a comment that although this is shown as a list, there is
no intrinsic meaning to the relative order of names, and that we
intentionally allow future versions of qemu to reorder and/or inject
enum values in the middle rather than only appending at the end.

> +#
> +# Since: 1.8

2.0

> +##
> +{ 'type': 'DataObjectEnumeration',
> +  'data': { 'data': [ 'str' ] } }
> +
> +##
> +# @DataObjectType
> +#
> +# Type schema
> +#
> +# @data: defined content of type, it's a dictionary or list
> +#
> +# Since: 1.8

2.0

> +##
> +{ 'type': 'DataObjectType',
> +  'data': { 'data': [ 'DataObjectMember' ] } }
> +
> +##
> +# @DataObjectUnion
> +#
> +# Union schema
> +#
> +# @data: main content of union
> +# @base: base attributes of union

#optional

> +# @discriminator: union discriminator

#optional

> +#
> +# Since: 1.8

2.0

> +##
> +{ 'type': 'DataObjectUnion',
> +  'data': { 'data': [ 'DataObjectMember' ], '*base': 'DataObject',
> +            '*discriminator': 'DataObject' } }
> +
> +##
> +# @DataObject
> +#
> +# Dynamic data struct, it can be command, enumeration, type, union, arbitrary
> +# struct or native type.
> +#
> +# @anonymous-struct: arbitrary struct, it can be dictionary, list or string
> +# @command: QMP command schema
> +# @enumeration: enumeration schema
> +# @reference-type: native type or unextended type
> +# @type: type schema, it will be extended
> +# @unionobj: union schema, 'union' is a conflicted name, so we use
> +#            unionobj instead
> +#
> +# Since: 1.8

2.0

We also need to start thinking about events, since there is a pending
series adding direct QAPI support for events.

> +##
> +{ 'union': 'DataObject',
> +  'base': 'DataObjectBase',
> +  'discriminator': 'type',
> +  'data': {
> +    'anonymous-struct': 'DataObjectAnonymousStruct',
> +    'command': 'DataObjectCommand',
> +    'enumeration': 'DataObjectEnumeration',
> +    'reference-type': 'String',

Do you need the full-blown 'String' wrapper type, or will the built-in
'str' work here?

> +    'type': 'DataObjectType',

I'm wondering if this would be better named as 'struct', since it is the
counterpart to a union type.

> +    'unionobj': 'DataObjectUnion' } }

Overall, I think this is a fairly decent QAPI representation of what
forms QMP data elements; I'll have a better feel for it after I review
your generated output, but looks like we are on the right track for a
self-describing grammar.

-- 
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: 604 bytes --]