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

Am 01.12.2020 um 22:23 hat Paolo Bonzini geschrieben: We have conditional union variants in existing code, too. Yes, I would like to get user creatable objects to a state that we like and only then start converting other object types. I feel user creatable objects are a nice set of types that aren't so trivial that major problems could go unnoticed, but still a managable enough number that it doesn't matter much if we take one or two extra steps until we find our preferred shape. Once QAPI knows about the class hierarchy you get the internal boilerplate generation almost for free, so I'm not sure why we would pass on it. But I agree it's not critical to have it. I'm not sure, I'm seeing pros and contras. Also, to be honest, I'm quite certain that the blockdev/chardev way is accidental rather than the result of a careful design process. Having to copy every option from the separate configuration into the run-time state is somewhat inconvenient. On the other hand, it ensures that every option is touched in the initialisation function, which probably increases chances that it's checked for validity. The separation might have made the kind of bugs more obvious where property setters just change the configuration without actually applying the updated value. I guess we might end up with a mixed model where configuration is separated into a different struct (the ObjectOptions branches), but still kept around for the lifetime of the object so that qom-get can keep working. Yes, I think object creation doesn't need properties. But no, that doesn't automatically make them useless because of property updates later on. If you want to get fully rid of them, you need a replacement for the latter. But you're right that I would like to make property setters only about runtime changes (i.e. changes after creation) rather than part of the creation itself. When they have only one job to do, it's more likely that they actually implement something working for it. I guess fully getting rid of them and removing qom-set/get in favour of something new could be a future step, but feature removal requires code changes in libvirt etc. so let's try to stay compatible for now. It's hard enough without dealing with changes to external interfaces. True, and you don't need properties for this case. I think you can express everything with the blockdev-reopen style. The client just happens to repeat itself a lot then, and updating the full state atomically might be a bit harder than doing just every property change individually. But there is probably nothing that can be done with individual properties, but not with an all-or-nothing update mechanism. Trivial sounds like it's something the computer should be doing. But there are two more important reasons why I think QAPI should take care of this: The first is that I want to get rid of the duplication that properties have to be defined both in the code and as creation options in QAPI. One place to describe them is enough. Otherwise you might end up with options that can't be queried later because you forgot to manually add the trivial getter property. The second is that the QAPI schema should actually be a full description of the external interface of an object, and properties are part of this. So if we don't describe in it which options are available as (writable) properties after object creation, the schema will tell only half of the story. Would you really feel at home in a QEMU without incomplete transitions? :-) More seriously, I'm certain that we can get the transition completed for user creatable options in a reasonable timeframe. And they form a separate group, as your suggestion to use ucc->complete to implement things shows, so it wouldn't even be an incomplete transition if we stopped there instead of extending the concept to other parts. qdev is a lot larger and it's unrealistic to change all devices at once, so I can see the risk of an incomplete transition there. But if you don't want to take this risk, you can't change anything. Yes, -readconfig is what would be in the way when doing the change on master. But since you had already posted RFC patches a while ago to address this, I considered it solved. I think so far we agree on these steps: 1. This series (mostly for documentation and introspection) 2. -object and HMP object_add (so that we get QAPI's validation, and to make sure that forgetting to update the schema means that the new options just doesn't work) 3. Create a separate 'object' entity in QAPI, generate ucc->complete implementations that call a create function in the C source code with type-specific parameters (like QMP command handlers) What's still open: Should QAPI cover run-time properties, too? Should run-time properties even exist at all in the final state? What is the exact QAPI representation of everything? (The last one includes that I need to have a closer look at how QAPI can best be taught about inheritance.) Kevin