Re: [PATCH hci-8.1 1/1] virtio-scsi: fix hot-plug event racing device realize #VSTOR-151351

[Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>]

LGTM
Ack, applied to hci-dev

On 10/26/26 1:11 PM, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> virtio_scsi_hotplug() is the HotplugHandler plug() callback, which
> device_set_realized() calls before it sets dev->realized. The callback
> pushes VIRTIO_SCSI_T_TRANSPORT_RESET/RESCAN to the guest at once, but
> until realized is set the request path ignores the new device:
> virtio_scsi_device_get() -> scsi_device_get() skips unrealized devices,
> and so does REPORT LUNS. A guest that reacts to the event quickly, on a
> vCPU while the request queue is served by an iothread, scans the target
> and gets VIRTIO_SCSI_S_BAD_TARGET.
> 
> Neither Linux nor Windows retries that scan, so the hot-plugged disk or
> CD-ROM stays invisible until somebody rescans the bus by hand. Linux
> calls scsi_scan_target() once from the event handler. Windows storport
> clears its rescan flag before enumerating the bus and never sets it
> again for a failed target.
> 
> Push the event, and the REPORTED LUNS CHANGED unit attention that goes
> with it, from a main-loop bottom half instead. It runs under the BQL,
> which device_add holds until device_set_realized() returns, so the guest
> is only told about a device it can reach. The iothread context would not
> do: its bottom half could run before realized is set, and the realized
> check and scsi_bus_set_ua() rely on the BQL as plug() did. The bottom
> half is only scheduled when the guest negotiated VIRTIO_SCSI_F_HOTPLUG,
> as before, so cold-plugged devices do not get one. It still checks that
> the device is realized: nothing orders it against a device_del that
> follows device_add, and an unplugged device has no parent bus.
> 
> Fixes: 8ddf958e8d62 ("scsi/scsi-bus: scsi_device_find: don't return unrealized devices")
> Cc: qemu-stable@nongnu.org
> Reported-by: Ilya Rudakov <irudakov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Fam Zheng <fam@euphon.net>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> ---
>  hw/scsi/virtio-scsi.c | 38 +++++++++++++++++++++++++++-----------
>  1 file changed, 27 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 3b635053b56..69b34adde22 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -1145,6 +1145,29 @@ static void virtio_scsi_pre_hotplug(HotplugHandler *hotplug_dev,
>      sd->hba_supports_iothread = true;
>  }
>  
> +static void virtio_scsi_hotplug_notify_bh(void *opaque)
> +{
> +    SCSIDevice *sd = opaque;
> +    DeviceState *dev = DEVICE(sd);
> +
> +    /* Nothing orders us against a following device_del */
> +    if (qdev_is_realized(dev)) {
> +        VirtIOSCSI *s = VIRTIO_SCSI(dev->parent_bus->parent);
> +        VirtIOSCSIEventInfo info = {
> +            .event   = VIRTIO_SCSI_T_TRANSPORT_RESET,
> +            .reason  = VIRTIO_SCSI_EVT_RESET_RESCAN,
> +            .address = {
> +                .id  = sd->id,
> +                .lun = sd->lun,
> +            },
> +        };
> +
> +        virtio_scsi_push_event(s, &info);
> +        scsi_bus_set_ua(&s->bus, SENSE_CODE(REPORTED_LUNS_CHANGED));
> +    }
> +    object_unref(OBJECT(sd));
> +}
> +
>  static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>                                  Error **errp)
>  {
> @@ -1163,17 +1186,10 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>      }
>  
>      if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
> -        VirtIOSCSIEventInfo info = {
> -            .event   = VIRTIO_SCSI_T_TRANSPORT_RESET,
> -            .reason  = VIRTIO_SCSI_EVT_RESET_RESCAN,
> -            .address = {
> -                .id  = sd->id,
> -                .lun = sd->lun,
> -            },
> -        };
> -
> -        virtio_scsi_push_event(s, &info);
> -        scsi_bus_set_ua(&s->bus, SENSE_CODE(REPORTED_LUNS_CHANGED));
> +        /* The request path ignores @sd until it is realized, so notify after */
> +        object_ref(OBJECT(sd));
> +        aio_bh_schedule_oneshot(qemu_get_aio_context(),
> +                                virtio_scsi_hotplug_notify_bh, sd);
>      }
>  }
>