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

[Paolo Bonzini]

On 01/12/20 20:35, Kevin Wolf wrote:
> Am 01.12.2020 um 18:16 hat Paolo Bonzini geschrieben:
> I don't think this is actually a new things. We already have types and
> commands declared with things like 'if': 'defined(TARGET_S390X)'.
> As far as I understand, QAPI generated files are already built per
> target, so not compiling things into all binaries should be entirely
> possible.

There is some complication due to having different discriminators per 
target.  So yes it should be possible.  But probably best left after 
objects because it's so much bigger a task and because objects have a 
bit more freedom for experimentation (less ties to other qdev-specific 
concepts, e.g. the magic "bus" property).

> So maybe only the abstract base class that actually defines the machine
> properties (like generic-pc-machine) should be described in QAPI, and
> then the concrete machine types would inherit from it without being
> described in QAPI themselves?

Yes, maybe.

> > 1) whether to generate _all_ boilerplate or only properties
>
> I would like to generate as much boilerplate as possible. That is, I
> don't want to constrain us to only properties, but at the same time, I'm
> not sure if it's possible to get rid of all boilerplate.
>
> Basically, the vision I have in mind is that QAPI would generate code
> that is the same for most instances, and then provide an option that
> prevents code generation for a specific part for more complicated cases,
> so that the respective function can (and must) be provided in the C
> source.

Ok, so that's a bit different from what I am thinking of.  I don't care 
very much about the internal boilerplate, only the external interface 
for configuration.  So I don't care about type registration, dynamic 
cast macros etc., only essentially the part that leads to ucc->complete.

> > 2) whether we want to introduce a separation between configuration
> > schema and run-time state
>
> You mean the internal run-time state? How is this separation not already
> present with getter/setter functions for each property? In many cases
> they just directly access the run-time state, but there are other cases
> where they actually do things.

I mean moving more towards the blockdev/chardev way of doing things, 
increasing the separation very much by having separate configuration 
structs that have (potentially) no link to the run-time state struct.

> > 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.
>
> How do you define properties, i.e. at which point would they stop
> existing and what would be a non-property alternative?

Properties are only a useful concept if they have a use.  If 
-object/object_add/object-add can do the same job without properties, 
properties are not needed anymore.

Right now QOM is all about exposing properties, and having multiple 
interfaces to set them (by picking a different visitor).  But in 
practice most QOM objects have a lifetime that consists of 1) set 
properties 2) flip a switch (realized/complete/open) 3) let the object 
live on its own.  1+2 are a single monitor command or CLI option; during 
3 you access the object through monitor commands, not properties.

> So in summary, it seems to me that the QOM way is more flexible because
> you can get both models out of it. Whether we actually need this
> flexibility I can't say.

I'm thinking there's no need for it, but maybe I'm overly optimistic.

> * Configuration options are described in the QAPI schema. This is mainly
>    for object creation, but runtime modifiable properties are a subset of
>    this.
>
> * Properties are generated for each option. By default, the getter
>    just returns the value from the configuration at creation time, though
>    generation of it can be disabled so that it can be overridden. Also,
>    setters just return an error by default.
>
> * Property setters aren't called for object creation. Instead, the
>    relevant ObjectOptions branch is made available to some init method.
>
> * Runtime modifiable properties (declared as such in the schema) don't
>    get the default setter, so you have to provide an implementation for
>    them.

I wouldn't bother with properties at all in the QAPI schema.  Just do 
the first and third bullet.  Declaring read-only QOM properties is trivial.

> So while this series is doing only one part of the whole solution, that
> the second part is missing doesn't make the first part wrong.

Yeah, I think it's clear that for the long term we're not really 
disagreeing (or perhaps I'm even more radical than you :)).  I'm just 
worried about having yet another incomplete transition.

> One possibly nasty detail to consider there is that we sometimes declare
> the USER_CREATABLE interface in the base class, so ucc->complete is for
> the base class rather than the actually instantiated class. If we only
> instantiate leaf classes (I think this is true), we can move
> USER_CREATABLE there.

You can also use a while loop covering each superclass to decide how to 
dispatch ucc->complete.  I don't care much about these details, they're 
Simple Matter Of Programming. :)

> I also had in mind just passing the whole configuration struct
> (essentially always 'boxed': true), but you're right that individual
> parameters like for commands would be possible. I'm not entirely
> convinced that they would be better (there was a reason why we
> introduced 'boxed': true), but it's an option.

Having 'boxed': true by default would be just an implementation choice, 
nothing to worry about.  (When I said the arguments would be the 
configuration, having a boxed struct as the argument would fit the 
description just as well).

> I was hoping that by converting object-add in this series, and the CLI
> options soon afterwards, it would be very obvious if you forget to
> change the schema because your new property just wouldn't work (at least
> not during creation).

Converting the CLI options is not entirely trivial due to -readconfig 
and friends, so I was expecting that to last until that part of my 6.0 
keyval work goes in.  (It's almost ready for posting BTW, 
https://gitlab.com/bonzini/qemu/-/commit/b59288c86c).

As soon as we have an idea of what we want UserCreatable to look in the 
end, on both the QAPI side and the object implementation side.  That's 
also the part where we have the biggest need to document the schema. 
With that part at least roughly sketched out (no code needed), I'm okay 
with this series going in.

I still don't like the triplication, but as George Michael puts it I 
just gotta have faith---because I must admit, I'm positively surprised 
at the ideas that came out of the discussion.

Paolo