Re: [Qemu-devel] [PATCH v4 00/30] virtio: cleanup, fix hot-unplug, move to realize

[Andreas Färber]

Am 29.11.2013 11:17, schrieb Paolo Bonzini:
> 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 2-5).  It then
> stops the virtio devices' overriding of dc->exit, moving their cleanup
> code to the new exit callback in VirtioDeviceClass (patches 6-11).
> Finally, patch 12 can make virtio-pci implement the device_unplugged
> callback.
> 
> A similar dangling-pointer bug is exposed by this change in virtio-ccw.
> Patch 1 avoids this; it is kept at the beginning to ensure bisectability.
> 
> Patches 13-30 are Andreas's conversion of virtio to realize/unrealize,
> which has better error propagation as shown by the small testcase fix.
> 
> v3->v4: rebased, added Andreas's realize patches on top
> 
> v2->v3: fix to s390 patch; added Reviewed-by and Cced patch 1 to
>       qemu-stable.
> 
> v1->v2: remove useless pointer chasing in virtio_pci_notify, add
>       patch 1 to fix breakage reported by Cornelia.
> 
> Andreas Färber (18):
>   virtio-blk-dataplane: Improve error reporting
>   virtio-9p: QOM realize preparations
>   virtio-blk: QOM realize preparations
>   virtio-serial: QOM realize preparations
>   virtio-net: QOM realize preparations
>   virtio-balloon: QOM realize preparations
>   virtio-rng: QOM realize preparations
>   virtio-scsi: QOM realize preparations
>   virtio: Start converting VirtioDevice to QOM realize
>   virtio-9p: Convert to QOM realize
>   virtio-blk: Convert to QOM realize
>   virtio-serial: Convert to QOM realize
>   virtio-net: Convert to QOM realize
>   virtio-balloon: Convert to QOM realize
>   virtio-rng: Convert to QOM realize
>   virtio-scsi: Convert to QOM realize
>   virtio: Complete converting VirtioDevice to QOM realize
>   virtio: Convert exit to unrealize
> 
> Paolo Bonzini (12):
>   virtio-ccw: move virtio_ccw_stop_ioeventfd to virtio_ccw_busdev_unplug
>   virtio-bus: remove vdev field
>   virtio-ccw: remove vdev field
>   virtio-pci: 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

These 6 patches still fail to acknowledge that by fixing the overridden
exit callback they also fix the issue that I was fixing differently in:
http://patchwork.ozlabs.org/patch/249786/ (v1)
http://patchwork.ozlabs.org/patch/263881/ (v2)

Request to add a sentence to the commit message such as:
"This also fixes a memory leak for bus_name, spotted by Andreas."
(Reported-by would be wrong since I didn't spot the 2nd hotunplug issue)

I will be offline for a couple of days and couldn't complete going
through all patches, so if the end result is working QOM-realize'ified
devices then I'm fine with these going through you, mst or whomever. :)

Thanks,
Andreas

>   virtio-pci: add device_unplugged callback

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg