[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3] virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon
From: |
Paolo Bonzini |
Subject: |
Re: [PATCH v3] virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events. |
Date: |
Thu, 29 Sep 2022 12:49:51 +0200 |
On Wed, Sep 28, 2022 at 8:06 PM Venu Busireddy
<venu.busireddy@oracle.com> wrote:
>
> Section 5.6.6.3 of VirtIO specification states, "Events will also
> be reported via sense codes..." However, no sense data is sent when
> VIRTIO_SCSI_EVT_RESET_RESCAN or VIRTIO_SCSI_EVT_RESET_REMOVED events
> are reported (when disk hotplug/hotunplug events occur). SCSI layer
> on Solaris depends on this sense data, and hence does not handle disk
> hotplug/hotunplug events.
>
> When disk inventory changes, return a CHECK_CONDITION status with sense
> data of 0x06/0x3F/0x0E (sense code REPORTED_LUNS_CHANGED), as per the
> specifications in Section 5.14 (h) of SAM-4.
>
> Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com>
>
> v2 -> v3:
> - Implement the suggestion from Paolo Bonzini <pbonzini@redhat.com>.
>
> v1 -> v2:
> - Send the sense data for VIRTIO_SCSI_EVT_RESET_REMOVED event too.
> ---
> hw/scsi/scsi-bus.c | 1 +
> hw/scsi/virtio-scsi.c | 16 ++++++++++++++++
> include/hw/scsi/scsi.h | 6 ++++++
> include/hw/virtio/virtio-scsi.h | 1 +
> 4 files changed, 24 insertions(+)
>
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index 4403717c4aaf..b7cb249f2eab 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -730,6 +730,7 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag,
> uint32_t lun,
> */
> !(buf[0] == REQUEST_SENSE && d->sense_is_ua))) {
> ops = &reqops_unit_attention;
> + d->clear_reported_luns_changed = true;
Any reason to have this flag, and not just clear
s->reported_luns_changed after scsi_req_new? Is it to handle the
invalid opcode case?
I just reread the code and noticed that there is also a *bus* unit
attention mechanism, which is unused but seems perfect for this
usecase. The first device on the bus to execute a command successfully
will consume it.
You need something like
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 4403717c4a..78274e8477 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1616,6 +1616,24 @@ static int scsi_ua_precedence(SCSISense sense)
return (sense.asc << 8) | sense.ascq;
}
+void scsi_bus_set_ua(SCSIBus *bus, SCSISense sense)
+{
+ int prec1, prec2;
+ if (sense.key != UNIT_ATTENTION) {
+ return;
+ }
+
+ /*
+ * Override a pre-existing unit attention condition, except for a more
+ * important reset condition.
+ */
+ prec1 = scsi_ua_precedence(bus->unit_attention);
+ prec2 = scsi_ua_precedence(sense);
+ if (prec2 < prec1) {
+ bus->unit_attention = sense;
+ }
+}
+
void scsi_device_set_ua(SCSIDevice *sdev, SCSISense sense)
{
int prec1, prec2;
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 88e1a48343..0c86d0359f 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -186,6 +186,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(
BlockdevOnError rerror,
BlockdevOnError werror,
const char *serial, Error **errp);
+void scsi_bus_set_ua(SCSIBus *bus, SCSISense sense);
void scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
void scsi_legacy_handle_cmdline(void);
and if you call the new function in the plug/unplug callbacks it
should just work.
Paolo