Re: [Qemu-devel] [PATCH v3] qmp: expose list of supported character device backends

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 02/10/2014 02:16 PM, Luiz Capitulino wrote:
>> On Sat,  1 Feb 2014 12:52:42 +0100
>> Martin Kletzander <address@hidden> wrote:
>> 
>>> Introduce 'query-chardev-backends' QMP command which lists all
>>> supported character device backends.
>>>
>>> Signed-off-by: Martin Kletzander <address@hidden>
>>> ---
>
>>> +##
>>> +{ 'type': 'ChardevBackendInfo', 'data': {'name': 'str'} }
>> 
>> We already have ChardevBackend, it's an union though. I'm wondering if
>> you could change it to an enum and use it instead of plain 'str'?
>
> Hmm, right now, the ChardevBackend union pre-dates when we added flat
> unions.  For flat unions, we can set a discriminator to be an enum type
> [1], at which point the code generator then validates that we cover all
> values of the enum in branches of the union; maybe it's worth
> retro-fitting simple unions to also take advantage of the additional
> coverage of the discriminator being an enum.

Yes, and Wenchao Xia has been working towards that: "[PATCH V5 00/10]
qapi script: support enum as discriminator and better enum name".

> That is, right now, we have:
>
> { 'union': 'ChardevBackend', 'data': { 'file'   : 'ChardevFile',
>                                        'serial' : 'ChardevHostdev',
>
> and we also document in qapi-code-gen.txt that when using
> 'discriminator', you either have to have a base class (and the
> discriminator is a string-typed member of that base class), or the
> discriminator is {} because it is an anonymous union.  But I'm asking
> about yet another situation, of having a typed discriminator with no
> change to the wire format (no base class), something like:
>
> { 'enum': 'ChardevBackendTypes', [ 'file', 'serial', ... ] }
> { 'union': 'ChardevBackend',
>   'discriminator': 'ChardevBackendTypes',
>   'data': { 'file': 'ChardevFile', ...
>
> The benefit of such a plan is that we then have an introspectible enum
> of all possible backends known at compile time (ChardevBackendTypes),
> and the new addition in this patch becomes:
>
> { 'type': 'ChardevBackendInfo',
>   'data': {'name', 'ChardevBackendTypes' } }
>
> rather than raw 'str', while still allowing potential future additions
> of additional backend info.  Note that there would still be a difference
> between ChardevBackendTypes (an enum of all possible known types at
> compile time) vs. query-chardev-backends (a runtime list of the possible
> types that can be used for this particular machine, even if it is a
> subset of all possible ChardevBackendTypes).
>
> [1] actually, did those patches ever get applied, and we just missed
> documenting it in qapi-code-gen.txt, or are they still pending review?

By "those", do you mean Wenchao Xia's patches?

[...]