qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Krempa <pkrempa@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: kwolf@redhat.com, vsementsov@virtuozzo.com, berrange@redhat.com,
	libvir-list@redhat.com, eblake@redhat.com,
	mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org,
	marcandre.lureau@redhat.com, jsnow@redhat.com,
	libguestfs@redhat.com
Subject: Re: [PATCH RFC 1/5] qapi: Enable enum member introspection to show more than name
Date: Fri, 17 Sep 2021 16:49:33 +0200	[thread overview]
Message-ID: <YUSq/ZDfLPInPIc8@angien.pipo.sk> (raw)
In-Reply-To: <20210915192425.4104210-2-armbru@redhat.com>

On Wed, Sep 15, 2021 at 21:24:21 +0200, Markus Armbruster wrote:
> The next commit will add feature flags to enum members.  There's a
> problem, though: query-qmp-schema shows an enum type's members as an
> array of member names (SchemaInfoEnum member @values).  If it showed
> an array of objects with a name member, we could simply add more
> members to these objects.  Since it's just strings, we can't.
> 
> I can see three ways to correct this design mistake:
> 
> 1. Do it the way we should have done it, plus compatibility goo.
> 
>    We want a ['SchemaInfoEnumMember'] member in SchemaInfoEnum.  Since
>    changing @values would be a compatibility break, add a new member
>    @members instead.
> 
>    @values is now redundant.  We should be able to get rid of it
>    eventually.
> 
>    In my testing, output of qemu-system-x86_64's query-qmp-schema
>    grows by 11% (18.5KiB).

I prefer this one. While the schema output grows, nobody is really
reading it manually.

The implementation in libvirt is very straightforward:

https://gitlab.com/pipo.sk/libvirt/-/commit/24f1709cfae72cd765d02dc2124d6c954554ea55
https://gitlab.com/pipo.sk/libvirt/-/commit/5909db9d4994327b3f64d5c329edd4b175495bfe

> 2. Like 1, but omit "boring" elements of @member, and empty @member.
> 
>    @values does not become redundant.  Output of query-qmp-schema
>    grows only as we make enum members non-boring.

This has 2 drawbacks:
- we would never get rid of either of those
- clients would have to check both, one for whether the member is
  present and second whether it's non-boring.

IMO it's simpler for clients just to prefer the new approach when
present as the old is simply a subset.

> 3. Versioned query-qmp-schema.
> 
>    query-qmp-schema provides either @values or @members.  The QMP
>    client can select which version it wants.

At least for libvirt this poses a chicken & egg problem. We'd have to
query the schema to see that it has the switch to do the selection and
then probe with the modern one.



  parent reply	other threads:[~2021-09-17 14:55 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-15 19:24 [PATCH RFC 0/5] Subject: [PATCH RFC 0/5] qapi: Add feature flags to enum members Markus Armbruster
2021-09-15 19:24 ` [PATCH RFC 1/5] qapi: Enable enum member introspection to show more than name Markus Armbruster
2021-09-17 13:56   ` Eric Blake
2021-09-20  8:57     ` Markus Armbruster
2021-09-17 14:49   ` Peter Krempa [this message]
2021-09-20  9:08     ` Markus Armbruster
2021-09-20  9:57       ` Peter Krempa
2021-10-09  8:08         ` Markus Armbruster
2021-09-15 19:24 ` [PATCH RFC 2/5] qapi: Add feature flags to enum members Markus Armbruster
2021-09-17 14:41   ` Eric Blake
2021-09-17 14:53   ` Peter Krempa
2021-09-15 19:24 ` [PATCH RFC 3/5] qapi: Move compat policy from QObject to generic visitor Markus Armbruster
2021-09-17 14:45   ` Eric Blake
2021-09-15 19:24 ` [PATCH RFC 4/5] qapi: Implement deprecated-input={reject, crash} for enum values Markus Armbruster
2021-09-15 19:24 ` [PATCH RFC 5/5] block: Deprecate transaction type drive-backup Markus Armbruster
2021-09-16  7:18 ` [PATCH RFC 0/5] Subject: [PATCH RFC 0/5] qapi: Add feature flags to enum members Vladimir Sementsov-Ogievskiy
2021-09-16 12:57   ` Markus Armbruster
2021-09-16 13:28     ` Vladimir Sementsov-Ogievskiy
2021-09-16 14:12       ` Markus Armbruster

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=YUSq/ZDfLPInPIc8@angien.pipo.sk \
    --to=pkrempa@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=eblake@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=libguestfs@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).