Eric Blake <ebl...@redhat.com> writes: > 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.
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? [...]