On Mon, 10 Feb 2014 14:36:20 -0700
Eric Blake <ebl...@redhat.com> wrote:
> On 02/10/2014 02:16 PM, Luiz Capitulino wrote:
> > On Sat, 1 Feb 2014 12:52:42 +0100
> > Martin Kletzander <mklet...@redhat.com> wrote:
> >
> >> Introduce 'query-chardev-backends' QMP command which lists all
> >> supported character device backends.
> >>
> >> Signed-off-by: Martin Kletzander <mklet...@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).
Do you envision whether, by applying Martin's patch now, it would be
compatible to do what you suggest on top?
>
> [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).
>
