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

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 23.11.2021 um 17:05 hat Markus Armbruster geschrieben:
>> Kevin Wolf <kwolf@redhat.com> writes:
>> 
>> > 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.
>> >
>> > This is loosely based on Paolo's old proposal in the wiki:
>> > https://wiki.qemu.org/Features/QOM-QAPI_integration
>> >
>> > The series is in a rather early stage and I don't really have any
>> > automated tests or documentation in this series yet. I'm also only
>> > converting the class hierarchy for the random number generator backends
>> > to show what the result looks like, the other objects still need to be
>> > done.
>> >
>> > So the question to you isn't whether this is mergeable (it isn't), but
>> > whether you think this is the right approach for starting to integrate
>> > QOM and QAPI better.
>> >
>> > You'll also see that this doesn't really remove the duplication between
>> > property definitions in the code and configuration struct definitions in
>> > the QAPI schema yet (because we want to keep at least a read-only
>> > runtime property for every configuration option), but at least they mean
>> > somewhat different things now (creation vs. runtime) instead of being
>> > completely redundant.
>> >
>> > Possible future steps:
>> >
>> > * Define at least those properties to the schema that correspond to a
>> >   config option. For both setters and getters for each option, we'll
>> >   probably want to select in the schema between 'not available',
>> >   'automatically generated function' and 'manually implemented'.
>> >
>> >   Other runtime properties could be either left in the code or added to
>> >   the schema as well. Either way, we need to figure out how to best
>> >   describe these things in the schema.
>> 
>> Permit me a diversion of sorts.
>> 
>> With QOM, we have properties.  A property is readable if it has a
>> getter, writable if it has a setter.  There is no real concept of
>> configuration vs. state.  Writable properties can be written at any
>> time.
>> 
>> In practice, some properties are to be used only like configuration, and
>> we check configuration at realize time (for devices), or by a surrogate
>> like qemu_add_machine_init_done_notifier().  If you set them later,
>> things may break, and you get to keep the pieces.
>> 
>> In this "QOM/QAPI integration part 1", configuration (expressed in QAPI
>> schema) makes it into QOM.
>> 
>> Now we have configuration *and* properties.
>> 
>> Do we need the properties?
>
> Configuration is for creating objects, properties are for runtime after
> the creation. So for the practical answer, as long as you can find a QOM
> type that wants to allow either changing an option at runtime or just
> exposing its current value, I would say, yes, we need both. And I can
> easily list some QOM types that do.
>
> The theoretical answer is that of course you can replace properties with
> custom query-* and set-* QMP commands, but that's not only hardly an
> improvment, but also a compatibility problem.

That would be nuts.

> The approach I'm taking here with QAPIfication of objects (and planning
> to take for future conversions) is to drop setters that can't work at
> runtime (which might be the majority of properties), but keep properties
> around otherwise. Everything else would be a per-object decision, not
> part of the infrastructure work.

Getting rid of such setters makes sense.

It's been a while since I reviewed...  I don't remember anymore whether
we can have configuration parameters that are also properties.  If yes,
would it make sense to generate such properties?

>> Note I'm not asking whether we need setters.  I'm asking whether we
>> need to expose configuration bits via qom-set & friends in addition to
>> the QAPI schema and query-qmp-schema.
>
> I'm not sure I follow here. How is querying or changing option values
> redundant with querying which options exist?
>
> Maybe qom-list could become obsolete if we move all properties (and not
> just the configuration) into the QAPI schema, but I don't see qom-get
> and qom-set going away.
>
>> > * Getting rid of the big 'object-add' union: While the union is not too
>> >   bad for the rather small number of user-creatable objects, it
>> >   wouldn't scale at all for devices.
>> >
>> >   My idea there is that we could define something like this:
>> >
>> >   { 'struct': 'ObjectOptions',
>> >     'data': {
>> >         'id': 'str',
>> >         'config': { 'type': 'qom-config-any:user-creatable',
>> >                     'embed': true } } }
>> >
>> >   Obviously this would be an extension of the schema language to add an
>> >   'embed' option (another hopefully more acceptable attempt to flatten
>> >   things...), so I'd like to hear opinions on this first before I go to
>> >   implement it.
>> 
>> 'embed': true would splice in the members of a struct type instead of a
>> single member of that struct type.  Correct?
>> 
>> Stretch goal: make it work for union types, too :)
>> 
>> I've thought of this before.  Plenty of nesting in the wire format
>> exists pretty much only to let us have the C structs we want.  Right
>> now, the only way to "splice in" such a struct is the base type.
>> General splicing could be useful.  It may take an introspection flag
>> day.
>
> Base types aren't visible in the introspection either, so probably not
> if you continue to just report the resulting structure?

Yes, this should be feasible, except for splicing a union into a union,
because then you get multiple (tag, variants), which the introspection
schema can't do.  So don't go there, at least for now.

>> >   Also note that 'qom-config-any:user-creatable' is new, too. The
>> >   'qom-config:...' types introduced by this series don't work for
>> >   subclasses, but only for the exact class.
>> >
>> >   On the external interface, the new 'qom-config-any:...' type including
>> >   subclasses would basically behave (and be introspected) like the union
>> >   we have today, just without being defined explicitly.
>> 
>> I'm not sure I follow.  How is the qom-config-any:user-creatable to be
>> defined?  QAPI collects all the qom-config:* types into a union
>> automatically?
>
> All classes that inherit from user-creatable, but yes, automatically
> collected.
>
> For user-creatable, we can either introduce interfaces in QAPI, too, or
> we just pretend it's actually the top-level parent class.

Thanks!