11:16 a.m.
On 01/12/20 17:20, Kevin Wolf wrote:
Am 30.11.2020 um 20:35 hat Paolo Bonzini geschrieben:
> 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.
Sure, the number of devices means that we can't convert all of them at
once. And we don't need to, we can make the change incrementally.
There's also the question that most devices are not present in all
binaries. So QAPI introspection would tell you what properties are
supported but not what devices are. Also the marshaling/unmarshaling
code for hundreds of devices would bloat the QEMU executables
unnecessarily (it'd all be reachable from visit_DeviceOptions so it'd
not be possible to compile it out, even with LTO).
Plus the above issue with machine types.
>>> 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.
Well, we all know that this series duplicates things. But at the same
time, we also know that this isn't going to be the final state.
Once QAPI learns about QOM inheritance (which it has to if it should
generate the boilerplate), it will know which objects are user creatable
without a an explicitly defined separate enum.
I think it will still need to have the enum internally, but as long as
it's automatically generated, that shouldn't be a big deal.
Right, I don't want to have the final state now but I'd like to have a
rough idea of the plan:
1) whether to generate _all_ boilerplate or only properties
2) whether we want to introduce a separation between configuration
schema and run-time state
3) in the latter case, whether properties will survive at all---iothread
and throttle-groups don't really need them even if they're writable
after creation.
These questions have a substantial effect on how to handle this series.
Without answers to this questions I cannot understand the long term
and its interaction with other long term plans for QOM.
>> 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.
QAPI is first and foremost about user-visible parts, and in fact most of
the problems I listed are about external interfaces.
Yes, but QAPI is also about interfacing with existing code. Also, QAPI
does not generate only structs, it also generate C function prototypes.
I'm not sure whether a QOM object's more similar to the struct case
(what you do here) or to the QMP command case:
- there's a 1:1 correspondence between ObjectOptions cases and
ucc->complete implementations
- there's a registry of object types just like there's one for QMP commands.
So another possibility could be that the QAPI generator essentially
generated the user_creatable_add_type function that called out to
user-provided functions qom_scsi_pr_helper_complete,
qom_throttle_group_complete, etc. The arguments to those functions
would be the configuration. That is a very interesting prospect (though
one that would require changes to the QAPI code generator).
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. 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.
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