On Fri, May 30, 2014 at 9:17 PM, 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>
Reviewed-by: Peter Crosthwaite <peter.crosthwa...@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
>
>
