Re: [Qemu-devel] [PATCH 10/18] qapi: Anonymous unions

[Eric Blake <eblake@redhat.com>]

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

On 07/23/2013 07:03 AM, Kevin Wolf wrote:
> The discriminator for anonymous unions is the data type. This allows to
> have a union type that allows both of these:
> 
>     { 'file': 'my_existing_block_device_id' }
>     { 'file': { 'filename': '/tmp/mydisk.qcow2', 'read-only': true } }
> 
> Unions like this are specified in the schema with an empty dict as
> discriminator. For this example you could take:
> 
>     { 'union': 'BlockRef',
>       'discriminator': {},
>       'data': { 'definition': 'BlockOptions'

Missing comma.

>                 'reference': 'str' } }
>     { 'type': 'ExampleObject',
>       'data: { 'file': 'BlockRef' } }
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---

> +++ b/docs/qapi-code-gen.txt
> @@ -125,6 +125,31 @@ Resulting in this JSON object:
>     "lazy-refcounts": true }
>  
>  
> +A special type of unions are anonymous unions. They don't form a dictionary in
> +the wire format but allow the direct use of different types in their place. As
> +they aren't structured, they don't have any explicit discriminator but use
> +the (QObject) data type of their value as an implicit discriminator. This means
> +that they are restricted to using only one discriminator value per QObject
> +type. For example, you cannot have two different complex types in an anonymous
> +union, or two different integer types.
> +
> +Anonymous unions are declared using an empty dictionary as their discriminator.
> +The disriminator values never appear on the wire, they are only used in the

s/disriminator/discriminator/

> +generated C code. Anonymous unions cannot have a base type.
> +
> + { 'union': 'BlockRef',
> +   'discriminator': {},
> +   'data': { 'definition': 'BlockdevOptions'

Missing comma.

> +++ b/qobject/qjson.c
> @@ -260,6 +260,8 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
>          /* XXX: should QError be emitted? */
>      case QTYPE_NONE:
>          break;
> +    case QTYPE_MAX:
> +        abort();

There's another recent patch proposing the use of g_assert_not_reached():

https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg04560.html

but I don't know if it is worth switching abort() over to this function
(and even if it was, there's enough other use of abort in the qapi code
that it would be better to switch it all as a separate patch).

Fix the typos, then you can add:
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 --]