On Thu, May 29, 2014 at 07:03:42PM +1000, Peter Crosthwaite wrote: > On Fri, May 23, 2014 at 1:40 AM, Stefan Hajnoczi <stefa...@redhat.com> wrote: > > virtio-blk-pci, virtio-blk-s390, and virtio-blk-ccw all duplicate the > > qdev properties of their VirtIOBlock child. This approach does not work > > well with string or pointer properties since we must be careful about > > leaking or double-freeing them. > > > > Use the QOM alias property to forward property accesses to the > > VirtIOBlock child. This way no duplication is necessary. > > > > Remember to stop calling virtio_blk_set_conf() so that we don't clobber > > the values already set on the VirtIOBlock instance. > > > > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> > > --- > > v2: > > * Add qdev_alias_all_properties() instead of virtio-blk-specific > > function [Paolo] > > --- > > hw/core/qdev.c | 19 +++++++++++++++++++ > > The commit message proper makes no reference to the new qedev core > feature for alias properties. That said, I think the qdev change would > work nicely as a separate patch and this ones commit message would > stand nicely as-is without the qdev patch content.
Good point, I'll split qdev_alias_all_properties() into a separate patch. > > hw/s390x/s390-virtio-bus.c | 9 +-------- > > hw/s390x/s390-virtio-bus.h | 1 - > > hw/s390x/virtio-ccw.c | 3 +-- > > hw/s390x/virtio-ccw.h | 1 - > > hw/virtio/virtio-pci.c | 3 +-- > > hw/virtio/virtio-pci.h | 1 - > > include/hw/qdev-properties.h | 2 ++ > > 8 files changed, 24 insertions(+), 15 deletions(-) > > > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > > index 936eae6..e41f5b8 100644 > > --- a/hw/core/qdev.c > > +++ b/hw/core/qdev.c > > @@ -724,6 +724,25 @@ void qdev_property_add_static(DeviceState *dev, > > Property *prop, > > } > > } > > > > +/* @qdev_alias_all_properties - Add alias properties to the source object > > for > > + * all qdev properties on the target DeviceState. > > + */ > > +void qdev_alias_all_properties(DeviceState *target, Object *source) > > +{ > > + ObjectClass *class; > > + Property *prop; > > + > > + class = object_get_class(OBJECT(target)); > > + do { > > + for (prop = DEVICE_CLASS(class)->props; prop && prop->name; > > prop++) { > > Don't de-reference an inline QOM cast macro. DEVICE_CLASS(class) will > need its own local variable. Andreas, can we update QOM conventions > with this finer point? I think we already have "Avoid using cast > macros other than OBJECT() inline" but I remember a thread a while > back along the lines of no de-referencing being the main intention of > this rule. Sure, this is easy to change. Could you explain the reasoning behind it though? By the way, this is copied from hw/core/qdev.c:device_initfn() where we also dereference inline. Stefan