Re: [RFC PATCH 00/12] QOM/QAPI integration part 1

[Kevin Wolf <kwolf@redhat.com>]

Am 03.11.2021 um 22:26 hat Paolo Bonzini geschrieben:
> On 11/3/21 18:29, Kevin Wolf wrote:
> > This series adds QOM class definitions to the QAPI schema, introduces
> > a new TypeInfo.instance_config() callback that configures the object at
> > creation time (instead of setting properties individually) and is
> > separate from runtime property setters (which often used to be not
> > really tested for runtime use), and finally generates a marshalling
> > function for .instance_config() from the QAPI schema that makes this a
> > natural C interface rather than a visitor based one.
> 
> That's pretty cool!
> 
> Just one question: why not always use boxed configuration?  It should not
> make the instance_config types any larger, and it avoids unwieldy argument
> lists.

Basically for the same reason as for commands (and for consistency with
commands): If you have only one or two options, then creating a separate
type for them feels just a little over the top, and boxing doesn't work
with implicit types.

I really like the concise definitions without a separate struct like in:

{ 'class': 'rng-egd',
  'parent': 'rng-backend',
  'config': { 'chardev': 'str' } }

Though maybe we could make it work by giving the implicit type another
prefixed name?

> Also, for the obligatory bikeshedding remark, do you have any other plans or
> ideas for the colon-separated auto generated typenames?  Having both the
> "namespace" (qom) and the more specific use (config) before the classname is
> a bit weird, compared to the existing structs like RngRandomProperties.
> Especially if boxed config is more used (or becomes the only possibility),
> it might be that qom:class-name:config, or even just class-name:config, make
> for nicer code in the object implementation.

'qom-config:classname' isn't a type that is useful for the object
implementations at all. Its only use is really storing the whole
configuration temporarily in a QAPI C struct before applying it.

The class implementations always want to store only their "local" config
options, but 'qom-config:classname' contains those of the parent class
as well.

In my example conversion, I left RngProperties as a separate type that
is just referenced in the class definition:

{ 'class': 'rng-backend',
  'config': 'RngProperties' }

This is what you would do when rng-backend wants to store its options in
a single struct, and then it's still RngProperties.

As for the naming, I chose a prefix because I think QOM doesn't restrict
the characters used in class names, so suffixes could become ambiguous.
The colon is just a character that is invalid in QAPI user types and
looked nice to me, any other reserved character would do.

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.

(Maybe there aren't currently many commands that take specific QOM
objects, but I'm dreaming of a future state with QMP commands that take
'block-node' arguments etc.)

Kevin