[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

From: Paolo Bonzini
Subject: Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
Date: Tue, 1 Dec 2020 18:16:14 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0

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

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

Yes, I found bugs that the QAPI schema would have prevented. They were
generally about not checking whether mandatory propertes are actually

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).


reply via email to

[Prev in Thread] Current Thread [Next in Thread]