Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On Tue, Dec 01, 2020 at 06:16:14PM +0100, Paolo Bonzini wrote:
On 01/12/20 17:20, Kevin Wolf wrote:
[...]
> BlockdevOptions is about external interfaces, not about
> implementation details. Same thing as QOM properties are external
> interfaces, not implementation details. There may be fields in the
> internal state that correspond 1:1 to the externally visible QAPI
> type/QOM property, but it's not necessarily the case.
Right. It may well be that we decide that, as a result of this series,
QOM's property interface is declared essentially a failed experiment. I
wouldn't be surprised, and that's why I want to understand better where we
want to go.
For example, Eduardo is focusing specifically on external interfaces that
correspond 1:1 to the internal implementation. If we decide that
non-1:1-mappings and checks on mandatory properties are an important part of
QOM, that may make large parts of his work moot. [...]
Whatever we decide, my first and biggest worry is to have a
reasonable migration path for any new API.
We could describe in detail what's the API we _want_, but we also
need a reasonable migration path for the code we already _have_.
If a new API that requires manual conversion of existing devices,
it will probably coexist with the existing qdev property API for
years.
(Note that I haven't read this whole thread yet.)
[...] If we decide that
most QOM
objects need no properties at all, then we don't want to move more
qdev-specific stuff from to QOM.
I don't understand what "move more qdev-specific stuff to QOM"
means. I consider the QOM field property API valuable even if we
decide the only user of the API will be legacy qdev code, because
it separates the core mechanism (that's code that already
existed) from qdev-specific policies.
> QAPI is already here and it's going to stay. QOM doesn't have to
> duplicate input validation that existing code can already perform.
>
> I'm not sure which complexity you think I'm introducing: QAPI is already
> there. I'm adding the schema, which you agree is valuable documentation,
> so we want to have it either case. The actual change to QOM that we have
> in this series is this:
The complexity is that properties used to be split in two places, and now
they're split in three places.
It may very well be that this is a good first step to at least have classes
described in the QAPI schema. But since _in the short term_ there are
things that the series makes worse (and has a risk of bringing things out of
sync), I'd like to understand the long term plan and ensure that the QAPI
maintainers are on board with it.
Can you at least add a comment to all UserCreatable classes that says "if
you add a property, remember to modify ... as well in the QAPI schema"?
> > 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).
>
> Yes, I found bugs that the QAPI schema would have prevented. They were
> generally about not checking whether mandatory propertes are actually
> set.
Okay, I found your series at
https://patchew.org/QEMU/20201130105615.21799-1-kwolf@redhat.com/ too, good
to know.
So that's another useful thing that can be chalked to this series at least
if -object and object_add are converted (and also, another thing against QOM
properties and 1:1 mappings between configuration schema and run-time
state).
Paolo
--
Eduardo