1:35 p.m.
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