Re: [Qemu-devel] [PATCH v5 01/28] qapi: Document type-safety considerations

[Kevin Wolf]

Am 24.03.2015 um 21:03 hat Eric Blake geschrieben:
> Go into more details about the various types of valid expressions
> in a qapi schema, including tweaks to document fixes being done
> later in the current patch series.  Also fix some stale and missing
> documentation in the QMP specification.
> 
> Signed-off-by: Eric Blake <address@hidden>

>  === Union types ===
> 
> -Union types are used to let the user choose between several different data
> -types.  A union type is defined using a dictionary as explained in the
> -following paragraphs.
> +Usage: { 'union': 'str', 'data': 'dict' }
> +or:    { 'union': 'str', 'data': 'dict', 'base': 'complex-type-name',
> +         'discriminator': 'enum-member-of-base' }
> +or:    { 'union': 'str', 'data': 'dict', 'discriminator': {} }
> 
> +Union types are used to let the user choose between several different
> +data types.  There are three flavors: simple (no discriminator), flat
> +(a base type is mandatory, and discriminator is the name of an enum
> +field within that base type), and anonymous (discriminator is an
> +empty dictionary).  A union type is defined using a data dictionary as
> +explained in the following paragraphs.
> 
> -A simple union type defines a mapping from discriminator values to data types
> -like in this example:
> +A simple union type defines a mapping from automatic discriminator
> +values to data types like in this example:
> 
>   { 'type': 'FileOptions', 'data': { 'filename': 'str' } }
>   { 'type': 'Qcow2Options',
> @@ -132,10 +273,17 @@ specified data type corresponding to the discriminator 
> value:
>   { "type": "qcow2", "data" : { "backing-file": "/some/place/my-image",
>                                 "lazy-refcounts": true } }
> 
> +Additionally, an implicit C enum NameKind is created, corresponding to
> +the union Name, for accessing the various branches of the union.  No
> +branch of the union can be named 'max', as this would collide with the
> +implicit enum.
> 
> -A union definition can specify a complex type as its base. In this case, the
> -fields of the complex type are included as top-level fields of the union
> -dictionary in the QMP wire format. An example definition is:
> +
> +A flat union definition specifies a complex type as its base, and
> +avoids nesting on the wire.  In this case, the fields of the complex
> +type are included as top-level fields of the union dictionary in the
> +QMP wire format, and the 'discriminator' field must be the name of an
> +enum-typed member of the base type. An example definition is:

Adding full context:

     { 'type': 'BlockdevCommonOptions', 'data': { 'readonly': 'bool' } }
     { 'union': 'BlockdevOptions',
       'base': 'BlockdevCommonOptions',
       'data': { 'raw': 'RawOptions',
                 'qcow2': 'Qcow2Options' } }

    And it looks like this on the wire:

     { "type": "qcow2",
       "readonly": false,
       "data" : { "backing-file": "/some/place/my-image",
                  "lazy-refcounts": true } }

This is not a flat union for me, but really a simple one with an added
base.

With this example treated as a flat union, I'm afraid the explanation
for the real flat unions below becomes hard to understand, because you
don't mention any more that the defining property for that case is the
existence of a 'discriminator' key, which must refer to a key in the
base type.

Somehow it seems to be expected that the reader already knows this and
you only need to explain the details. I would appreciate if you could
reintroduce the information that the old text of this section had.

> -
> -Flat union types avoid the nesting on the wire. They are used whenever a
> -specific field of the base type is declared as the discriminator ('type' is
> -then no longer generated). The discriminator must be of enumeration type.
> -The above example can then be modified as follows:
> +Notice that in a flat union, a 'type' field is no longer generated,
> +and the keys of the 'data' dictionary must match the valid values for
> +the discriminator (although not necessarily in the same order). The
> +above example for simple unions can be modified to a flat union as
> +follows:
> 
>   { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
>   { 'type': 'BlockdevCommonOptions',

Everything else (that hasn't been mentioned in this thread yet) looks
good to me.

Kevin