[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug
From: |
Christian Borntraeger |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing |
Date: |
Sun, 24 Feb 2013 15:38:12 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 |
On 24/02/13 12:30, Andreas Färber wrote:
> Am 22.02.2013 20:01, schrieb Jens Freimann:
>> From: Christian Borntraeger <address@hidden>
>>
>> This patch fixes a crash when unplugging a virtio-ccw device. We no
>> longer need to do that in virtio-ccw since common code does now
>> proper handling.
>>
>> Signed-off-by: Christian Borntraeger <address@hidden>
>> Signed-off-by: Jens Freimann <address@hidden>
>> ---
>> hw/s390x/virtio-ccw.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>> index 231f81e..679afa6 100644
>> --- a/hw/s390x/virtio-ccw.c
>> +++ b/hw/s390x/virtio-ccw.c
>> @@ -865,7 +865,6 @@ static int virtio_ccw_busdev_unplug(DeviceState *dev)
>>
>> css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, 1, 0);
>>
>> - object_unparent(OBJECT(dev));
>> qdev_free(dev);
>> return 0;
>> }
>
> qdev_free() does exactly object_unparent(), so in light of wanting to
> get away from qdev I would suggest to rather drop qdev_free() here. Paolo?
Agreed, this is identical from a functional perspective and I was asking
that myself, but it seems that qdev_free is an interface. used by many busses:
$ git grep qdev_free
hw/acpi_piix4.c: qdev_free(qdev);
hw/pci/pci-hotplug.c: qdev_free(&dev->qdev);
hw/pci/pci_bridge.c: /* qbus_free() is called automatically by qdev_free() */
hw/pci/pcie.c: qdev_free(&pci_dev->qdev);
hw/pci/shpc.c: qdev_free(&affected_dev->qdev);
hw/qdev-core.h:void qdev_free(DeviceState *dev);
hw/qdev-monitor.c: qdev_free(qdev);
hw/qdev.c: qdev_free(dev);
hw/qdev.c: qdev_free(dev);
hw/qdev.c:void qdev_free(DeviceState *dev)
hw/qdev.c: qdev_free(dev);
hw/s390x/virtio-ccw.c: qdev_free(dev);
hw/scsi-bus.c: qdev_free(&d->qdev);
hw/scsi-bus.c: qdev_free(dev);
hw/usb/bus.c: qdev_free(&port->dev->qdev);
hw/usb/bus.c: qdev_free(&dev->qdev);
hw/usb/dev-storage.c: qdev_free(&dev->qdev);
hw/usb/host-linux.c: qdev_free(&dev->qdev);
hw/virtio-bus.c: qdev_free(qdev);
hw/xen_platform.c: qdev_free(&d->qdev);
...while object_unparent is still somewhat internal.
$ git grep object_unparent
hw/qdev.c: object_unparent(OBJECT(dev));
hw/qdev.c: object_unparent(OBJECT(bus));
include/qom/object.h:void object_unparent(Object *obj);
qom/object.c:void object_unparent(Object *obj)
Another thing is, that qdev_free looks now different, some days ago
it also did an unref. As far as I can see the object_unparent in virtio-ccw
was always the wrong thing to do. So for a potential backport this version
of the patch might be better.
Paolo, do you have any guidance?
Christian
[Qemu-devel] [PATCH 3/3] s390/virtio-ccw: remove redundant call to blockdev_mark_auto_del, Jens Freimann, 2013/02/22
[Qemu-devel] [PATCH 2/3] s390/css: Fix subchannel detection, Jens Freimann, 2013/02/22