* [PATCH v4] virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events.
@ 2022-10-06 19:49 Venu Busireddy
2022-10-11 10:34 ` Paolo Bonzini
2023-07-04 14:14 ` Stefano Garzarella
0 siblings, 2 replies; 5+ messages in thread
From: Venu Busireddy @ 2022-10-06 19:49 UTC (permalink / raw)
To: venu.busireddy, qemu-devel; +Cc: Fam Zheng, Michael S. Tsirkin, Paolo Bonzini
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 the disk inventory changes, use the bus unit attention mechanism
to return a CHECK_CONDITION status with sense data of 0x06/0x3F/0x0E
(sense code REPORTED_LUNS_CHANGED).
Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com>
v3 -> v4:
- As suggested by Paolo Bonzini, use the bus unit attention mechanism
instead of the device unit attention mechanism.
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 | 18 ++++++++++++++++++
hw/scsi/virtio-scsi.c | 2 ++
include/hw/scsi/scsi.h | 1 +
3 files changed, 21 insertions(+)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 4403717c4aaf..ceceafb2cdf3 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/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 41f2a5630173..cf2721aa46c0 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -956,6 +956,7 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
virtio_scsi_push_event(s, sd,
VIRTIO_SCSI_T_TRANSPORT_RESET,
VIRTIO_SCSI_EVT_RESET_RESCAN);
+ scsi_bus_set_ua(&s->bus, SENSE_CODE(REPORTED_LUNS_CHANGED));
virtio_scsi_release(s);
}
}
@@ -973,6 +974,7 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev,
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);
}
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 001103488c23..3b1b3d278ee8 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -186,6 +186,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
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);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4] virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events.
2022-10-06 19:49 [PATCH v4] virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events Venu Busireddy
@ 2022-10-11 10:34 ` Paolo Bonzini
2022-10-11 11:14 ` Venu Busireddy
2023-07-04 14:14 ` Stefano Garzarella
1 sibling, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2022-10-11 10:34 UTC (permalink / raw)
To: Venu Busireddy; +Cc: qemu-devel, Fam Zheng, Michael S . Tsirkin
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events.
2022-10-11 10:34 ` Paolo Bonzini
@ 2022-10-11 11:14 ` Venu Busireddy
0 siblings, 0 replies; 5+ messages in thread
From: Venu Busireddy @ 2022-10-11 11:14 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Fam Zheng, Michael S . Tsirkin
On 2022-10-11 12:34:56 +0200, Paolo Bonzini wrote:
> Queued, thanks.
Thank you!
Venu
>
> Paolo
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events.
2022-10-06 19:49 [PATCH v4] virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events Venu Busireddy
2022-10-11 10:34 ` Paolo Bonzini
@ 2023-07-04 14:14 ` Stefano Garzarella
2023-07-05 12:37 ` Mark Kanda
1 sibling, 1 reply; 5+ messages in thread
From: Stefano Garzarella @ 2023-07-04 14:14 UTC (permalink / raw)
To: Mark Kanda
Cc: Joao Martins, Venu Busireddy, qemu devel list, Fam Zheng,
Michael S. Tsirkin, Paolo Bonzini
Hi Mark,
we have a bug [1] possibly related to this patch.
I saw this Oracle Linux errata [2] where you reverted this patch, but
there are no details.
Do you think we should revert it upstream as well?
Do you have any details about the problem it causes in Linux?
[1] https://bugzilla.redhat.com/show_bug.cgi?id=2176702
[2] https://linux.oracle.com/errata/ELSA-2023-12065.html
Thanks,
Stefano
On Thu, Oct 6, 2022 at 9:53 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 the disk inventory changes, use the bus unit attention mechanism
> to return a CHECK_CONDITION status with sense data of 0x06/0x3F/0x0E
> (sense code REPORTED_LUNS_CHANGED).
>
> Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com>
>
> v3 -> v4:
> - As suggested by Paolo Bonzini, use the bus unit attention mechanism
> instead of the device unit attention mechanism.
>
> 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 | 18 ++++++++++++++++++
> hw/scsi/virtio-scsi.c | 2 ++
> include/hw/scsi/scsi.h | 1 +
> 3 files changed, 21 insertions(+)
>
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index 4403717c4aaf..ceceafb2cdf3 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/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 41f2a5630173..cf2721aa46c0 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -956,6 +956,7 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
> virtio_scsi_push_event(s, sd,
> VIRTIO_SCSI_T_TRANSPORT_RESET,
> VIRTIO_SCSI_EVT_RESET_RESCAN);
> + scsi_bus_set_ua(&s->bus, SENSE_CODE(REPORTED_LUNS_CHANGED));
> virtio_scsi_release(s);
> }
> }
> @@ -973,6 +974,7 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev,
> 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);
> }
>
> diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> index 001103488c23..3b1b3d278ee8 100644
> --- a/include/hw/scsi/scsi.h
> +++ b/include/hw/scsi/scsi.h
> @@ -186,6 +186,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
> 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);
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events.
2023-07-04 14:14 ` Stefano Garzarella
@ 2023-07-05 12:37 ` Mark Kanda
0 siblings, 0 replies; 5+ messages in thread
From: Mark Kanda @ 2023-07-05 12:37 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Joao Martins, Venu Busireddy, qemu devel list, Fam Zheng,
Michael S. Tsirkin, Paolo Bonzini
HI Stefano,
On 7/4/2023 9:14 AM, Stefano Garzarella wrote:
> Hi Mark,
> we have a bug [1] possibly related to this patch.
>
> I saw this Oracle Linux errata [2] where you reverted this patch, but
> there are no details.
>
> Do you think we should revert it upstream as well?
> Do you have any details about the problem it causes in Linux?
While the patch fixed an issue with Solaris VMs, we unfortunately discovered
regressions with hotplug on Linux VMs. The symptoms were similar to what is
reported in bz2176702. I think reverting it is the right thing to do until this
can be investigated further.
Thanks/regards,
-Mark
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=2176702
> [2] https://linux.oracle.com/errata/ELSA-2023-12065.html
>
> Thanks,
> Stefano
>
> On Thu, Oct 6, 2022 at 9:53 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 the disk inventory changes, use the bus unit attention mechanism
>> to return a CHECK_CONDITION status with sense data of 0x06/0x3F/0x0E
>> (sense code REPORTED_LUNS_CHANGED).
>>
>> Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com>
>>
>> v3 -> v4:
>> - As suggested by Paolo Bonzini, use the bus unit attention mechanism
>> instead of the device unit attention mechanism.
>>
>> 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 | 18 ++++++++++++++++++
>> hw/scsi/virtio-scsi.c | 2 ++
>> include/hw/scsi/scsi.h | 1 +
>> 3 files changed, 21 insertions(+)
>>
>> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
>> index 4403717c4aaf..ceceafb2cdf3 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/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
>> index 41f2a5630173..cf2721aa46c0 100644
>> --- a/hw/scsi/virtio-scsi.c
>> +++ b/hw/scsi/virtio-scsi.c
>> @@ -956,6 +956,7 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>> virtio_scsi_push_event(s, sd,
>> VIRTIO_SCSI_T_TRANSPORT_RESET,
>> VIRTIO_SCSI_EVT_RESET_RESCAN);
>> + scsi_bus_set_ua(&s->bus, SENSE_CODE(REPORTED_LUNS_CHANGED));
>> virtio_scsi_release(s);
>> }
>> }
>> @@ -973,6 +974,7 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>> 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);
>> }
>>
>> diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
>> index 001103488c23..3b1b3d278ee8 100644
>> --- a/include/hw/scsi/scsi.h
>> +++ b/include/hw/scsi/scsi.h
>> @@ -186,6 +186,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
>> 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);
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-05 12:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-06 19:49 [PATCH v4] virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events Venu Busireddy
2022-10-11 10:34 ` Paolo Bonzini
2022-10-11 11:14 ` Venu Busireddy
2023-07-04 14:14 ` Stefano Garzarella
2023-07-05 12:37 ` Mark Kanda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).