[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 00/12] QOM/QAPI integration part 1
From: |
Kevin Wolf |
Subject: |
Re: [RFC PATCH 00/12] QOM/QAPI integration part 1 |
Date: |
Thu, 4 Nov 2021 15:26:42 +0100 |
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
- Re: [RFC PATCH 08/12] qapi: Create qom-config:... type for classes, (continued)
- [RFC PATCH 09/12] qapi/qom: Convert rng-backend/random to class, Kevin Wolf, 2021/11/03
- [RFC PATCH 12/12] qapi/qom: Add class definition for rng-egd, Kevin Wolf, 2021/11/03
- [RFC PATCH 11/12] qapi/qom: Add class definition for rng-builtin, Kevin Wolf, 2021/11/03
- [RFC PATCH 10/12] qapi: Generate QOM config marshalling code, Kevin Wolf, 2021/11/03
- Re: [RFC PATCH 00/12] QOM/QAPI integration part 1, Paolo Bonzini, 2021/11/03
- Re: [RFC PATCH 00/12] QOM/QAPI integration part 1, Damien Hedde, 2021/11/04
- Re: [RFC PATCH 00/12] QOM/QAPI integration part 1, Kevin Wolf, 2021/11/05
Re: [RFC PATCH 00/12] QOM/QAPI integration part 1, Markus Armbruster, 2021/11/23