Re: [Qemu-devel] [RFC 1/5] qdev: add child alias properties

[Peter Crosthwaite]

On Wed, May 14, 2014 at 2:17 PM, Andreas Färber <address@hidden> wrote:
> Am 14.05.2014 16:13, schrieb Peter Crosthwaite:
>> On Wed, Mar 19, 2014 at 3:48 PM, Stefan Hajnoczi <address@hidden> wrote:
>>> Child alias properties allow a parent object to expose child object
>>> properties.  This makes it possible for a parent object to forward all
>>> or a subset of a child's properties.
>>>
>>
>> So this sounds useful to my MemoryRegion and GPIO QOMification plan.
>> One of the missing pieces is how do you set a child property when you
>> only have a ref to the parent which I guess is a big motivator here.
>> The idea is, memory region properties can be set by the parent device
>> that created them (Seems similar to what you are doing with blk stuffs
>> here really).
>>
>> This implementation however seems to assume a 1:1 mapping between the
>> parent and child property namespaces limiting its use in cases where
>> multiple child objects of the same type want to expose on the parent
>> level. Is it somehow possible to change the name of the property as
>> exposed by the parent ...
>>
>>> Currently we achieve similar behavior by duplicating property
>>> definitions and copying the fields around.  It turns out virtio has
>>> several double-frees since we didn't get the memory management right.
>>>
>>> Using child alias properties it will be much easier for a parent object
>>> to set a child's properties without fragile memory management issues
>>> since the actual field only exists once in the child object.
>>>
>>> Signed-off-by: Stefan Hajnoczi <address@hidden>
>>> ---
>>>  hw/core/qdev-properties.c    | 28 ++++++++++++++++++++++++++++
>>>  include/hw/qdev-properties.h | 28 ++++++++++++++++++++++++++++
>>>  2 files changed, 56 insertions(+)
>>>
>>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>>> index 77d0c66..e62a4fd 100644
>>> --- a/hw/core/qdev-properties.c
>>> +++ b/hw/core/qdev-properties.c
>>> @@ -1002,3 +1002,31 @@ PropertyInfo qdev_prop_size = {
>>>      .get = get_size,
>>>      .set = set_size,
>>>  };
>>> +
>>> +/* --- alias that forwards to a child object's property --- */
>>> +
>>> +static void get_child_alias(Object *obj, Visitor *v, void *opaque,
>>> +                            const char *name, Error **errp)
>>> +{
>>> +    DeviceState *dev = DEVICE(obj);
>>> +    Property *prop = opaque;
>>> +    Object *child = OBJECT(qdev_get_prop_ptr(dev, prop));
>>> +
>>> +    object_property_get(child, v, name, errp);
>>> +}
>>> +
>>> +static void set_child_alias(Object *obj, Visitor *v, void *opaque,
>>> +                            const char *name, Error **errp)
>>> +{
>>> +    DeviceState *dev = DEVICE(obj);
>>> +    Property *prop = opaque;
>>> +    Object *child = OBJECT(qdev_get_prop_ptr(dev, prop));
>>> +
>>> +    object_property_set(child, v, name, errp);
>>> +}
>>> +
>>> +PropertyInfo qdev_prop_child_alias = {
>>> +    .name  = "ChildAlias",
>>> +    .get = get_child_alias,
>>> +    .set = set_child_alias,
>>> +};
>>> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
>>> index 3c000ee..5ab1cac 100644
>>> --- a/include/hw/qdev-properties.h
>>> +++ b/include/hw/qdev-properties.h
>>> @@ -13,6 +13,7 @@ extern PropertyInfo qdev_prop_uint32;
>>>  extern PropertyInfo qdev_prop_int32;
>>>  extern PropertyInfo qdev_prop_uint64;
>>>  extern PropertyInfo qdev_prop_size;
>>> +extern PropertyInfo qdev_prop_child_alias;
>>>  extern PropertyInfo qdev_prop_string;
>>>  extern PropertyInfo qdev_prop_chr;
>>>  extern PropertyInfo qdev_prop_ptr;
>>> @@ -61,6 +62,33 @@ extern PropertyInfo qdev_prop_arraylen;
>>>          .defval    = (bool)_defval,                              \
>>>          }
>>>
>>> +/**
>>> + * DEFINE_PROP_CHILD_ALIAS:
>>> + * @_name: property name
>>> + * @_state: name of the device state structure type
>>> + * @_field: name of field in @_state, must be Object subclass
>>> + *
>>> + * Define device properties that alias a child object's property.  For 
>>> example,
>>> + * use the following to forward the 'baz' property where Foo embeds a Bar
>>
>> ... such that the parent renames the property to "bar-baz" to clarify
>> whose "baz" is being forwarded?
>
> Didn't you claim just yesterday that bar.baz=foo or similar was already
> working today?
>

Yeh but I had to patch QOM to make that work. Thats newly invented
syntax. This would be a possible alternative. Just shopping for
options here :)

> I think that in your case adding properties to the parent is wrong.
>

Ok. I'll post that QOM patch along with the rest.

Regards,
Peter

> virtio on the other hand needs this because devices live both on their
> own on a virtio-mmio bus and get created as PCI/CCW devices by the user.
>
> Regards,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>