On 30/11/20 16:46, Kevin Wolf wrote:
Am 30.11.2020 um 15:58 hat Paolo Bonzini geschrieben:
With this series it's basically pointless to have QOM properties at
all.
Not entirely, because there are still some writable properties that can
be changed later on.
Are there really any (that are not bugs like opened/loaded)? That's also
why Eduardo and I discussed a class-wide allow_set function for his
field properties series.
So with this in mind, I think I'm in favour of completely leaving the
initialisation of properties on object creation to QAPI, and only
providing individual setters if we actually intend to allow property
changes after creation.
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.
So the question is, are we okay with shoveling half of QEMU's backend data
model into a single struct? If so, there are important consequences.
Yeah, the single struct bothers me a bit, both in the QAPI schema and in
the C source.
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. So I start to wonder if QOM as
it exists now is the right solution for all kind of objects:
- backends and other object-add types (not per-target, relatively few
classes and even fewer hierarchies)
- machine (per-target, many classes, no hierarchy)
- device (can be treated as not per-target, many many classes, very few
hierarchies)
- accelerator (per-target, few classes, no hierarchy)
- chardev (ok those are the same as the first category)
If QOM is the right solution, this patch goes in the wrong direction.
If QOM is the wrong solution, this patch is okay but then we have
another problem to solve. :)
The problem with this series is that you are fine with deduplicating things
as a third step, but you cannot be sure that such deduplication is possible
at all. So while I don't have any problems in principle with the
ObjectOptions concept, I don't think it should be committed without a clear
idea of how to do the third step.
Do you have any specific concerns why the deduplication might not
possible, or just because it's uncharted territory?
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.
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!
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?)
. Something like
+{ 'object': 'InputBarrier',
+ 'data': { 'name': 'str',
+ 'x-origin': 'int16',
+ 'y-origin': 'int16',
+ 'width': 'int16',
+ 'height': 'int16' },
+ 'properties': { 'server': 'str',
+ 'port': 'str' } }
would create a macro QOM_InputBarrier_FIELDS defining properties for the
following fields of the InputBarrier struct:
gchar *name;
int16_t x_origin, y_origin;
int16_t width, height;
while server and port would only appear in the documentation (or
alternatively you could leave them out completely, as you wish).
The advantages would be noticeable:
1) the information would be moved in the QAPI schema JSON from the
beginning, decoupling the conflict-heavy part from the complex question
of how to expose the QOM schema in the introspection data
2) there would not be any more duplication than before (there would be
duplication between structs and QAPI schema, but not between structs and
C code that defines properties).
3) it would be opt-in, so it doesn't put on you the burden of keeping
the series in sync with new objects that are added (I have one for the
qtest server for example). At the same time it would be quite appealing
for owners of QOM code to convert their objects to field properties and
get documentation for free.
4) we could special-case 'object' definitions and generate them in the
system manual as well, since they are also useful to document -object.
Yes it's a huge change but you have the boring part already done. What
do you think?
Paolo
In the meanwhile, of course I have no problem with deprecating the opened
and loaded properties.
If we decide that we don't want to have the schema at all (which I hope
we won't decide), I can split the deprecation into separate patches.
Kevin
