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

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. What probably needs a solution is that an explicit 'if' in the schema would duplicate information that is actually defined in the build system configuration. So we would somehow need a common source for both. As I said, I don't know the machine type code well, so I'm not really sure about the best way there. But maybe QAPI isn't for version-specific machine types. I think they never have additional properties, but only different init functions. So their description in QAPI would be essentially empty anyway. 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? 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. 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. Or do you mean between create-time options and properties that can be written at runtime? In that case, yes, I think that would be very desirable because mashing them together has resulted in lots of bugs. How do you define properties, i.e. at which point would they stop existing and what would be a non-property alternative? Outside of QOM, usually have a query-* command that returns the whole state rather than a single property, and possibly individual QMP commands to update the state. blockdev-reopen takes a new configuration (the same structure as blockdev-add) and then applies that whole new configuration to the existing block node. I guess this would be an alternative, though it is somewhat inconvenient because you have to repeat all the options that you don't want to change. The blockdev-reopen way has one important advantage: You can change multiple options at the same time when there are dependencies between the options so that individually changing each option would be invalid, but together it's possible. throttle-group needs this in QOM, and it's easily implemented: It just has a single 'limits' property, which is a struct and is always updated as a single unit. 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. But there is another reason why I didn't intend to remove properties entirely, which is that things are a lot easier as long as you don't have to break external interfaces, which qom-set/get/list are. The way I imagined it so far is roughly like this: * 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. This is an interesting comparison. I never thought of likening objects to commands, but there is some truth to it. I certainly did envision objects as separate type of QAPI entities, which I guess is why I didn't really like Dan's suggestion to make objects an extension of structs, but I couldn't really express why. I think your comparison gives a good answer. However, this series is still not doing the wrong thing: If you think about commands, they don't exist without an arguments struct. Similarly, objects don't exist without a properties struct. 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. I was thinking of using .instance_init because it's more generic, but using ucc->complete (and in the long run realize for qdev) might require less modifications of non-user-creatable objects. 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. 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. Hm, it's a good point. 1:1 properties would actually end up in generated code anyway, so simplifying them is maybe not that useful in the end. It would probably result in nicer generated code, but that's it. 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). But I can add comments if you think this is still worthwhile. And two more patches of the same type here: https://patchew.org/QEMU/20201117163045.307451-1-kwolf@redhat.com/ Yes. object-add is already covered by this series and -object should be easy, so we can have this more or less now. Getting the design right for QOM code generation will certainly take more time, discussion and also experimentation. Kevin