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