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

[Andreas Färber]

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?

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

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