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). [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
signature.asc
Description: OpenPGP digital signature