Commit 1880ad4f4e ("virtio-scsi: Batched prepare for cmd reqs") split
calls to scsi_req_new() and scsi_req_enqueue() in the virtio-scsi device.
This had no drawback, until commit 8cc5583abe ("virtio-scsi: Send
"REPORTED LUNS CHANGED" sense data upon disk hotplug events") added a
bus unit attention.
Having the two calls separated, all requests in the batch were prepared
calling scsi_req_new() to report a sense.
Then only the first one submitted calling scsi_req_enqueue() reported the
right sense and reset it to NO_SENSE.
The others reported NO_SENSE, causing SCSI errors in Linux.
To solve this issue, let's fetch the unit attention as early as possible
when we prepare the request, that way only the first request in the batch
will carry the right sense.
Fixes: 1880ad4f4e ("virtio-scsi: Batched prepare for cmd reqs")
Fixes: 8cc5583abe ("virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk
hotplug events")
Reported-by: Thomas Huth <thuth@redhat.com>
Buglink:
https://urldefense.com/v3/__https://bugzilla.redhat.com/show_bug.cgi?id=2176702__;!!GjvTz_vk!RKyuBiMbJ5CywoidrGSu50njz30GGlEg54dEmfsw_y2Ab8NxP2zkHyYy-Iajg9-KC0IO_9h3L-iac6O3$
Co-developed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
include/hw/scsi/scsi.h | 1 +
hw/scsi/scsi-bus.c | 36 +++++++++++++++++++++++++++++++++---
2 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index e2bb1a2fbf..3692ca82f3 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -108,6 +108,7 @@ int cdrom_read_toc_raw(int nb_sectors, uint8_t *buf, int
msf, int session_num);
/* scsi-bus.c */
struct SCSIReqOps {
size_t size;
+ void (*init_req)(SCSIRequest *req);
void (*free_req)(SCSIRequest *req);
int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
void (*read_data)(SCSIRequest *req);
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index f80f4cb4fc..f083373021 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -412,19 +412,35 @@ static const struct SCSIReqOps reqops_invalid_opcode = {
/* SCSIReqOps implementation for unit attention conditions. */
-static int32_t scsi_unit_attention(SCSIRequest *req, uint8_t *buf)
+static void scsi_fetch_unit_attention_sense(SCSIRequest *req)
{
+ SCSISense *ua = NULL;
+
if (req->dev->unit_attention.key == UNIT_ATTENTION) {
- scsi_req_build_sense(req, req->dev->unit_attention);
+ ua = &req->dev->unit_attention;
} else if (req->bus->unit_attention.key == UNIT_ATTENTION) {
- scsi_req_build_sense(req, req->bus->unit_attention);
+ ua = &req->bus->unit_attention;
}
+
+ /*
+ * Fetch the unit attention sense immediately so that another
+ * scsi_req_new does not use reqops_unit_attention.
+ */
+ if (ua) {
+ scsi_req_build_sense(req, *ua);
+ *ua = SENSE_CODE(NO_SENSE);
+ }
+}
+
+static int32_t scsi_unit_attention(SCSIRequest *req, uint8_t *buf)
+{
scsi_req_complete(req, CHECK_CONDITION);
return 0;
}
static const struct SCSIReqOps reqops_unit_attention = {
.size = sizeof(SCSIRequest),
+ .init_req = scsi_fetch_unit_attention_sense,
.send_command = scsi_unit_attention
};
@@ -699,6 +715,11 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops,
SCSIDevice *d,
object_ref(OBJECT(d));
object_ref(OBJECT(qbus->parent));
notifier_list_init(&req->cancel_notifiers);
+
+ if (reqops->init_req) {
+ reqops->init_req(req);
+ }
+
trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
return req;
}
@@ -798,6 +819,15 @@ uint8_t *scsi_req_get_buf(SCSIRequest *req)
static void scsi_clear_unit_attention(SCSIRequest *req)
{
SCSISense *ua;
+
+ /*
+ * scsi_fetch_unit_attention_sense() already cleaned the unit attention
+ * in this case.
+ */
+ if (req->ops == &reqops_unit_attention) {
+ return;
+ }
+
if (req->dev->unit_attention.key != UNIT_ATTENTION &&
req->bus->unit_attention.key != UNIT_ATTENTION) {
return;