qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
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



  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).