Re: [Qemu-devel] [PATCH v2 2/2] virtio-scsi: fix hotplug ->reset() vs event race

[Michael S. Tsirkin]

On Mon, Jul 16, 2018 at 09:37:32AM +0100, Stefan Hajnoczi wrote:
> There is a race condition during hotplug when iothread is used.  It
> occurs because virtio-scsi may be processing command queues in the
> iothread while the monitor performs SCSI device hotplug.
> 
> When a SCSI device is hotplugged the HotplugHandler->plug() callback is
> invoked and virtio-scsi emits a rescan event to the guest.
> 
> If the guest submits a SCSI command at this point then it may be
> cancelled before hotplug completes.  This happens because ->reset() is
> called by hw/core/qdev.c:device_set_realized() after
> HotplugHandler->plug() has been called and
> hw/scsi/scsi-disk.c:scsi_disk_reset() purges all requests.
> 
> This patch uses the new HotplugHandler->post_plug() callback to emit the
> rescan event after ->reset().  This eliminates the race conditions where
> requests could be cancelled.
> 
> Reported-by: l00284672 <address@hidden>
> Cc: Paolo Bonzini <address@hidden>
> Cc: Fam Zheng <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>

Acked-by: Michael S. Tsirkin <address@hidden>

Pls merge through scsi tree.

> ---
>  hw/scsi/virtio-scsi.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 3aa99717e2..5a3057d1f8 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -797,8 +797,16 @@ static void virtio_scsi_hotplug(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>          virtio_scsi_acquire(s);
>          blk_set_aio_context(sd->conf.blk, s->ctx);
>          virtio_scsi_release(s);
> -
>      }
> +}
> +
> +/* Announce the new device after it has been plugged */
> +static void virtio_scsi_post_hotplug(HotplugHandler *hotplug_dev,
> +                                     DeviceState *dev)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev);
> +    VirtIOSCSI *s = VIRTIO_SCSI(vdev);
> +    SCSIDevice *sd = SCSI_DEVICE(dev);
>  
>      if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
>          virtio_scsi_acquire(s);
> @@ -968,6 +976,7 @@ static void virtio_scsi_class_init(ObjectClass *klass, 
> void *data)
>      vdc->start_ioeventfd = virtio_scsi_dataplane_start;
>      vdc->stop_ioeventfd = virtio_scsi_dataplane_stop;
>      hc->plug = virtio_scsi_hotplug;
> +    hc->post_plug = virtio_scsi_post_hotplug;
>      hc->unplug = virtio_scsi_hotunplug;
>  }
>  
> -- 
> 2.17.1