Kevin Wolf <kw...@redhat.com> writes:
> Am 23.11.2021 um 17:05 hat Markus Armbruster geschrieben:
>> Kevin Wolf <kw...@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!
