Re: [PATCH v2 00/44] Make qdev static property API usable by any QOM type

[Paolo Bonzini <pbonzini@redhat.com>]

On 09/11/20 12:34, Kevin Wolf wrote:
>> If all properties were like this, it would be okay.  But the API in v2 is
>> the one that is most consistent with QOM in general. Here is how this change
>> would be a loss in term of consistency:
>>
>> - you have the field properties split in two (with the property itself in
>> one place and the allow-set function in a different place), and also you'd
>> have some properties listed as array and some as function calls.
> 
> Why would you have properties defined as function calls for the same
> object that uses the array?

Because some properties would not be field properties, for example.  For 
example, any non-scalar property would need to invoke 
visit_SomeQapiStruct manually and would not be a field property.

> I'm not entirely sure what you mean with allow-set. The only things I
> can find are related to link properties, specifically the check()
> callback of object_class_property_add_link(). If it's this, what would
> be the problem with just adding it to DEFINE_PROP_LINK instead of
> using a separate function call to define link properties?

Eduardo's series is adding allow-set functions to field properties as 
well.  If it's be specified in the function call to 
object_class_add_field_properties, you'd have part of the property 
described in the array and part in the object_class_add_field_properties.

>> - we would have different ways to handle device field properties (with
>> dc->props) compared to object properties.
> 
> You mean dynamic per-object properties, i.e. not class properties?

No, I mean that device properties would be handled as

    dc->props = foo;

while object properties would be handled as

    object_class_add_field_properties(oc, foo, prop_allow_set_always);

There would be two different preferred ways to do field properties in 
qdev vs. non-qdev.

> I think having different ways for different things (class vs. object) is
> better than having different ways for the same things (class in qdev vs.
> class in non-qdev).

Right, but qdev's DEFINE_PROP_STRING would be easy to change to 
something like

- DEFINE_PROP_STRING("name", ...),
+ device_class_add_field_property(dc, "name", PROP_STRING(...));

>> The choice to describe class properties as function calls was made in 2016
>> (commit 16bf7f522a, "qom: Allow properties to be registered against
>> classes", 2016-01-18); so far I don't see that it has been misused.
> 
> This was the obvious incremental step forward at the time because you
> just had to replace one function call with another function call. The
> commit message doesn't explain that not using data was a conscious
> decision. I think it would probably have been out of scope then.
> 
>> Also, I don't think it's any easier to write an introspection code generator
>> with DEFINE_PROP_*.  You would still have to parse the class init function
>> to find the reference to the array (and likewise the TypeInfo struct to find
>> the class init function).
> 
> I don't think we should parse any C code. In my opinion, both
> introspection and the array should eventually be generated by the QAPI
> generator from the schema.

That'd be a good plan, and I'd add generating the property description 
from the doc comment.  (Though there's still the issue of how to add 
non-field properties to the introspection.  Those would be harder to 
move to the QAPI generator).

But at that point the array vs. function call would not change anything 
(if anything the function call would be a tiny bit better), so that's 
another reason to stay with the API that Eduardo has in v2.

Paolo