[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 4/7] virtio-blk: use aliases instead of dupli
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v2 4/7] virtio-blk: use aliases instead of duplicate qdev properties |
Date: |
Fri, 30 May 2014 11:01:07 +0200 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Thu, May 29, 2014 at 07:03:42PM +1000, Peter Crosthwaite wrote:
> On Fri, May 23, 2014 at 1:40 AM, Stefan Hajnoczi <address@hidden> 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 <address@hidden>
> > ---
> > 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
- [Qemu-devel] [PATCH v2 0/7] virtio-blk: use alias properties in transport devices, Stefan Hajnoczi, 2014/05/22
- [Qemu-devel] [PATCH v2 2/7] virtio-blk: avoid qdev property definition duplication, Stefan Hajnoczi, 2014/05/22
- [Qemu-devel] [PATCH v2 1/7] qom: add object_property_add_alias(), Stefan Hajnoczi, 2014/05/22
- [Qemu-devel] [PATCH v2 4/7] virtio-blk: use aliases instead of duplicate qdev properties, Stefan Hajnoczi, 2014/05/22
- [Qemu-devel] [PATCH v2 3/7] virtio-blk: move x-data-plane qdev property to virtio-blk.h, Stefan Hajnoczi, 2014/05/22
- [Qemu-devel] [PATCH v2 5/7] virtio-blk: drop virtio_blk_set_conf(), Stefan Hajnoczi, 2014/05/22
- [Qemu-devel] [PATCH v2 6/7] virtio: fix virtio-blk child refcount in transports, Stefan Hajnoczi, 2014/05/22
- [Qemu-devel] [PATCH v2 7/7] virtio-blk: move qdev properties into virtio-blk.c, Stefan Hajnoczi, 2014/05/22