Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes

[Andreas Färber]

Am 07.09.2015 um 10:46 schrieb Daniel P. Berrange:
> On Fri, Sep 04, 2015 at 11:38:06PM +0200, Marc-André Lureau wrote:
>> On Wed, Aug 26, 2015 at 2:03 PM, Daniel P. Berrange <address@hidden> wrote:
>>> +ObjectProperty *
>>> +object_class_property_add(ObjectClass *klass,
>>> +                          const char *name,
>>> +                          const char *type,
>>> +                          ObjectPropertyAccessor *get,
>>> +                          ObjectPropertyAccessor *set,
>>> +                          ObjectPropertyRelease *release,
>>> +                          void *opaque,
>>> +                          Error **errp)
>>> +{
>>> +    ObjectProperty *prop;
>>> +    size_t name_len = strlen(name);
>>> +
>>> +    if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) {
>>> +        int i;
>>> +        ObjectProperty *ret;
>>> +        char *name_no_array = g_strdup(name);
>>> +
>>
>> I question the need for dynamic/array property name registered in
>> classes. What would be more useful is an array property instead. It
>> would help to introspect classes for dynamic "children[*]" case.
>> object_property_add_child() could verify/check against the class
>> declaration, and grow the instance properties list (like it does now,
>> but it would be only for instances of children[] items). On
>> introspection of classes, the class "children[*]" property would be
>> visible, but would be hidden when introspecting the instance, and you
>> wouldn't be able to lookup that "array" property.
>>
>> It seems relatively straightforward to deal with the link<> case, by
>> storing the offset of the "child" pointer. This seems fine if limited
>> to a single link<> (it should probably check the prop is not of the
>> name[*] style already), ex:
>> https://gist.github.com/elmarco/905241b683fb9c5f2a08
>>
>> Your patches looks good  to me in general but object_property_del()
>> should be fixed, since the prop find may belong to the class.
> 
> Actually I skipped object_property_del() intentionally. Classes should
> be immutable once defined, so deleting a property from a class would
> not be appropriate.

Agreed, I don't see a use case either.

Can you propose a sentence to amend the commit message with? Then I
would apply this patch to my upcoming qom-next pull, then the unreviewed
rest could go through their respective maintainers.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)