[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes |
Date: |
Mon, 7 Sep 2015 15:11:56 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
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)
- Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes, (continued)
- Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes, Andreas Färber, 2015/09/03
- Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes, Markus Armbruster, 2015/09/03
- Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes, Daniel P. Berrange, 2015/09/03
- Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes, Andreas Färber, 2015/09/03
- Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes, Daniel P. Berrange, 2015/09/03
- Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes, Markus Armbruster, 2015/09/04
- Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes, Paolo Bonzini, 2015/09/07
Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes, Daniel P. Berrange, 2015/09/11
Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes, Marc-André Lureau, 2015/09/04