[PATCH 00/18] qapi/qom: QAPIfy object-add

[Kevin Wolf]

Am 02.12.2020 um 17:05 hat Eduardo Habkost geschrieben:
> > > Looks nice as end goal.  Then, these are a few questions I would
> > > have about the transition plan:
> > > 
> > > Would it require changing both device implementation and device
> > > users in lockstep?  Should we have a compatibility layer to allow
> > > existing qdev_new()+qdev_prop_set_*() code to keep working after
> > > the devices are converted to the new system?  If not, why not?
> > 
> > Technically, it doesn't strictly require a lockstep update. You can have
> > two code paths leading to a fully initialised device, one being the
> > traditional way of setting properties and finally calling dc->realize,
> > the other being a new method that takes the configuration in its
> > parameters and also sets dev->realized = true at the end.
> > 
> > If at all possible, I would however prefer a lockstep update because
> > having two paths is a weird intermediate state and the code paths could
> > easily diverge. Keeping the old way around for a device also means that
> > property setters are still doing two different jobs (initial
> > configuration and updates at runtime).
> 
> I'd like to understand better how that intermediate state would
> look like and why there's risk of separate code paths diverging.
>
> Could we have an intermediate state that doesn't require any
> duplication and thus have no separate code paths that could
> diverge?

The one requirement we have for an intermediate state is that it
supports both interfaces: The well-know create/set properties/realize
dance, and a new DeviceClass method, say .create(), that takes the
configuration in parameters instead of relying on previously set
properties.

I assumed two separate implementations of transferring the configuration
into the internal state. On second thought, this assumption is maybe
wrong.

You can implement the new method as wrapper around the old way: It could
just set all the properties and call realize. Of course, you don't win
much in terms of improving the class implementation this way, but just
support the new interface, but I guess it can be a reasonable
intermediate step to resolve complicated dependencies etc.

It would be much nicer to do the wrapper the other way round, i.e.
setting properties before the device is realized would update a
configuration struct and realize would then call .create() with that
struct. To me, this sounds much harder, though also a more useful state.

As you have worked a lot with properties recently, maybe you have a good
idea how we could get an intermediate state closer to this?

> > > If we add a compatibility layer, is the end goal to convert all
> > > existing qdev_new() users to the new system?  If yes, why?  If
> > > not, why not?
> > 
> > My personal goal is covering -object and -device, i.e. the external
> > interfaces. Converting purely internally created devices is not as
> > interesting (especially as long as we focus only on object creation),
> > but might be desirable for consistency.
> 
> I wonder how much consistency we will lose and how much confusion
> we'll cause if we end up with two completely separate methods for
> creating devices.

I do think we should follow through and convert everything. It's just
not my main motivation, and if the people who work more with qdev think
it's better to leave that part unchanged (or that it won't make much of
a difference), I won't insist.

> > > What about subclasses?  Would base classes need to be converted
> > > in lockstep with all subclasses?  How would the transition
> > > process of (e.g.) PCI devices look like?
> > 
> > I don't think so.
> > 
> > If you want to convert base classes first, you may need to take the
> > path shown above where both initialisation paths coexist while the
> > children are converted because instantiation of a child class involves
> > setting properties of the base class. So you can only restrict these
> > properties to runtime-only after all children have been converted.
> > 
> > The other way around might be easier: You will need to describe the
> > properties of base classes in the QAPI schema from the beginning, but
> > that doesn't mean that their initialisation code has to change just yet.
> > The child classes will need to forward the part of their configuration
> > that belongs to the base class. The downside is that this code will have
> > to be changed again when the base class is finally converted.
> > 
> > So we have options there, and we can decide case by case which one is
> > most appropriate for the specific class to be converted (depending on
> > how many child classes exist, how many properties are inherited, etc.)
> 
> Right now it's hard for me to understand what those intermediate
> states would look like.  It sounds like it requires too many
> complicated manual changes to be done by humans, and lots of room
> for mistakes when maintaining two parallel code paths.  I'd
> prefer to delegate the translation job to a machine.

Maybe devices are in a better shape, but my conclusion from user
creatable objects is that it needs to be done by a human.

Even just writing a schema for an existing object without actually
changing its code (i.e. what this series does for user creatable
objects) involves:

* Figuring out which properties even exist.

  I guess if you know your way around QOM, this could be automatically
  be generated for the common case. If property definitions are
  conditional or dynamic, you'll probably miss them.

* Finding the right data type for each property.

  The const char *type passed to object_(class_)property_add() is often
  enough wrong that it becomes useless. If object_property_add_<type> is
  used, chances are that you know the right type, but strings might
  actually be hidden enums. If not, figuring out the type involves
  analysing the setter and getter callbacks.

* Finding out which properties are mandatory and which are optional.

  This is usually some handwritten check in complete/realize that
  returns an error. Sometimes it's just a segfault that happens sooner
  or later if the property hasn't been set.

* Finding the default for documentation.

  There are multiple ways to do this. It's code, not data.

* Writing (preferably good) documentation for the property.

I see very little opportunity for automating a significant part of this.

Once you have this information, going to the intermediate state where
.create() is just a wrapper that sets properties and calls realize is
fairly easy. Maybe we can have QAPI support for this so that you can
request generation of the wrapper function in the schema. Then you would
just have to set the pointer to it in .class_init.

> In other words, I'd prefer to have compatibility layer(s) that
> would make the same implementation work with the both old-style
> and new-style APIs for creating devices.  Maybe this would
> involve generating QAPI code from QOM/qdev property lists, and/or
> generating qdev property registration code from the QAPI schema,
> I don't know.
> 
> Providing a compatibility layer would also help us be more
> confident that there are no gaps in the abstractions provided by
> the two systems (QOM properties and QAPI schema) that we still
> need to address.

qdev properties could be more useful to generate at least a skeleton
schema from than generic QOM properties. But there will still be large
parts that a human needs to do.

My concern with the compatibility layer is just that it could happen
more easily that we're stuck with it forever.

Kevin