From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33798) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WD8lZ-0001gK-05 for qemu-devel@nongnu.org; Tue, 11 Feb 2014 03:32:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WD8lS-0007VG-WA for qemu-devel@nongnu.org; Tue, 11 Feb 2014 03:32:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60714) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WD8lS-0007V1-Nm for qemu-devel@nongnu.org; Tue, 11 Feb 2014 03:32:22 -0500 From: Markus Armbruster References: <20140210161600.4f9b34a1@redhat.com> <52F94654.8080401@redhat.com> Date: Tue, 11 Feb 2014 09:32:17 +0100 In-Reply-To: <52F94654.8080401@redhat.com> (Eric Blake's message of "Mon, 10 Feb 2014 14:36:20 -0700") Message-ID: <87vbwmt5gu.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v3] qmp: expose list of supported character device backends List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Martin Kletzander , qemu-devel@nongnu.org, Anthony Liguori , Luiz Capitulino Eric Blake writes: > On 02/10/2014 02:16 PM, Luiz Capitulino wrote: >> On Sat, 1 Feb 2014 12:52:42 +0100 >> Martin Kletzander wrote: >> >>> Introduce 'query-chardev-backends' QMP command which lists all >>> supported character device backends. >>> >>> Signed-off-by: Martin Kletzander >>> --- > >>> +## >>> +{ '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? [...]