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

["Denis V. Lunev" <den@openvz.org>]

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);
     }
 }
 
-- 
2.53.0