qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 01/18] qom: add new dynamic property infrastruct


From: Kevin Wolf
Subject: Re: [Qemu-devel] [PATCH 01/18] qom: add new dynamic property infrastructure based on Visitors
Date: Fri, 02 Dec 2011 10:43:16 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20111115 Thunderbird/8.0

Am 02.12.2011 02:08, schrieb Anthony Liguori:
> On 12/01/2011 09:52 AM, Kevin Wolf wrote:
>> Am 30.11.2011 22:03, schrieb Anthony Liguori:
>>> qdev properties are settable only during construction and static to classes.
>>> This isn't flexible enough for QOM.
>>>
>>> This patch introduces a property interface for qdev that provides dynamic
>>> properties that are tied to objects, instead of classes.  These properties 
>>> are
>>> Visitor based instead of string based too.
>>>
>>> Signed-off-by: Anthony Liguori<address@hidden>
>>> ---
>>>   hw/qdev.c |   99 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   hw/qdev.h |  118 
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   qerror.c  |    4 ++
>>>   qerror.h  |    3 ++
>>>   4 files changed, 224 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/hw/qdev.c b/hw/qdev.c
>>> index 106407f..ad2d44f 100644
>>> --- a/hw/qdev.c
>>> +++ b/hw/qdev.c
>>> @@ -390,12 +390,33 @@ void qdev_init_nofail(DeviceState *dev)
>>>       }
>>>   }
>>>
>>> +static void qdev_property_del_all(DeviceState *dev)
>>> +{
>>> +    while (dev->properties) {
>>> +        GSList *i = dev->properties;
>>> +        DeviceProperty *prop = i->data;
>>> +
>>> +        dev->properties = i->next;
>>> +
>>> +        if (prop->release) {
>>> +            prop->release(dev, prop->name, prop->opaque);
>>> +        }
>>> +
>>> +        g_free(prop->name);
>>> +        g_free(prop->type);
>>> +        g_free(prop);
>>> +        g_free(i);
>>> +    }
>>> +}
>>> +
>>>   /* Unlink device from bus and free the structure.  */
>>>   void qdev_free(DeviceState *dev)
>>>   {
>>>       BusState *bus;
>>>       Property *prop;
>>>
>>> +    qdev_property_del_all(dev);
>>> +
>>>       if (dev->state == DEV_STATE_INITIALIZED) {
>>>           while (dev->num_child_bus) {
>>>               bus = QLIST_FIRST(&dev->child_bus);
>>> @@ -962,3 +983,81 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
>>>
>>>       return strdup(path);
>>>   }
>>> +
>>> +void qdev_property_add(DeviceState *dev, const char *name, const char 
>>> *type,
>>> +                       DevicePropertyEtter *get, DevicePropertyEtter *set,
>>> +                       DevicePropertyRelease *release, void *opaque,
>>> +                       Error **errp)
>>
>> How about letting the caller pass in a DeviceProperty for improved
>> readability and usability? Instead of memorizing the order of currently
>> eight parameters (could probably become more in the future) you can use
>> proper C99 initializers then.
> 
> Yeah, instead of taking a void *opaque, it could then just take the 
> DeviceProperty and use container_of adding a good bit more type safety.  I 
> like 
> it, thanks for the suggestion.
> 
>>
>>> @@ -45,6 +82,7 @@ struct DeviceState {
>>>       QTAILQ_ENTRY(DeviceState) sibling;
>>>       int instance_id_alias;
>>>       int alias_required_for_version;
>>> +    GSList *properties;
>>>   };
>>
>> Why GSList instead of qemu-queue.h macros that would provide type safety?
> 
> You're clearly thwarting my attempts at slowly introducing GSList as a 
> replacement for qemu-queue ;-)
> 
> I really dislike qemu-queue.  I think it's a whole lot more difficult to use 
> in 
> practice.  The glib data structures are much more rich than qemu-queue.

qemu-queue.h is type safe, GSList is not. IMO that's a show stopper and
I can't understand why we even need to talk about it.

If you want to convince me of the opposite, it certainly needs more than
vague "easier to use" hand waving.

Kevin



reply via email to

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