| Message ID | 20220531202237.274483-1-venu.busireddy@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon a disk hotplug. | expand |
Ping? On 2022-05-31 15:22:37 -0500, Venu Busireddy wrote: > When a disk is hotplugged, QEMU reports a VIRTIO_SCSI_EVT_RESET_RESCAN > event, but does not send the "REPORTED LUNS CHANGED" sense data. This > does not conform to Section 5.6.6.3 of the VirtIO specification, which > states "Events will also be reported via sense codes..." SCSI layer on > Solaris depends on this sense data, and hence does not recognize the > hotplugged disks (until a reboot). > > As specified in SAM-4, Section 5.14, return a CHECK_CONDITION status with > a sense data of 0x06/0x3F/0x0E, whenever a command other than INQUIRY, > REPORT_LUNS, or REQUEST_SENSE is received. > > Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> > --- > hw/scsi/virtio-scsi.c | 15 ++++++++++++++- > include/hw/virtio/virtio-scsi.h | 1 + > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index 4141dddd517a..7ae1cfa6e584 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -608,7 +608,19 @@ static void virtio_scsi_command_complete(SCSIRequest *r, size_t resid) > > req->resp.cmd.response = VIRTIO_SCSI_S_OK; > req->resp.cmd.status = r->status; > - if (req->resp.cmd.status == GOOD) { > + if (req->dev->reported_luns_changed && > + (req->req.cmd.cdb[0] != INQUIRY) && > + (req->req.cmd.cdb[0] != REPORT_LUNS) && > + (req->req.cmd.cdb[0] != REQUEST_SENSE)) { > + req->dev->reported_luns_changed = false; > + req->resp.cmd.resid = 0; > + req->resp.cmd.status_qualifier = 0; > + req->resp.cmd.status = CHECK_CONDITION; > + sense_len = scsi_build_sense(sense, SENSE_CODE(REPORTED_LUNS_CHANGED)); > + qemu_iovec_from_buf(&req->resp_iov, sizeof(req->resp.cmd), > + sense, sense_len); > + req->resp.cmd.sense_len = virtio_tswap32(vdev, sense_len); > + } else if (req->resp.cmd.status == GOOD) { > req->resp.cmd.resid = virtio_tswap32(vdev, resid); > } else { > req->resp.cmd.resid = 0; > @@ -956,6 +968,7 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, > VIRTIO_SCSI_T_TRANSPORT_RESET, > VIRTIO_SCSI_EVT_RESET_RESCAN); > virtio_scsi_release(s); > + s->reported_luns_changed = true; > } > } > > diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h > index a36aad9c8695..efbcf9ba069a 100644 > --- a/include/hw/virtio/virtio-scsi.h > +++ b/include/hw/virtio/virtio-scsi.h > @@ -81,6 +81,7 @@ struct VirtIOSCSI { > SCSIBus bus; > int resetting; > bool events_dropped; > + bool reported_luns_changed; > > /* Fields for dataplane below */ > AioContext *ctx; /* one iothread per virtio-scsi-pci for now */
Ping? On 2022-05-31 15:22:37 -0500, Venu Busireddy wrote: > When a disk is hotplugged, QEMU reports a VIRTIO_SCSI_EVT_RESET_RESCAN > event, but does not send the "REPORTED LUNS CHANGED" sense data. This > does not conform to Section 5.6.6.3 of the VirtIO specification, which > states "Events will also be reported via sense codes..." SCSI layer on > Solaris depends on this sense data, and hence does not recognize the > hotplugged disks (until a reboot). > > As specified in SAM-4, Section 5.14, return a CHECK_CONDITION status with > a sense data of 0x06/0x3F/0x0E, whenever a command other than INQUIRY, > REPORT_LUNS, or REQUEST_SENSE is received. > > Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> > --- > hw/scsi/virtio-scsi.c | 15 ++++++++++++++- > include/hw/virtio/virtio-scsi.h | 1 + > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index 4141dddd517a..7ae1cfa6e584 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -608,7 +608,19 @@ static void virtio_scsi_command_complete(SCSIRequest *r, size_t resid) > > req->resp.cmd.response = VIRTIO_SCSI_S_OK; > req->resp.cmd.status = r->status; > - if (req->resp.cmd.status == GOOD) { > + if (req->dev->reported_luns_changed && > + (req->req.cmd.cdb[0] != INQUIRY) && > + (req->req.cmd.cdb[0] != REPORT_LUNS) && > + (req->req.cmd.cdb[0] != REQUEST_SENSE)) { > + req->dev->reported_luns_changed = false; > + req->resp.cmd.resid = 0; > + req->resp.cmd.status_qualifier = 0; > + req->resp.cmd.status = CHECK_CONDITION; > + sense_len = scsi_build_sense(sense, SENSE_CODE(REPORTED_LUNS_CHANGED)); > + qemu_iovec_from_buf(&req->resp_iov, sizeof(req->resp.cmd), > + sense, sense_len); > + req->resp.cmd.sense_len = virtio_tswap32(vdev, sense_len); > + } else if (req->resp.cmd.status == GOOD) { > req->resp.cmd.resid = virtio_tswap32(vdev, resid); > } else { > req->resp.cmd.resid = 0; > @@ -956,6 +968,7 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, > VIRTIO_SCSI_T_TRANSPORT_RESET, > VIRTIO_SCSI_EVT_RESET_RESCAN); > virtio_scsi_release(s); > + s->reported_luns_changed = true; > } > } > > diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h > index a36aad9c8695..efbcf9ba069a 100644 > --- a/include/hw/virtio/virtio-scsi.h > +++ b/include/hw/virtio/virtio-scsi.h > @@ -81,6 +81,7 @@ struct VirtIOSCSI { > SCSIBus bus; > int resetting; > bool events_dropped; > + bool reported_luns_changed; > > /* Fields for dataplane below */ > AioContext *ctx; /* one iothread per virtio-scsi-pci for now */
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 4141dddd517a..7ae1cfa6e584 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -608,7 +608,19 @@ static void virtio_scsi_command_complete(SCSIRequest *r, size_t resid) req->resp.cmd.response = VIRTIO_SCSI_S_OK; req->resp.cmd.status = r->status; - if (req->resp.cmd.status == GOOD) { + if (req->dev->reported_luns_changed && + (req->req.cmd.cdb[0] != INQUIRY) && + (req->req.cmd.cdb[0] != REPORT_LUNS) && + (req->req.cmd.cdb[0] != REQUEST_SENSE)) { + req->dev->reported_luns_changed = false; + req->resp.cmd.resid = 0; + req->resp.cmd.status_qualifier = 0; + req->resp.cmd.status = CHECK_CONDITION; + sense_len = scsi_build_sense(sense, SENSE_CODE(REPORTED_LUNS_CHANGED)); + qemu_iovec_from_buf(&req->resp_iov, sizeof(req->resp.cmd), + sense, sense_len); + req->resp.cmd.sense_len = virtio_tswap32(vdev, sense_len); + } else if (req->resp.cmd.status == GOOD) { req->resp.cmd.resid = virtio_tswap32(vdev, resid); } else { req->resp.cmd.resid = 0; @@ -956,6 +968,7 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_RESCAN); virtio_scsi_release(s); + s->reported_luns_changed = true; } } diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index a36aad9c8695..efbcf9ba069a 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -81,6 +81,7 @@ struct VirtIOSCSI { SCSIBus bus; int resetting; bool events_dropped; + bool reported_luns_changed; /* Fields for dataplane below */ AioContext *ctx; /* one iothread per virtio-scsi-pci for now */
When a disk is hotplugged, QEMU reports a VIRTIO_SCSI_EVT_RESET_RESCAN event, but does not send the "REPORTED LUNS CHANGED" sense data. This does not conform to Section 5.6.6.3 of the VirtIO specification, which states "Events will also be reported via sense codes..." SCSI layer on Solaris depends on this sense data, and hence does not recognize the hotplugged disks (until a reboot). As specified in SAM-4, Section 5.14, return a CHECK_CONDITION status with a sense data of 0x06/0x3F/0x0E, whenever a command other than INQUIRY, REPORT_LUNS, or REQUEST_SENSE is received. Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> --- hw/scsi/virtio-scsi.c | 15 ++++++++++++++- include/hw/virtio/virtio-scsi.h | 1 + 2 files changed, 15 insertions(+), 1 deletion(-)