qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get of


From: Paolo Bonzini
Subject: Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
Date: Wed, 28 Oct 2020 16:22:40 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1

On 23/10/20 17:33, Igor Mammedov wrote:
> On Wed, 21 Oct 2020 09:30:41 -0400
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
>> On Wed, Oct 21, 2020 at 02:24:08PM +0200, Igor Mammedov wrote:
>>> On Fri,  9 Oct 2020 12:01:13 -0400
>>> Eduardo Habkost <ehabkost@redhat.com> wrote:
>>>   
>>>> The existing object_class_property_add_uint*_ptr() functions are
>>>> not very useful, because they need a pointer to the property
>>>> value, which can't really be provided before the object is
>>>> created.
>>>>
>>>> Replace the pointer parameter in those functions with a
>>>> `ptrdiff_t offset` parameter.
>>>>
>>>> Include a uint8 class property in check-qom-proplist unit tests,
>>>> to ensure the feature is working.  
>>>
>>>
>>> Not sure I like approach, it's reinventing qdev pointer properties in QOM 
>>> form.  
>>
>> Yes, and that's on purpose.  If we want to eventually merge the
>> two competing APIs into a single one, we need to make them
>> converge.
>>
>>> I had an impression that Paolo wanted qdev pointer properties be gone
>>> and replaced by something like link properties.  
>>
>> This is completely unrelated to qdev pointer properties and link
>> properties.  The properties that use object_property_add_uint*_ptr()
>> today are not qdev pointer properties and will never be link
>> properties.  They are just integer properties.

I think this series a step in the right direction, but please take more
"inspiration" from link properties, which are done right.  In
particular, properties should have an optional check function and be
read-only unless the check function is there.

You can make the check function take an uint64_t for simplicity, so that
all the check functions for uint properties have the same prototype.
For example a single "property_check_uint_allow" function can allow
setting the property (which is almost always wrong, but an easy cop out
for this series).

Paolo




reply via email to

[Prev in Thread] Current Thread [Next in Thread]