On Wed, Sep 28, 2022 at 8:06 PM Venu Busireddy <venu.busire...@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.busire...@oracle.com> > > v2 -> v3: > - Implement the suggestion from Paolo Bonzini <pbonz...@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