From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: eblake@redhat.com, berrange@redhat.com, armbru@redhat.com,
ehabkost@redhat.com, qemu-devel@nongnu.org
Subject: Re: [RFC PATCH 00/12] QOM/QAPI integration part 1
Date: Thu, 4 Nov 2021 15:26:42 +0100 [thread overview]
Message-ID: <YYPtokwlkWzhrJ2u@redhat.com> (raw)
In-Reply-To: <13f8981a-55e5-e5d9-415f-4658aba16270@redhat.com>
Am 04.11.2021 um 13:39 hat Paolo Bonzini geschrieben:
> On 11/4/21 10:07, Kevin Wolf wrote:
> > The class implementations always want to store only their "local" config
> > options, but 'qom-config:classname' contains those of the parent class
> > as well.
>
> Ah, I didn't understand that (hence the rubbish tag above). It makes sense
> given that instance_config is called per-class while ObjectOptions stores
> all the info in one class. That's a major change from my sketch, which
> planned to call the base class config function by hand (and handle the
> marshalling via QAPI 'base': '...').
Yeah, handling inheritance and how to represent things in the schema is
probably the two more interesting things this series changes compared to
your proposal.
I started with your model, but it just didn't work out nicely, because I
always had the full configuration in the child class and apart from just
being ugly, having all options of the parent class duplicated, but
ignored, would certainly be a source for a lot of confusion and bugs.
It took me a while to figure out how to deal with this, but I'm quite
happy with the result.
> > Oh, and I also wanted to say something about why not just directly using
> > the class name, which was my first idea: 'foo': 'iothread' looks more
> > like referencing an existing iothread rather than the configuration for
> > a new one. I wanted to leave us the option that we could possibly later
> > take a string for such options (a QOM path) and then pass the referenced
> > object to QMP commands as the proper QOM type.
>
> I agree that 'iothread' is going to be confusing when you're referring to
> the configuration.
>
> Anyway I'm totally fine with 'qom-config:classname'. Given this
> explanation, however, one alternative that makes sense could be
> 'classname:full-config'. Then you could use 'classname:config' for the
> autoboxed configs---and reserve 'classname' to mean the pointer to an
> object. Classes are (generally) lowercase and QAPI structs are
> CamelCase, so there is not much potential for collisions.
Makes sense to me, too.
I just checked and I actually already forbid class names with colons in
them (check_name_str() takes care of this), so yes, suffixes actually
work on the QAPI level.
If we actually want to use these types in manually written C code, we
might have to convert the name to CamelCase, though, for consistency
with the coding style.
We already have a function camel_to_upper(), we'd need a new
lower_to_camel(), so that from a class 'rng-random', you would get types
'RngRandomConfig' (the local ones) and 'RngRandomFullConfig' (with
parent options).
Kevin
next prev parent reply other threads:[~2021-11-04 14:33 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-03 17:29 [RFC PATCH 00/12] QOM/QAPI integration part 1 Kevin Wolf
2021-11-03 17:29 ` [RFC PATCH 01/12] qapi: Add visit_next_struct_member() Kevin Wolf
2021-11-03 17:29 ` [RFC PATCH 02/12] qom: Create object_configure() Kevin Wolf
2021-11-23 15:23 ` Markus Armbruster
2021-12-14 9:52 ` Kevin Wolf
2021-11-03 17:29 ` [RFC PATCH 03/12] qom: Make object_configure() public Kevin Wolf
2021-11-03 17:29 ` [RFC PATCH 04/12] qom: Add instance_config() to TypeInfo Kevin Wolf
2021-11-03 17:29 ` [RFC PATCH 05/12] rng-random: Implement .instance_config Kevin Wolf
2021-11-03 17:29 ` [RFC PATCH 06/12] rng-backend: " Kevin Wolf
2021-11-03 17:29 ` [RFC PATCH 07/12] qapi: Allow defining QOM classes Kevin Wolf
2021-11-23 10:02 ` Markus Armbruster
2021-12-10 17:53 ` Kevin Wolf
2021-11-03 17:29 ` [RFC PATCH 08/12] qapi: Create qom-config:... type for classes Kevin Wolf
2021-11-23 13:00 ` Markus Armbruster
2021-12-10 17:41 ` Kevin Wolf
2021-11-03 17:29 ` [RFC PATCH 09/12] qapi/qom: Convert rng-backend/random to class Kevin Wolf
2021-11-23 13:15 ` Markus Armbruster
2021-12-10 17:57 ` Kevin Wolf
2021-11-03 17:30 ` [RFC PATCH 10/12] qapi: Generate QOM config marshalling code Kevin Wolf
2021-11-23 14:16 ` Markus Armbruster
2021-12-10 16:50 ` Kevin Wolf
2021-11-03 17:30 ` [RFC PATCH 11/12] qapi/qom: Add class definition for rng-builtin Kevin Wolf
2021-11-03 17:30 ` [RFC PATCH 12/12] qapi/qom: Add class definition for rng-egd Kevin Wolf
2021-11-03 21:26 ` [RFC PATCH 00/12] QOM/QAPI integration part 1 Paolo Bonzini
2021-11-04 9:07 ` Kevin Wolf
2021-11-04 12:39 ` Paolo Bonzini
2021-11-04 14:26 ` Kevin Wolf [this message]
2021-11-04 14:49 ` Paolo Bonzini
2021-11-04 15:51 ` Kevin Wolf
2021-11-04 15:52 ` Damien Hedde
2021-11-05 13:55 ` Kevin Wolf
2021-11-23 16:05 ` Markus Armbruster
2021-12-14 10:23 ` Kevin Wolf
2021-12-14 10:40 ` Peter Maydell
2021-12-14 11:52 ` Kevin Wolf
2021-12-14 14:45 ` Markus Armbruster
2021-12-14 16:00 ` Kevin Wolf
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=YYPtokwlkWzhrJ2u@redhat.com \
--to=kwolf@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=eblake@redhat.com \
--cc=ehabkost@redhat.com \
--cc=pbonzini@redhat.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 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).