| Message ID | 1400703772-3324-5-git-send-email-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
Il 21/05/2014 22:22, Stefan Hajnoczi ha scritto: > 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. Which properties are _not_ being added? This is probably needed for all other virtio devices so a generic solution would be nice. Paolo > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > hw/block/virtio-blk.c | 24 ++++++++++++++++++++++++ > 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/virtio/virtio-blk.h | 2 ++ > 8 files changed, 29 insertions(+), 15 deletions(-) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 8a568e5..52cd3c9 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -754,6 +754,30 @@ static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp) > virtio_cleanup(vdev); > } > > +void virtio_blk_add_child_aliases(Object *obj, VirtIOBlock *s) > +{ > + const char *aliases[] = {"drive", "logical_block_size", > + "physical_block_size", "min_io_size", "opt_io_size", "bootindex", > + "discard_granularity", "cyls", "heads", "secs", "serial", "config-wce", > + "x-iothread", > +#ifdef __linux__ > + "scsi", > +#endif > +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE > + "x-data-plane", > +#endif > + NULL > + }; > + Object *child = OBJECT(s); > + int i; > + > + for (i = 0; aliases[i]; i++) { > + object_property_add_alias(obj, aliases[i], > + child, aliases[i], > + &error_abort); > + } > +} > + > static Property virtio_blk_properties[] = { > DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlock, blk), > DEFINE_PROP_END_OF_LIST(), > diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c > index 9c71afa..e7efcdb 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); > + virtio_blk_add_child_aliases(obj, &dev->vdev); > } > > 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 082bb42..9e98713 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -725,7 +725,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; > @@ -739,6 +738,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); > + virtio_blk_add_child_aliases(obj, &dev->vdev); > } > > static int virtio_ccw_serial_init(VirtioCcwDevice *ccw_dev) > @@ -1126,7 +1126,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 4393e44..7eea6b9 100644 > --- a/hw/s390x/virtio-ccw.h > +++ b/hw/s390x/virtio-ccw.h > @@ -134,7 +134,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..c13fb09 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); > + virtio_blk_add_child_aliases(obj, &dev->vdev); > } > > 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; > }; > > /* > diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h > index 78e7f81..bea540c 100644 > --- a/include/hw/virtio/virtio-blk.h > +++ b/include/hw/virtio/virtio-blk.h > @@ -161,4 +161,6 @@ typedef struct VirtIOBlock { > > void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk); > > +void virtio_blk_add_child_aliases(Object *obj, VirtIOBlock *s); > + > #endif >
On Thu, May 22, 2014 at 12:04 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 21/05/2014 22:22, Stefan Hajnoczi ha scritto: > >> 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. > > > Which properties are _not_ being added? This is probably needed for all > other virtio devices so a generic solution would be nice. I think your idea is something like: def qdev_add_alias_properties(obj, dev): for qdev_prop in dev: object_property_add_alias(obj, qdev_prop.name, dev, qdev_prop.name) That way we don't explicitly list all properties and duplicate code when fixing up net, rng, serial, scsi, etc. I took a quick look at net, rng, and serial. This approach should work because the VirtIODevice qdev properties need to be exposed wholesale on the transport device. If that's what you're hinting at, I'll try it in v2. Stefan
Am 22.05.2014 00:04, schrieb Paolo Bonzini: > Il 21/05/2014 22:22, Stefan Hajnoczi ha scritto: >> 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. > > Which properties are _not_ being added? This is probably needed for all > other virtio devices so a generic solution would be nice. "type", "realized" and the child<> property for VirtIODevice come to mind, possibly one or two more. If we follow a generic scheme, we could add an .instance_post_init hook for VirtIOPCIProxy iterating over all properties and blacklisting some. Regards, Andreas
On Thu, May 22, 2014 at 12:18 PM, Andreas Färber <afaerber@suse.de> wrote: > Am 22.05.2014 00:04, schrieb Paolo Bonzini: >> Il 21/05/2014 22:22, Stefan Hajnoczi ha scritto: >>> 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. >> >> Which properties are _not_ being added? This is probably needed for all >> other virtio devices so a generic solution would be nice. > > "type", "realized" and the child<> property for VirtIODevice come to > mind, possibly one or two more. > > If we follow a generic scheme, we could add an .instance_post_init hook > for VirtIOPCIProxy iterating over all properties and blacklisting some. I think the trick is to alias all the qdev properties, not the QOM ones. That way we get all the explicitly declared properties and none of the implicit ones. Stefan
Am 22.05.2014 12:24, schrieb Stefan Hajnoczi: > On Thu, May 22, 2014 at 12:18 PM, Andreas Färber <afaerber@suse.de> wrote: >> Am 22.05.2014 00:04, schrieb Paolo Bonzini: >>> Il 21/05/2014 22:22, Stefan Hajnoczi ha scritto: >>>> 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. >>> >>> Which properties are _not_ being added? This is probably needed for all >>> other virtio devices so a generic solution would be nice. >> >> "type", "realized" and the child<> property for VirtIODevice come to >> mind, possibly one or two more. >> >> If we follow a generic scheme, we could add an .instance_post_init hook >> for VirtIOPCIProxy iterating over all properties and blacklisting some. > > I think the trick is to alias all the qdev properties, not the QOM > ones. That way we get all the explicitly declared properties and none > of the implicit ones. I wouldn't oppose that, but you then need to iterate over parent classes until you hit VirtioDeviceClass (or DeviceClass?), to avoid properties falling through the cracks. I just figured it easier and in line with your QMP patch to avoid distinguishing them in new code. But a quick solution is more important than futureproofness here, so I'll take or ack whatever works here. Andreas
Il 22/05/2014 12:17, Stefan Hajnoczi ha scritto: > I took a quick look at net, rng, and serial. This approach should > work because the VirtIODevice qdev properties need to be exposed > wholesale on the transport device. > > If that's what you're hinting at, I'll try it in v2. Yes, exactly. scsi too. Paolo
On Thu, May 22, 2014 at 8:32 PM, Andreas Färber <afaerber@suse.de> wrote: > Am 22.05.2014 12:24, schrieb Stefan Hajnoczi: >> On Thu, May 22, 2014 at 12:18 PM, Andreas Färber <afaerber@suse.de> wrote: >>> Am 22.05.2014 00:04, schrieb Paolo Bonzini: >>>> Il 21/05/2014 22:22, Stefan Hajnoczi ha scritto: >>>>> 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. >>>> >>>> Which properties are _not_ being added? This is probably needed for all >>>> other virtio devices so a generic solution would be nice. >>> >>> "type", "realized" and the child<> property for VirtIODevice come to >>> mind, possibly one or two more. >>> link<> ? >>> If we follow a generic scheme, we could add an .instance_post_init hook >>> for VirtIOPCIProxy iterating over all properties and blacklisting some. >> >> I think the trick is to alias all the qdev properties, not the QOM >> ones. That way we get all the explicitly declared properties and none >> of the implicit ones. > > I wouldn't oppose that, but you then need to iterate over parent classes > until you hit VirtioDeviceClass (or DeviceClass?), IMO DeviceClass. Makes this whole alias concept non-virtio specific and it will be more consistent with the existing qdev multi-level property system. Add it to the qdev core for all to use: qdev_alias_all_properties(DeviceState *target, Object *forwarder); Or some such. Regards, Peter to avoid properties > falling through the cracks. > > I just figured it easier and in line with your QMP patch to avoid > distinguishing them in new code. But a quick solution is more important > than futureproofness here, so I'll take or ack whatever works here. > > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 8a568e5..52cd3c9 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -754,6 +754,30 @@ static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp) virtio_cleanup(vdev); } +void virtio_blk_add_child_aliases(Object *obj, VirtIOBlock *s) +{ + const char *aliases[] = {"drive", "logical_block_size", + "physical_block_size", "min_io_size", "opt_io_size", "bootindex", + "discard_granularity", "cyls", "heads", "secs", "serial", "config-wce", + "x-iothread", +#ifdef __linux__ + "scsi", +#endif +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE + "x-data-plane", +#endif + NULL + }; + Object *child = OBJECT(s); + int i; + + for (i = 0; aliases[i]; i++) { + object_property_add_alias(obj, aliases[i], + child, aliases[i], + &error_abort); + } +} + static Property virtio_blk_properties[] = { DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlock, blk), DEFINE_PROP_END_OF_LIST(), diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index 9c71afa..e7efcdb 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); + virtio_blk_add_child_aliases(obj, &dev->vdev); } 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 082bb42..9e98713 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -725,7 +725,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; @@ -739,6 +738,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); + virtio_blk_add_child_aliases(obj, &dev->vdev); } static int virtio_ccw_serial_init(VirtioCcwDevice *ccw_dev) @@ -1126,7 +1126,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 4393e44..7eea6b9 100644 --- a/hw/s390x/virtio-ccw.h +++ b/hw/s390x/virtio-ccw.h @@ -134,7 +134,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..c13fb09 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); + virtio_blk_add_child_aliases(obj, &dev->vdev); } 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; }; /* diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 78e7f81..bea540c 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -161,4 +161,6 @@ typedef struct VirtIOBlock { void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk); +void virtio_blk_add_child_aliases(Object *obj, VirtIOBlock *s); + #endif
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> --- hw/block/virtio-blk.c | 24 ++++++++++++++++++++++++ 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/virtio/virtio-blk.h | 2 ++ 8 files changed, 29 insertions(+), 15 deletions(-)