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

[Eric Blake <eblake@redhat.com>]

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

On 02/10/2014 02:16 PM, Luiz Capitulino wrote:
> On Sat,  1 Feb 2014 12:52:42 +0100
> Martin Kletzander <mkletzan@redhat.com> wrote:
> 
>> Introduce 'query-chardev-backends' QMP command which lists all
>> supported character device backends.
>>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---

>> +##
>> +{ '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.

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?

> 
>> +
>> +##
>> +# @query-chardev-backends:
>> +#
>> +# Returns information about character device backends.
> 
> Actually, it returns information about registered backends (registration
> is done by register_char_driver_qapi(). So, I think it's good thing to
> mention that this list is about available backends.

Again, it sounds like you want to emphasize an enum of all possible
types (compile time, learned via introspection, whenever we get that)
vs. registered backends (possibly a subset based on what libraries were
used in building this qemu binary, and learned via the new QMP command).

-- 
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 --]