Message ID | 1400773210-7584-7-git-send-email-stefanha@redhat.com |
---|---|
State | New |
Headers | show |
On Fri, May 23, 2014 at 1:40 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > object_initialize() leaves the object with a refcount of 1. > object_property_add_child() adds its own reference which is dropped > again when the property is deleted. > > The upshot of this is that we always have a refcount >= 1. Upon hot > unplug the virtio-blk child is not finalized! > Doesn't this suggest that hot unplug is what's broken? My understanding (which is fresh and not 100% yet) is the original == 1 refcount should be dropped at object deletion time which is this sense would be unplug time. This would mean that hot-unplug should explicitly object_unref the object (should the intention of hot-unplug be to always finalise the device?). Regards, Peter > Drop our reference after the child property has been added to the > parent. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > hw/s390x/s390-virtio-bus.c | 1 + > hw/s390x/virtio-ccw.c | 1 + > hw/virtio/virtio-pci.c | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c > index 041d4e2..bf92cb6 100644 > --- a/hw/s390x/s390-virtio-bus.c > +++ b/hw/s390x/s390-virtio-bus.c > @@ -191,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); > + object_unref(OBJECT(&dev->vdev)); > qdev_alias_all_properties(DEVICE(&dev->vdev), obj); > } > > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index f33293d..eb8427b 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -738,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); > + object_unref(OBJECT(&dev->vdev)); > qdev_alias_all_properties(DEVICE(&dev->vdev), obj); > } > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 3bb782f..abf05a9 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1102,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); > + object_unref(OBJECT(&dev->vdev)); > qdev_alias_all_properties(DEVICE(&dev->vdev), obj); > } > > -- > 1.9.0 > >
Il 29/05/2014 11:11, Peter Crosthwaite ha scritto: >> > object_initialize() leaves the object with a refcount of 1. >> > object_property_add_child() adds its own reference which is dropped >> > again when the property is deleted. >> > >> > The upshot of this is that we always have a refcount >= 1. Upon hot >> > unplug the virtio-blk child is not finalized! >> > > Doesn't this suggest that hot unplug is what's broken? My > understanding (which is fresh and not 100% yet) is the original == 1 > refcount should be dropped at object deletion time which is this sense > would be unplug time. This would mean that hot-unplug should > explicitly object_unref the object (should the intention of hot-unplug > be to always finalise the device?). I think it makes sense either way, as long as it's used consistently. Paolo
On Thu, May 29, 2014 at 07:11:27PM +1000, Peter Crosthwaite wrote: > On Fri, May 23, 2014 at 1:40 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > > object_initialize() leaves the object with a refcount of 1. > > object_property_add_child() adds its own reference which is dropped > > again when the property is deleted. > > > > The upshot of this is that we always have a refcount >= 1. Upon hot > > unplug the virtio-blk child is not finalized! > > > > Doesn't this suggest that hot unplug is what's broken? My > understanding (which is fresh and not 100% yet) is the original == 1 > refcount should be dropped at object deletion time which is this sense > would be unplug time. This would mean that hot-unplug should > explicitly object_unref the object (should the intention of hot-unplug > be to always finalise the device?). We could add an explicit object_unref() but I think it's simpler to rely on the QOM child property refcount semantics instead of keeping a duplicate reference. object_property_add_child() increments the refcount. Releasing the child property decrements the refcount again. Now, in the context of the virtio-blk-pci parent object and its VirtIOBlock child: When the parent object is finalized and its properties are released, the ref to the child will automatically be decremented. This is exactly what we need. Stefan
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index 041d4e2..bf92cb6 100644 --- a/hw/s390x/s390-virtio-bus.c +++ b/hw/s390x/s390-virtio-bus.c @@ -191,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); + object_unref(OBJECT(&dev->vdev)); qdev_alias_all_properties(DEVICE(&dev->vdev), obj); } diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index f33293d..eb8427b 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -738,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); + object_unref(OBJECT(&dev->vdev)); qdev_alias_all_properties(DEVICE(&dev->vdev), obj); } diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 3bb782f..abf05a9 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1102,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); + object_unref(OBJECT(&dev->vdev)); qdev_alias_all_properties(DEVICE(&dev->vdev), obj); }
object_initialize() leaves the object with a refcount of 1. object_property_add_child() adds its own reference which is dropped again when the property is deleted. The upshot of this is that we always have a refcount >= 1. Upon hot unplug the virtio-blk child is not finalized! Drop our reference after the child property has been added to the parent. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/s390x/s390-virtio-bus.c | 1 + hw/s390x/virtio-ccw.c | 1 + hw/virtio/virtio-pci.c | 1 + 3 files changed, 3 insertions(+)