Re: [Qemu-devel] [PATCH 1/3] virtio-ccw: remove qdev_unparent in unplug routing

[Christian Borntraeger]

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