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

[Kevin Wolf]

Am 04.11.2021 um 16:52 hat Damien Hedde geschrieben:
> On 11/4/21 10:07, Kevin Wolf wrote:
> > 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!
> 
> Hi,
> 
> Just to be sure I understand. Is this config a generalization of the
> qdev-properties we have to describe the parameter used to create a
> device ?

Everything I'm doing here is on the QOM level. But qdev is just a layer
on top of QOM, so yes, eventually qdev properties should be declared in
the schema, too. It's useful to keep this long-term goal in mind, but
for now, the immediate goal is making it work for user-creatable
objects.

> > > 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?
> 
> What's an implicit type in this context ?

The value of 'config' creates an implicit struct type that is considered
anonymous on the QAPI level. With an explicit type, you wouldn't put an
object containing all the options there, but just a type name, like
this:

{ 'struct': 'RngEgdConfig',
  'data': { 'chardev': 'str' } }

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

> > 
> > > 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.
> 
> Would not this type be useful to generate read-only properties (and store
> the values) corresponding to the config if we expect to always have such
> properties ?

No, that's what I explained in the following paragraph. That would be a
different type, because 'qom-config:classname' contains the options for
the parent class, too, but you only want to store the options for the
class itself.

Kevin