| Message ID | 1401448669-4655-6-git-send-email-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, May 30, 2014 at 9:17 PM, Stefan Hajnoczi <stefanha@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 <stefanha@redhat.com> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > v3: > * Split qdev_alias_all_properties() into its own patch [Peter Crosthwaite] > > v2: > * Add qdev_alias_all_properties() instead of virtio-blk-specific > function [Paolo] > --- > 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 - > 6 files changed, 3 insertions(+), 15 deletions(-) > > diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c > index 9c71afa..041d4e2 100644 > --- a/hw/s390x/s390-virtio-bus.c > +++ b/hw/s390x/s390-virtio-bus.c > @@ -179,7 +179,6 @@ static int s390_virtio_blk_init(VirtIOS390Device *s390_dev) > { > VirtIOBlkS390 *dev = VIRTIO_BLK_S390(s390_dev); > DeviceState *vdev = DEVICE(&dev->vdev); > - virtio_blk_set_conf(vdev, &(dev->blk)); > qdev_set_parent_bus(vdev, BUS(&s390_dev->bus)); > if (qdev_init(vdev) < 0) { > return -1; > @@ -192,6 +191,7 @@ static void s390_virtio_blk_instance_init(Object *obj) > VirtIOBlkS390 *dev = VIRTIO_BLK_S390(obj); > object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BLK); > object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL); > + qdev_alias_all_properties(DEVICE(&dev->vdev), obj); > } > > static int s390_virtio_serial_init(VirtIOS390Device *s390_dev) > @@ -526,18 +526,11 @@ static const TypeInfo s390_virtio_net = { > .class_init = s390_virtio_net_class_init, > }; > > -static Property s390_virtio_blk_properties[] = { > - DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkS390, blk), > - DEFINE_PROP_END_OF_LIST(), > -}; > - > static void s390_virtio_blk_class_init(ObjectClass *klass, void *data) > { > - DeviceClass *dc = DEVICE_CLASS(klass); > VirtIOS390DeviceClass *k = VIRTIO_S390_DEVICE_CLASS(klass); > > k->init = s390_virtio_blk_init; > - dc->props = s390_virtio_blk_properties; > } > > static const TypeInfo s390_virtio_blk = { > diff --git a/hw/s390x/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h > index ac81bd8..ffd0df7 100644 > --- a/hw/s390x/s390-virtio-bus.h > +++ b/hw/s390x/s390-virtio-bus.h > @@ -124,7 +124,6 @@ void s390_virtio_reset_idx(VirtIOS390Device *dev); > typedef struct VirtIOBlkS390 { > VirtIOS390Device parent_obj; > VirtIOBlock vdev; > - VirtIOBlkConf blk; > } VirtIOBlkS390; > > /* virtio-scsi-s390 */ > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index f3f53dc..0cc8132 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -800,7 +800,6 @@ static int virtio_ccw_blk_init(VirtioCcwDevice *ccw_dev) > { > VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(ccw_dev); > DeviceState *vdev = DEVICE(&dev->vdev); > - virtio_blk_set_conf(vdev, &(dev->blk)); > qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus)); > if (qdev_init(vdev) < 0) { > return -1; > @@ -814,6 +813,7 @@ static void virtio_ccw_blk_instance_init(Object *obj) > VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(obj); > object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BLK); > object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL); > + qdev_alias_all_properties(DEVICE(&dev->vdev), obj); > } > > static int virtio_ccw_serial_init(VirtioCcwDevice *ccw_dev) > @@ -1309,7 +1309,6 @@ static const TypeInfo virtio_ccw_net = { > static Property virtio_ccw_blk_properties[] = { > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), > DEFINE_VIRTIO_BLK_FEATURES(VirtioCcwDevice, host_features[0]), > - DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkCcw, blk), > DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags, > VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true), > DEFINE_PROP_END_OF_LIST(), > diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h > index b8b8a8a..5a1f16e 100644 > --- a/hw/s390x/virtio-ccw.h > +++ b/hw/s390x/virtio-ccw.h > @@ -144,7 +144,6 @@ typedef struct VHostSCSICcw { > typedef struct VirtIOBlkCcw { > VirtioCcwDevice parent_obj; > VirtIOBlock vdev; > - VirtIOBlkConf blk; > } VirtIOBlkCcw; > > /* virtio-balloon-ccw */ > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 0751a1e..3bb782f 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1068,7 +1068,6 @@ static Property virtio_blk_pci_properties[] = { > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), > DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features), > - DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkPCI, blk), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -1076,7 +1075,6 @@ static int virtio_blk_pci_init(VirtIOPCIProxy *vpci_dev) > { > VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(vpci_dev); > DeviceState *vdev = DEVICE(&dev->vdev); > - virtio_blk_set_conf(vdev, &(dev->blk)); > qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); > if (qdev_init(vdev) < 0) { > return -1; > @@ -1104,6 +1102,7 @@ static void virtio_blk_pci_instance_init(Object *obj) > VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(obj); > object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BLK); > object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL); > + qdev_alias_all_properties(DEVICE(&dev->vdev), obj); > } > > static const TypeInfo virtio_blk_pci_info = { > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h > index dc332ae..1cea157 100644 > --- a/hw/virtio/virtio-pci.h > +++ b/hw/virtio/virtio-pci.h > @@ -131,7 +131,6 @@ struct VHostSCSIPCI { > struct VirtIOBlkPCI { > VirtIOPCIProxy parent_obj; > VirtIOBlock vdev; > - VirtIOBlkConf blk; > }; > > /* > -- > 1.9.3 > >
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index 9c71afa..041d4e2 100644 --- a/hw/s390x/s390-virtio-bus.c +++ b/hw/s390x/s390-virtio-bus.c @@ -179,7 +179,6 @@ static int s390_virtio_blk_init(VirtIOS390Device *s390_dev) { VirtIOBlkS390 *dev = VIRTIO_BLK_S390(s390_dev); DeviceState *vdev = DEVICE(&dev->vdev); - virtio_blk_set_conf(vdev, &(dev->blk)); qdev_set_parent_bus(vdev, BUS(&s390_dev->bus)); if (qdev_init(vdev) < 0) { return -1; @@ -192,6 +191,7 @@ static void s390_virtio_blk_instance_init(Object *obj) VirtIOBlkS390 *dev = VIRTIO_BLK_S390(obj); object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BLK); object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL); + qdev_alias_all_properties(DEVICE(&dev->vdev), obj); } static int s390_virtio_serial_init(VirtIOS390Device *s390_dev) @@ -526,18 +526,11 @@ static const TypeInfo s390_virtio_net = { .class_init = s390_virtio_net_class_init, }; -static Property s390_virtio_blk_properties[] = { - DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkS390, blk), - DEFINE_PROP_END_OF_LIST(), -}; - static void s390_virtio_blk_class_init(ObjectClass *klass, void *data) { - DeviceClass *dc = DEVICE_CLASS(klass); VirtIOS390DeviceClass *k = VIRTIO_S390_DEVICE_CLASS(klass); k->init = s390_virtio_blk_init; - dc->props = s390_virtio_blk_properties; } static const TypeInfo s390_virtio_blk = { diff --git a/hw/s390x/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h index ac81bd8..ffd0df7 100644 --- a/hw/s390x/s390-virtio-bus.h +++ b/hw/s390x/s390-virtio-bus.h @@ -124,7 +124,6 @@ void s390_virtio_reset_idx(VirtIOS390Device *dev); typedef struct VirtIOBlkS390 { VirtIOS390Device parent_obj; VirtIOBlock vdev; - VirtIOBlkConf blk; } VirtIOBlkS390; /* virtio-scsi-s390 */ diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index f3f53dc..0cc8132 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -800,7 +800,6 @@ static int virtio_ccw_blk_init(VirtioCcwDevice *ccw_dev) { VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(ccw_dev); DeviceState *vdev = DEVICE(&dev->vdev); - virtio_blk_set_conf(vdev, &(dev->blk)); qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus)); if (qdev_init(vdev) < 0) { return -1; @@ -814,6 +813,7 @@ static void virtio_ccw_blk_instance_init(Object *obj) VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(obj); object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BLK); object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL); + qdev_alias_all_properties(DEVICE(&dev->vdev), obj); } static int virtio_ccw_serial_init(VirtioCcwDevice *ccw_dev) @@ -1309,7 +1309,6 @@ static const TypeInfo virtio_ccw_net = { static Property virtio_ccw_blk_properties[] = { DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), DEFINE_VIRTIO_BLK_FEATURES(VirtioCcwDevice, host_features[0]), - DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkCcw, blk), DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags, VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true), DEFINE_PROP_END_OF_LIST(), diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h index b8b8a8a..5a1f16e 100644 --- a/hw/s390x/virtio-ccw.h +++ b/hw/s390x/virtio-ccw.h @@ -144,7 +144,6 @@ typedef struct VHostSCSICcw { typedef struct VirtIOBlkCcw { VirtioCcwDevice parent_obj; VirtIOBlock vdev; - VirtIOBlkConf blk; } VirtIOBlkCcw; /* virtio-balloon-ccw */ diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 0751a1e..3bb782f 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1068,7 +1068,6 @@ static Property virtio_blk_pci_properties[] = { VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features), - DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkPCI, blk), DEFINE_PROP_END_OF_LIST(), }; @@ -1076,7 +1075,6 @@ static int virtio_blk_pci_init(VirtIOPCIProxy *vpci_dev) { VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(vpci_dev); DeviceState *vdev = DEVICE(&dev->vdev); - virtio_blk_set_conf(vdev, &(dev->blk)); qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); if (qdev_init(vdev) < 0) { return -1; @@ -1104,6 +1102,7 @@ static void virtio_blk_pci_instance_init(Object *obj) VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(obj); object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BLK); object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL); + qdev_alias_all_properties(DEVICE(&dev->vdev), obj); } static const TypeInfo virtio_blk_pci_info = { diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index dc332ae..1cea157 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -131,7 +131,6 @@ struct VHostSCSIPCI { struct VirtIOBlkPCI { VirtIOPCIProxy parent_obj; VirtIOBlock vdev; - VirtIOBlkConf blk; }; /*
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 <stefanha@redhat.com> --- v3: * Split qdev_alias_all_properties() into its own patch [Peter Crosthwaite] v2: * Add qdev_alias_all_properties() instead of virtio-blk-specific function [Paolo] --- 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 - 6 files changed, 3 insertions(+), 15 deletions(-)