Re: [Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug
On Tue, Oct 08, 2013 at 07:02:44PM +0200, Paolo Bonzini wrote: Il 20/09/2013 16:57, Paolo Bonzini ha scritto: This series fixes hot-unplug of virtio devices, which can crash due to dangling pointer accesses. 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. Michael, I prepared a rebase of http://permalink.gmane.org/gmane.comp.emulators.qemu/225985 on top of these patches and sent it to Andreas. My understanding is that he will send them to qemu-devel. Let me know if you want to handle these patches yourself, or I can send a pull request for both directly with your Acked-bys. As to review, I think it can be usefully split as follows: - 4-10 for Andreas - 3 for Alex - 2 11 for you - 1 for either you or Andreas Paolo OK so I can merge this but need Acks from Alex and Andreas, right? 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(-)
Re: [Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug
Il 22/09/2013 10:08, Paolo Bonzini ha scritto: Il 21/09/2013 21:17, Michael S. Tsirkin ha scritto: 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? See patch 11. I didn't find out why it fails with PCIe but not PCI, probably a difference in how malloc reuses freed blocks. Ping? Paolo
Re: [Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug
Il 20/09/2013 16:57, Paolo Bonzini ha scritto: This series fixes hot-unplug of virtio devices, which can crash due to dangling pointer accesses. 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. Michael, I prepared a rebase of http://permalink.gmane.org/gmane.comp.emulators.qemu/225985 on top of these patches and sent it to Andreas. My understanding is that he will send them to qemu-devel. Let me know if you want to handle these patches yourself, or I can send a pull request for both directly with your Acked-bys. As to review, I think it can be usefully split as follows: - 4-10 for Andreas - 3 for Alex - 2 11 for you - 1 for either you or Andreas Paolo 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(-)
Re: [Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug
Il 21/09/2013 21:17, Michael S. Tsirkin ha scritto: 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? See patch 11. I didn't find out why it fails with PCIe but not PCI, probably a difference in how malloc reuses freed blocks. Paolo
Re: [Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug
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
[Qemu-devel] [PATCH 00/11] virtio: cleanup and fix hot-unplug
This series fixes hot-unplug of virtio devices, which can crash due to dangling pointer accesses. 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
