Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
virtio-scsi: avoid race between unplug and transport event 
Only report a transport reset event to the guest after the SCSIDevice
has been unrealized by qdev_simple_device_unplug_cb().

qdev_simple_device_unplug_cb() sets the SCSIDevice's qdev.realized field
to false so that scsi_device_find/get() no longer see it.

scsi_target_emulate_report_luns() also needs to be updated to filter out
SCSIDevices that are unrealized.

Change virtio_scsi_push_event() to take event information as an argument
instead of the SCSIDevice. This allows virtio_scsi_hotunplug() to emit a
VIRTIO_SCSI_T_TRANSPORT_RESET event after the SCSIDevice has already
been unrealized.

These changes ensure that the guest driver does not see the SCSIDevice
that's being unplugged if it responds very quickly to the transport
reset event.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230516190238.8401-4-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
Stefan Hajnoczi authored and Kevin Wolf committed May 30, 2023
1 parent 26462a7 commit 4382f16
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 26 deletions.
3 changes: 2 additions & 1 deletion hw/scsi/scsi-bus.c
View file
Expand Up @@ -487,7 +487,8 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
DeviceState *qdev = kid->child;
SCSIDevice *dev = SCSI_DEVICE(qdev);

if (dev->channel == channel && dev->id == id && dev->lun != 0) {
if (dev->channel == channel && dev->id == id && dev->lun != 0 &&
qdev_is_realized(&dev->qdev)) {
store_lun(tmp, dev->lun);
g_byte_array_append(buf, tmp, 8);
len += 8;
Expand Down
86 changes: 61 additions & 25 deletions hw/scsi/virtio-scsi.c
View file
Expand Up @@ -933,13 +933,27 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
s->events_dropped = false;
}

static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
uint32_t event, uint32_t reason)
typedef struct {
uint32_t event;
uint32_t reason;
union {
/* Used by messages specific to a device */
struct {
uint32_t id;
uint32_t lun;
} address;
};
} VirtIOSCSIEventInfo;

static void virtio_scsi_push_event(VirtIOSCSI *s,
const VirtIOSCSIEventInfo *info)
{
VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
VirtIOSCSIReq *req;
VirtIOSCSIEvent *evt;
VirtIODevice *vdev = VIRTIO_DEVICE(s);
uint32_t event = info->event;
uint32_t reason = info->reason;

if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
return;
Expand All @@ -965,27 +979,28 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
memset(evt, 0, sizeof(VirtIOSCSIEvent));
evt->event = virtio_tswap32(vdev, event);
evt->reason = virtio_tswap32(vdev, reason);
if (!dev) {
assert(event == VIRTIO_SCSI_T_EVENTS_MISSED);
} else {
if (event != VIRTIO_SCSI_T_EVENTS_MISSED) {
evt->lun[0] = 1;
evt->lun[1] = dev->id;
evt->lun[1] = info->address.id;

/* Linux wants us to keep the same encoding we use for REPORT LUNS. */
if (dev->lun >= 256) {
evt->lun[2] = (dev->lun >> 8) | 0x40;
if (info->address.lun >= 256) {
evt->lun[2] = (info->address.lun >> 8) | 0x40;
}
evt->lun[3] = dev->lun & 0xFF;
evt->lun[3] = info->address.lun & 0xFF;
}
trace_virtio_scsi_event(virtio_scsi_get_lun(evt->lun), event, reason);

virtio_scsi_complete_req(req);
}

static void virtio_scsi_handle_event_vq(VirtIOSCSI *s, VirtQueue *vq)
{
if (s->events_dropped) {
virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0);
VirtIOSCSIEventInfo info = {
.event = VIRTIO_SCSI_T_NO_EVENT,
};
virtio_scsi_push_event(s, &info);
}
}

Expand All @@ -1009,9 +1024,17 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense)

if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_CHANGE) &&
dev->type != TYPE_ROM) {
VirtIOSCSIEventInfo info = {
.event = VIRTIO_SCSI_T_PARAM_CHANGE,
.reason = sense.asc | (sense.ascq << 8),
.address = {
.id = dev->id,
.lun = dev->lun,
},
};

virtio_scsi_acquire(s);
virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE,
sense.asc | (sense.ascq << 8));
virtio_scsi_push_event(s, &info);
virtio_scsi_release(s);
}
}
Expand Down Expand Up @@ -1046,10 +1069,17 @@ 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_acquire(s);
virtio_scsi_push_event(s, sd,
VIRTIO_SCSI_T_TRANSPORT_RESET,
VIRTIO_SCSI_EVT_RESET_RESCAN);
virtio_scsi_push_event(s, &info);
scsi_bus_set_ua(&s->bus, SENSE_CODE(REPORTED_LUNS_CHANGED));
virtio_scsi_release(s);
}
Expand All @@ -1062,15 +1092,14 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev,
VirtIOSCSI *s = VIRTIO_SCSI(vdev);
SCSIDevice *sd = SCSI_DEVICE(dev);
AioContext *ctx = s->ctx ?: qemu_get_aio_context();

if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
virtio_scsi_acquire(s);
virtio_scsi_push_event(s, sd,
VIRTIO_SCSI_T_TRANSPORT_RESET,
VIRTIO_SCSI_EVT_RESET_REMOVED);
scsi_bus_set_ua(&s->bus, SENSE_CODE(REPORTED_LUNS_CHANGED));
virtio_scsi_release(s);
}
VirtIOSCSIEventInfo info = {
.event = VIRTIO_SCSI_T_TRANSPORT_RESET,
.reason = VIRTIO_SCSI_EVT_RESET_REMOVED,
.address = {
.id = sd->id,
.lun = sd->lun,
},
};

aio_disable_external(ctx);
qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
Expand All @@ -1082,6 +1111,13 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev,
blk_set_aio_context(sd->conf.blk, qemu_get_aio_context(), NULL);
virtio_scsi_release(s);
}

if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
virtio_scsi_acquire(s);
virtio_scsi_push_event(s, &info);
scsi_bus_set_ua(&s->bus, SENSE_CODE(REPORTED_LUNS_CHANGED));
virtio_scsi_release(s);
}
}

static struct SCSIBusInfo virtio_scsi_scsi_info = {
Expand Down

0 comments on commit 4382f16

Please sign in to comment.