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

[Paolo Bonzini]

On 30/11/20 19:10, Kevin Wolf wrote:
> Am 30.11.2020 um 17:57 hat Paolo Bonzini geschrieben:
> > The main problem is that it wouldn't extend well, if at all, to
> > machines and devices.  So those would still not be integrated into the
> > QAPI schema.
>
> What do you think is the biggest difference there? Don't devices work
> the same as user creatable objects in that you first set a bunch of
> properties (which would now be done through QAPI instead) and then call
> the completion/realize method that actually makes use of them?

For devices it's just the practical issue that there are too many to 
have something like this series.  For machine types the main issue is 
that the version-specific machine types would have to be defined in one 
more place.

> > The single struct doesn't bother me _too much_ actually.  What bothers me is
> > that it won't be a single source of all QOM objects, only those that happen
> > to be created by object-add.
>
> But isn't it only natural that a list of these objects will exist in
> some way, implicitly or explicitly? object-add must somehow decide which
> object types it allows the user to create.

There's already one, it's objects that implement user creatable.  I 
don't think having it as a QAPI enum (or discriminator) is worthwhile, 
and it's one more thing that can go out of sync between the QAPI schema 
and the C code.

> I'm also pretty sure that QOM as it exists now is not the right solution
> for any of them because it has some major shortcomings. It's too easy to
> get things wrong (like the writable properties after creation), its
> introspection is rather weak and separated from the QAPI introspection,
> it doesn't encourage documentation, and it involves quite a bit of
> boilerplate and duplicated code between class implementations.
>
> A modified QOM might be the right solution, though. I would like to
> bring QAPI and QOM together because most of these weaknesses are
> strengths of QAPI.

I agree wholeheartedly.  But your series goes to the opposite excess. 
Eduardo is doing work in QOM to mitigate the issues you point out, and 
you have to meet in the middle with him.  Starting with the user-visible 
parts focuses on the irrelevant parts.

> > Mostly because it's more or less the same issue that you have with
> > BlockdevOptions, with the extra complication that this series only deals
> > with the easy one of the four above categories.
>
> I'm not sure which exact problem with BlockdevOptions you mean. The
> reason why the block layer doesn't use BlockdevOptions everywhere is
> -drive support.

You don't have a single BlockdevOptions* field in the structs of block/. 
 My interpretation of this is that BlockdevOptions* is a good schema 
for configuring things but not for run-time state.

> > > Maybe if we don't want to commit to keeping the ObjectOptions schema,
> > > the part that should wait is object-add and I should do the command line
> > > options first? Then the schema remains an implementation detail for now
> > > that is invisible in introspection.
> >
> > I don't see much benefit in converting _any_ of the three actually.  The
> > only good reason I see for QAPIfying this is the documentation, and the
> > promise of deduplicating QOM boilerplate.  The latter is only a promise, but
> > documentation alone is a damn good reason and it's enough to get this work
> > into a mergeable shape as soon as possible!
>
> I think having some input validation done by QAPI instead of in each QOM
> object individually is nice, too. You get it after CLI, QMP and HMP all
> go through QAPI.

But the right way to do that is to use Eduardo's field properties: they 
make QOM do the right thing, adding another layer of parsing is just 
adding more complexity.  Are there any validation bugs that you're 
fixing?  Is that something that cannot be fixed elsewhere, or are you 
papering over bad QOM coding?  (Again, I'm not debating that QOM 
properties are hard to write correctly).

> > But maybe, we could start in the opposite direction: start with the use QAPI
> > to eliminate QOM boilerplate.  Basing your work on Eduardo's field
> > properties series, you could add a new 'object' "kind" to QAPI that would
> > create an array of field properties (e.g. a macro expanding to a compound
> > literal?)
>
> There is a very simple reason why I don't want to start with the QAPI
> generator: John has multiple series pending that touch basically every
> part of the QAPI generator. This means not only that I need to be
> careful about merge conflict (or base my work on top of five other
> series, which feels adventurous), but also that I would be competing
> with John for Markus' reviewer capacity, further slowing things down.

That's something that you have to discuss with John and Markus.  It may 
be that John has to shelve part of his work or rebase on top of yours 
instead.

By adding an extra parsing layer you're adding complexity that may not 
be needed in the end, and we're very bad of removing it later.  So I'm 
worried that this series will become technical debt in just a few months.

> Well, two reasons: Also because this series for the external interface
> of the objects already exists and it's an incremental step towards your
> proposal: The types for 'properties' will already exist then and I won't
> have to convert both internal state and external interfaces at the same
> time.

I don't think converting internal state to QAPI is useful.  QAPI and QOM 
are external interfaces and that's what they should remain; anything 
that is not an external interface should (must) remain plain C code.

Paolo