Re: [Qemu-devel] [PATCH] call HotplugHandler->plug() as the last step in device realization

[Stefan Hajnoczi]

On Tue, Oct 16, 2018 at 03:33:40PM +0200, Igor Mammedov wrote:
> When [2] was fixed it was agreed that adding and calling post_plug()
> callback after device_reset() was low risk approach to hotfix issue
> right before release. So it was merged instead of moving already
> existing plug() callback after device_reset() is called which would
> be more risky and require all plug() callbacks audit.
> 
> Looking at the current plug() callbacks, it doesn't seem that moving
> plug() callback after device_reset() is breaking anything, so here
> goes agreed upon [3] proper fix which essentially reverts [1][2]
> and moves plug() callback after device_reset().
> This way devices always comes to plug() stage, after it's been fully
> initialized (including being reset), which fixes race condition [2]
> without need for an extra post_plug() callback.
> 
>  1. (25e897881 "qdev: add HotplugHandler->post_plug() callback")
>  2. (8449bcf94 "virtio-scsi: fix hotplug ->reset() vs event race")
>  3. https://www.mail-archive.com/address@hidden/msg549915.html
> 
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> TODO:
>  remove usage of Error** from plug() callback, we need to factor out
>  pre_plug part from plug() callbacks, before proceeding with it.
>  DavidH has recently finished it for pc-dimm/memory_devices, cpus
>  mostly have pre_plug parts factored out, but there still are parts
>  that could fail so it needs some more work to eliminate failure points
>  from plug() callbacks. Meanwhile, I'll plan to treat other misc
>  handlers (pci[e]/acpi/usb/...) and introduce pre_plug() where
>  necessary.
> ---
>  include/hw/hotplug.h  | 11 -----------
>  hw/core/hotplug.c     | 10 ----------
>  hw/core/qdev.c        | 16 ++++++----------
>  hw/scsi/virtio-scsi.c | 11 +----------
>  4 files changed, 7 insertions(+), 41 deletions(-)

Thanks.  I think we're okay from the virtio-scsi perspective.  I have
not audited all devices.

Reviewed-by: Stefan Hajnoczi <address@hidden>

signature.asc
Description: PGP signature