From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: marcandre.lureau@redhat.com, qemu-devel@nongnu.org,
ehabkost@redhat.com, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH v7 07/14] qapi: Move union tag quirks into subclass
Date: Thu, 8 Oct 2015 09:02:29 -0600 [thread overview]
Message-ID: <56168585.7070503@redhat.com> (raw)
In-Reply-To: <878u7dk0mr.fsf@blackfin.pond.sub.org>
[-- Attachment #1: Type: text/plain, Size: 2008 bytes --]
On 10/08/2015 06:25 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> Right now, simple unions have a quirk of using 'kind' in the C
>> struct to match the QMP wire name 'type'. This has resulted in
>> messy clients each doing special cases. While we plan to
>> eventually rename things to match, it is better in the meantime
>> to consolidate the quirks into a special subclass, by adding a
>> new member.c_name() function. This will also make it easier
>> for reworking how alternate types are laid out in a future
>> patch. Use the new c_name() function where possible.
>>
>> No change to generated code.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>
>> ---
>> v7: new patch, but borrows idea of subclass from v6 10/12, as
>> well as c_name() from 7/12
>> ---
>> scripts/qapi-commands.py | 8 ++++----
>> scripts/qapi-types.py | 12 +++++-------
>> scripts/qapi-visit.py | 17 +++++------------
>> scripts/qapi.py | 15 +++++++++++++--
>> 4 files changed, 27 insertions(+), 25 deletions(-)
>
> My immediate reaction to the subclass idea was "instead of encapsulating
> the flaw more nicely, why not fix it?" So gave that a try, see my other
> reply.
I had already done the same sort of fix, but it was just sitting later
in my series where you hadn't reached reviewing yet.
>
> That said, the diffstat shows the subclass idea doesn't take much code.
> May make sense if we feel we shouldn't fix the flaw now.
I also like the subclass idea because it makes simplifying alternates
easier (see my just-posted subset C).
But it sounds like getting rid of the 'type'/'kind' mismatch sooner
rather than later seems like the direction we should be heading.
If I need to spin a v8 of this series, I'll certainly include that
conversion (whether from mine, yours, or a combination of the two).
--
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 --]
next prev parent reply other threads:[~2015-10-08 15:02 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-04 3:40 [Qemu-devel] [PATCH v7 00/14] post-introspection cleanups, subset B Eric Blake
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 01/14] qapi: Use predicate callback to determine visit filtering Eric Blake
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 02/14] qapi: Prepare for errors during check() Eric Blake
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 03/14] qapi: Drop redundant alternate-good test Eric Blake
2015-10-07 16:15 ` Markus Armbruster
2015-10-07 16:33 ` Eric Blake
2015-10-13 8:12 ` Markus Armbruster
2015-10-13 12:31 ` Eric Blake
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 04/14] qapi: Don't use info as witness of implicit object type Eric Blake
2015-10-07 16:27 ` Markus Armbruster
2015-10-09 22:41 ` Eric Blake
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 05/14] qapi: Lazy creation of array types Eric Blake
2015-10-07 16:38 ` Markus Armbruster
2015-10-10 20:16 ` Eric Blake
2015-10-12 8:24 ` Markus Armbruster
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 06/14] qapi: Create simple union type member earlier Eric Blake
2015-10-07 16:44 ` Markus Armbruster
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 07/14] qapi: Move union tag quirks into subclass Eric Blake
2015-10-07 16:11 ` [Qemu-devel] [PATCH] fixup to " Eric Blake
2015-10-08 12:25 ` [Qemu-devel] [PATCH v7 07/14] " Markus Armbruster
2015-10-08 15:02 ` Eric Blake [this message]
2015-10-08 12:26 ` [Qemu-devel] [RFC PATCH] qapi: Rename simple union's generated tag member to type Markus Armbruster
2015-10-08 14:56 ` Eric Blake
2015-10-14 13:16 ` Eric Blake
2015-10-14 16:04 ` Markus Armbruster
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 08/14] qapi: Track location that created an implicit type Eric Blake
2015-10-08 14:19 ` Markus Armbruster
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 09/14] qapi: Track owner of each object member Eric Blake
2015-10-09 13:17 ` Markus Armbruster
2015-10-09 14:30 ` Eric Blake
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 10/14] qapi: Detect collisions in C member names Eric Blake
2015-10-09 14:11 ` Markus Armbruster
2015-10-09 14:33 ` Eric Blake
2015-10-12 8:34 ` Markus Armbruster
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check() Eric Blake
2015-10-12 15:53 ` Markus Armbruster
2015-10-12 16:22 ` Eric Blake
2015-10-13 4:10 ` Eric Blake
2015-10-13 7:08 ` Markus Armbruster
2015-10-13 12:46 ` Eric Blake
2015-10-13 15:39 ` Markus Armbruster
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 12/14] qapi: Move duplicate enum value " Eric Blake
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 13/14] qapi: Add test for alternate branch 'kind' clash Eric Blake
2015-10-04 3:41 ` [Qemu-devel] [PATCH v7 14/14] qapi: Detect base class loops Eric Blake
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56168585.7070503@redhat.com \
--to=eblake@redhat.com \
--cc=armbru@redhat.com \
--cc=ehabkost@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.