Re: [Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug

[Michael S. Tsirkin]

On Fri, Sep 20, 2013 at 04:57:49PM +0200, Paolo Bonzini wrote:
> This series fixes hot-unplug of virtio devices, which can crash due to
> dangling pointer accesses.

Could you please describe the sequence of steps that makes
qemu crash?

> The current implementation supports guest-initiated hot-unplug via the
> virtio_bus_destroy_device function, but not hot-unplugging the virtio
> device by virtue of unplugging its parent container device.
> 
> The problem is that the callback for the bus implementation to cleanup
> is placed in the wrong place; it is in virtio_bus_destroy_device, which
> should be called by the bus, instead of being somewhere in device code.
> We need to have the callback in device code (for example in dc->exit),
> so that we invoke it on every unplug action, no matter who starts it.
> 
> Thus, the series cleans up plugging and unplugging of virtio devices
> so that it does not need any help from the bus (patches 1-4).  It then
> stops the virtio devices' overriding of dc->exit, moving their cleanup
> code to the new exit callback in VirtioDeviceClass (patches 5-10).
> Finally, patch 11 can make virtio-pci implement the device_unplugged
> callback.
> 
> Something similar is probably needed in virtio-ccw too.  However,
> virtio-ccw needs more surgery because it does not include a device_plugged
> callback either, so I did not touch it.
> 
> Paolo Bonzini (11):
>   virtio-bus: remove vdev field
>   virtio-pci: remove vdev field
>   virtio-ccw: remove vdev field
>   virtio-bus: cleanup plug/unplug interface
>   virtio-blk: switch exit callback to VirtioDeviceClass
>   virtio-serial: switch exit callback to VirtioDeviceClass
>   virtio-net: switch exit callback to VirtioDeviceClass
>   virtio-scsi: switch exit callback to VirtioDeviceClass
>   virtio-balloon: switch exit callback to VirtioDeviceClass
>   virtio-rng: switch exit callback to VirtioDeviceClass
>   virtio-pci: add device_unplugged callback
> 
>  hw/block/virtio-blk.c           |  10 ++--
>  hw/char/virtio-serial-bus.c     |  10 ++--
>  hw/net/virtio-net.c             |  11 ++--
>  hw/s390x/virtio-ccw.c           |  80 +++++++++++++++------------
>  hw/s390x/virtio-ccw.h           |   1 -
>  hw/scsi/vhost-scsi.c            |  11 ++--
>  hw/scsi/virtio-scsi.c           |  15 +++--
>  hw/virtio/virtio-balloon.c      |  10 ++--
>  hw/virtio/virtio-bus.c          |  81 +++++++++++++++------------
>  hw/virtio/virtio-mmio.c         |   9 +--
>  hw/virtio/virtio-pci.c          | 119 
> ++++++++++++++++++++++++----------------
>  hw/virtio/virtio-pci.h          |   1 -
>  hw/virtio/virtio-rng.c          |  10 ++--
>  hw/virtio/virtio.c              |   7 ++-
>  include/hw/virtio/virtio-bus.h  |  22 +++++---
>  include/hw/virtio/virtio-scsi.h |   2 +-
>  include/hw/virtio/virtio.h      |   1 +
>  17 files changed, 223 insertions(+), 177 deletions(-)
> 
> -- 
> 1.8.3.1