Yes, it looks like an easy backport. Adding Michael Tokarev and qemu-stable.
Paolo
On Wed, Oct 9, 2024 at 6:03 PM Michael Galaxy <mgal...@akamai.com> wrote:
>
> Hi All,
>
> We have stumbled upon this bug in our production systems on QEMU 7.2.x.
> This is a pretty nasty bug because it has the effect of causing the root
> filesystem in the guest to switch into read only mode if our block
> storage products change attachments to running virtual machines.
>
> Could we kindly ask to pull this identical patch for 7.2.15?
>
> Last year, it just went to master and landed in 8.0.50. We're planning
> to upgrade, but it will be quite some time before we get around to that,
> and I suspect others are also running 7.2.x in production.
>
> - Michael Galaxy
>
> On 7/12/23 08:43, Stefano Garzarella wrote:
> > 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 <th...@redhat.com>
> > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2176702
> > Co-developed-by: Paolo Bonzini <pbonz...@redhat.com>
> > Signed-off-by: Stefano Garzarella <sgarz...@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;
>
