Patchwork [04/16] scsi: move sense handling to generic code

login
register
mail settings
Submitter Paolo Bonzini
Date Aug. 3, 2011, 8:49 a.m.
Message ID <1312361359-15445-5-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/108083/
State New
Headers show

Comments

Paolo Bonzini - Aug. 3, 2011, 8:49 a.m.
With this patch, sense data is stored in the generic data structures
for SCSI devices and requests.  The SCSI layer takes care of storing
sense data in the SCSIDevice for the subsequent REQUEST SENSE command.

At the same time, get_sense is removed and scsi_req_get_sense can use
an entirely generic implementation.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-bus.c     |   79 +++++++++++++++++++++++++++++++++++++++++++++-------
 hw/scsi-disk.c    |   69 +++++++++++++++-------------------------------
 hw/scsi-generic.c |   76 +++++++++++---------------------------------------
 hw/scsi.h         |   10 +++++-
 trace-events      |    1 +
 5 files changed, 117 insertions(+), 118 deletions(-)
Christoph Hellwig - Aug. 3, 2011, 4:34 p.m.
On Wed, Aug 03, 2011 at 10:49:07AM +0200, Paolo Bonzini wrote:
> With this patch, sense data is stored in the generic data structures
> for SCSI devices and requests.  The SCSI layer takes care of storing
> sense data in the SCSIDevice for the subsequent REQUEST SENSE command.
> 
> At the same time, get_sense is removed and scsi_req_get_sense can use
> an entirely generic implementation.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Looks sensible to me,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Patch

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index bff2f5b..067a615 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -7,6 +7,8 @@ 
 #include "trace.h"
 
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
+static int scsi_build_sense(uint8_t *in_buf, int in_len,
+                            uint8_t *buf, int len, bool fixed);
 
 static struct BusInfo scsi_bus_info = {
     .name  = "SCSI",
@@ -144,6 +146,7 @@  SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
     req->lun = lun;
     req->hba_private = hba_private;
     req->status = -1;
+    req->sense_len = 0;
     trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
     return req;
 }
@@ -161,11 +164,28 @@  uint8_t *scsi_req_get_buf(SCSIRequest *req)
 
 int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len)
 {
-    if (req->dev->info->get_sense) {
-        return req->dev->info->get_sense(req, buf, len);
-    } else {
+    assert(len >= 14);
+    if (!req->sense_len) {
         return 0;
     }
+    return scsi_build_sense(req->sense, req->sense_len, buf, len, true);
+}
+
+int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed)
+{
+    return scsi_build_sense(dev->sense, dev->sense_len, buf, len, fixed);
+}
+
+void scsi_req_build_sense(SCSIRequest *req, SCSISense sense)
+{
+    trace_scsi_req_build_sense(req->dev->id, req->lun, req->tag,
+                               sense.key, sense.asc, sense.ascq);
+    memset(req->sense, 0, 18);
+    req->sense[0] = 0xf0;
+    req->sense[2] = sense.key;
+    req->sense[12] = sense.asc;
+    req->sense[13] = sense.ascq;
+    req->sense_len = 18;
 }
 
 int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
@@ -486,14 +506,40 @@  const struct SCSISense sense_code_LUN_FAILURE = {
 /*
  * scsi_build_sense
  *
- * Build a sense buffer
+ * Convert between fixed and descriptor sense buffers
  */
-int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed)
+int scsi_build_sense(uint8_t *in_buf, int in_len,
+                     uint8_t *buf, int len, bool fixed)
 {
+    bool fixed_in;
+    SCSISense sense;
     if (!fixed && len < 8) {
         return 0;
     }
 
+    if (in_len == 0) {
+        sense.key = NO_SENSE;
+        sense.asc = 0;
+        sense.ascq = 0;
+    } else {
+        fixed_in = (in_buf[0] & 2) == 0;
+
+        if (fixed == fixed_in) {
+            memcpy(buf, in_buf, MIN(len, in_len));
+            return MIN(len, in_len);
+        }
+
+        if (fixed_in) {
+            sense.key = in_buf[2];
+            sense.asc = in_buf[12];
+            sense.ascq = in_buf[13];
+        } else {
+            sense.key = in_buf[1];
+            sense.asc = in_buf[2];
+            sense.ascq = in_buf[3];
+        }
+    }
+
     memset(buf, 0, len);
     if (fixed) {
         /* Return fixed format sense buffer */
@@ -676,6 +722,17 @@  void scsi_req_complete(SCSIRequest *req, int status)
 {
     assert(req->status == -1);
     req->status = status;
+
+    assert(req->sense_len < sizeof(req->sense));
+    if (status == GOOD) {
+        req->sense_len = 0;
+    }
+
+    if (req->sense_len) {
+        memcpy(req->dev->sense, req->sense, req->sense_len);
+    }
+    req->dev->sense_len = req->sense_len;
+
     scsi_req_ref(req);
     scsi_req_dequeue(req);
     req->bus->ops->complete(req, req->status);
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 8beaebf..4c1e58f 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -73,7 +73,6 @@  struct SCSIDiskState
     QEMUBH *bh;
     char *version;
     char *serial;
-    SCSISense sense;
     SCSIDriveKind drive_kind;
 };
 
@@ -100,20 +99,13 @@  static void scsi_free_request(SCSIRequest *req)
     qemu_vfree(r->iov.iov_base);
 }
 
-static void scsi_disk_clear_sense(SCSIDiskState *s)
+/* Helper function for command completion with sense.  */
+static void scsi_check_condition(SCSIDiskReq *r, SCSISense sense)
 {
-    memset(&s->sense, 0, sizeof(s->sense));
-}
-
-/* Helper function for command completion.  */
-static void scsi_command_complete(SCSIDiskReq *r, int status, SCSISense sense)
-{
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
-
     DPRINTF("Command complete tag=0x%x status=%d sense=%d/%d/%d\n",
             r->req.tag, status, sense.key, sense.asc, sense.ascq);
-    s->sense = sense;
-    scsi_req_complete(&r->req, status);
+    scsi_req_build_sense(&r->req, sense);
+    scsi_req_complete(&r->req, CHECK_CONDITION);
 }
 
 /* Cancel a pending data transfer.  */
@@ -165,7 +157,8 @@  static void scsi_read_data(SCSIRequest *req)
     }
     DPRINTF("Read sector_count=%d\n", r->sector_count);
     if (r->sector_count == 0) {
-        scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
+        /* This also clears the sense buffer for REQUEST SENSE.  */
+        scsi_req_complete(&r->req, GOOD);
         return;
     }
 
@@ -213,16 +206,13 @@  static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
     } else {
         switch (error) {
         case ENOMEM:
-            scsi_command_complete(r, CHECK_CONDITION,
-                                  SENSE_CODE(TARGET_FAILURE));
+            scsi_check_condition(r, SENSE_CODE(TARGET_FAILURE));
             break;
         case EINVAL:
-            scsi_command_complete(r, CHECK_CONDITION,
-                                  SENSE_CODE(INVALID_FIELD));
+            scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
             break;
         default:
-            scsi_command_complete(r, CHECK_CONDITION,
-                                  SENSE_CODE(IO_ERROR));
+            scsi_check_condition(r, SENSE_CODE(IO_ERROR));
             break;
         }
         bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read);
@@ -248,7 +238,7 @@  static void scsi_write_complete(void * opaque, int ret)
     r->sector += n;
     r->sector_count -= n;
     if (r->sector_count == 0) {
-        scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
+        scsi_req_complete(&r->req, GOOD);
     } else {
         len = r->sector_count * 512;
         if (len > SCSI_DMA_BUF_SIZE) {
@@ -317,7 +307,7 @@  static void scsi_dma_restart_bh(void *opaque)
             case SCSI_REQ_STATUS_RETRY_FLUSH:
                 ret = scsi_disk_emulate_command(r, r->iov.iov_base);
                 if (ret == 0) {
-                    scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
+                    scsi_req_complete(&r->req, GOOD);
                 }
             }
         }
@@ -345,14 +335,6 @@  static uint8_t *scsi_get_buf(SCSIRequest *req)
     return (uint8_t *)r->iov.iov_base;
 }
 
-/* Copy sense information into the provided buffer */
-static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
-{
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
-
-    return scsi_build_sense(s->sense, outbuf, len, len > 14);
-}
-
 static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
@@ -831,9 +813,8 @@  static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
     case REQUEST_SENSE:
         if (req->cmd.xfer < 4)
             goto illegal_request;
-        buflen = scsi_build_sense(s->sense, outbuf, req->cmd.xfer,
-                                  req->cmd.xfer > 13);
-        scsi_disk_clear_sense(s);
+        buflen = scsi_device_get_sense(&s->qdev, outbuf, req->cmd.xfer,
+                                       (req->cmd.buf[1] & 1) == 0);
         break;
     case INQUIRY:
         buflen = scsi_disk_emulate_inquiry(req, outbuf);
@@ -970,21 +951,21 @@  static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
         }
         break;
     default:
-        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
+        scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
         return -1;
     }
     return buflen;
 
 not_ready:
     if (!bdrv_is_inserted(s->bs)) {
-        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(NO_MEDIUM));
+        scsi_check_condition(r, SENSE_CODE(NO_MEDIUM));
     } else {
-        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(LUN_NOT_READY));
+        scsi_check_condition(r, SENSE_CODE(LUN_NOT_READY));
     }
     return -1;
 
 illegal_request:
-    scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_FIELD));
+    scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
     return -1;
 }
 
@@ -1008,7 +989,7 @@  static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
 
     if (scsi_req_parse(&r->req, buf) != 0) {
         BADF("Unsupported command length, command %x\n", command);
-        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
+        scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
         return 0;
     }
 #ifdef DEBUG_SCSI
@@ -1025,8 +1006,7 @@  static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
         /* Only LUN 0 supported.  */
         DPRINTF("Unimplemented LUN %d\n", req->lun);
         if (command != REQUEST_SENSE && command != INQUIRY) {
-            scsi_command_complete(r, CHECK_CONDITION,
-                                  SENSE_CODE(LUN_NOT_SUPPORTED));
+            scsi_check_condition(r, SENSE_CODE(LUN_NOT_SUPPORTED));
             return 0;
         }
     }
@@ -1135,17 +1115,17 @@  static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
         break;
     default:
         DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
-        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
+        scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
         return 0;
     fail:
-        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_FIELD));
+        scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
         return 0;
     illegal_lba:
-        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(LBA_OUT_OF_RANGE));
+        scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
         return 0;
     }
     if (r->sector_count == 0 && r->iov.iov_len == 0) {
-        scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
+        scsi_req_complete(&r->req, GOOD);
     }
     len = r->sector_count * 512 + r->iov.iov_len;
     if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
@@ -1275,7 +1255,6 @@  static SCSIDeviceInfo scsi_disk_info[] = {
         .write_data   = scsi_write_data,
         .cancel_io    = scsi_cancel_io,
         .get_buf      = scsi_get_buf,
-        .get_sense    = scsi_get_sense,
         .qdev.props   = (Property[]) {
             DEFINE_SCSI_DISK_PROPERTIES(),
             DEFINE_PROP_BIT("removable", SCSIDiskState, removable, 0, false),
@@ -1296,7 +1275,6 @@  static SCSIDeviceInfo scsi_disk_info[] = {
         .write_data   = scsi_write_data,
         .cancel_io    = scsi_cancel_io,
         .get_buf      = scsi_get_buf,
-        .get_sense    = scsi_get_sense,
         .qdev.props   = (Property[]) {
             DEFINE_SCSI_DISK_PROPERTIES(),
             DEFINE_PROP_END_OF_LIST(),
@@ -1316,7 +1294,6 @@  static SCSIDeviceInfo scsi_disk_info[] = {
         .write_data   = scsi_write_data,
         .cancel_io    = scsi_cancel_io,
         .get_buf      = scsi_get_buf,
-        .get_sense    = scsi_get_sense,
         .qdev.props   = (Property[]) {
             DEFINE_SCSI_DISK_PROPERTIES(),
             DEFINE_PROP_BIT("removable", SCSIDiskState, removable, 0, false),
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index f119f33..fc7cf01 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -61,41 +61,8 @@  struct SCSIGenericState
     SCSIDevice qdev;
     BlockDriverState *bs;
     int lun;
-    int driver_status;
-    uint8_t sensebuf[SCSI_SENSE_BUF_SIZE];
-    uint8_t senselen;
 };
 
-static void scsi_set_sense(SCSIGenericState *s, SCSISense sense)
-{
-    s->senselen = scsi_build_sense(sense, s->sensebuf, SCSI_SENSE_BUF_SIZE, 0);
-    s->driver_status = SG_ERR_DRIVER_SENSE;
-}
-
-static void scsi_clear_sense(SCSIGenericState *s)
-{
-    memset(s->sensebuf, 0, SCSI_SENSE_BUF_SIZE);
-    s->senselen = 0;
-    s->driver_status = 0;
-}
-
-static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
-{
-    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
-    int size = SCSI_SENSE_BUF_SIZE;
-
-    if (!(s->driver_status & SG_ERR_DRIVER_SENSE)) {
-        size = scsi_build_sense(SENSE_CODE(NO_SENSE), s->sensebuf,
-                                SCSI_SENSE_BUF_SIZE, 0);
-    }
-    if (size > len) {
-        size = len;
-    }
-    memcpy(outbuf, s->sensebuf, size);
-
-    return size;
-}
-
 static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
                                      void *hba_private)
 {
@@ -117,12 +84,10 @@  static void scsi_command_complete(void *opaque, int ret)
 {
     int status;
     SCSIGenericReq *r = (SCSIGenericReq *)opaque;
-    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, r->req.dev);
 
     r->req.aiocb = NULL;
-    s->driver_status = r->io_header.driver_status;
-    if (s->driver_status & SG_ERR_DRIVER_SENSE)
-        s->senselen = r->io_header.sb_len_wr;
+    if (r->io_header.driver_status & SG_ERR_DRIVER_SENSE)
+        r->req.sense_len = r->io_header.sb_len_wr;
 
     if (ret != 0) {
         switch (ret) {
@@ -131,24 +96,24 @@  static void scsi_command_complete(void *opaque, int ret)
             break;
         case -EINVAL:
             status = CHECK_CONDITION;
-            scsi_set_sense(s, SENSE_CODE(INVALID_FIELD));
+            scsi_req_build_sense(&r->req, SENSE_CODE(INVALID_FIELD));
             break;
         case -ENOMEM:
             status = CHECK_CONDITION;
-            scsi_set_sense(s, SENSE_CODE(TARGET_FAILURE));
+            scsi_req_build_sense(&r->req, SENSE_CODE(TARGET_FAILURE));
             break;
         default:
             status = CHECK_CONDITION;
-            scsi_set_sense(s, SENSE_CODE(IO_ERROR));
+            scsi_req_build_sense(&r->req, SENSE_CODE(IO_ERROR));
             break;
         }
     } else {
-        if (s->driver_status & SG_ERR_DRIVER_TIMEOUT) {
+        if (r->io_header.driver_status & SG_ERR_DRIVER_TIMEOUT) {
             status = BUSY;
             BADF("Driver Timeout\n");
         } else if (r->io_header.status) {
             status = r->io_header.status;
-        } else if (s->driver_status & SG_ERR_DRIVER_SENSE) {
+        } else if (r->io_header.driver_status & SG_ERR_DRIVER_SENSE) {
             status = CHECK_CONDITION;
         } else {
             status = GOOD;
@@ -176,16 +141,14 @@  static int execute_command(BlockDriverState *bdrv,
                            SCSIGenericReq *r, int direction,
 			   BlockDriverCompletionFunc *complete)
 {
-    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, r->req.dev);
-
     r->io_header.interface_id = 'S';
     r->io_header.dxfer_direction = direction;
     r->io_header.dxferp = r->buf;
     r->io_header.dxfer_len = r->buflen;
     r->io_header.cmdp = r->req.cmd.buf;
     r->io_header.cmd_len = r->req.cmd.len;
-    r->io_header.mx_sb_len = sizeof(s->sensebuf);
-    r->io_header.sbp = s->sensebuf;
+    r->io_header.mx_sb_len = sizeof(r->req.sense);
+    r->io_header.sbp = r->req.sense;
     r->io_header.timeout = MAX_UINT;
     r->io_header.usr_ptr = r;
     r->io_header.flags |= SG_FLAG_DIRECT_IO;
@@ -234,21 +197,19 @@  static void scsi_read_data(SCSIRequest *req)
         return;
     }
 
-    if (r->req.cmd.buf[0] == REQUEST_SENSE && s->driver_status & SG_ERR_DRIVER_SENSE)
-    {
-        s->senselen = MIN(r->len, s->senselen);
-        memcpy(r->buf, s->sensebuf, s->senselen);
+    if (r->req.cmd.buf[0] == REQUEST_SENSE) {
         r->io_header.driver_status = 0;
         r->io_header.status = 0;
-        r->io_header.dxfer_len  = s->senselen;
+        r->io_header.dxfer_len =
+            scsi_device_get_sense(&s->qdev, r->buf, r->req.cmd.xfer,
+                                  (r->req.cmd.buf[1] & 1) == 0);
         r->len = -1;
-        DPRINTF("Data ready tag=0x%x len=%d\n", r->req.tag, s->senselen);
+        DPRINTF("Data ready tag=0x%x len=%d\n", r->req.tag, r->io_header.dxfer_len);
         DPRINTF("Sense: %d %d %d %d %d %d %d %d\n",
                 r->buf[0], r->buf[1], r->buf[2], r->buf[3],
                 r->buf[4], r->buf[5], r->buf[6], r->buf[7]);
-        scsi_req_data(&r->req, s->senselen);
-        /* Clear sensebuf after REQUEST_SENSE */
-        scsi_clear_sense(s);
+        scsi_req_data(&r->req, r->io_header.dxfer_len);
+        /* The sense buffer is cleared when we return GOOD */
         return;
     }
 
@@ -342,7 +303,7 @@  static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
 
     if (cmd[0] != REQUEST_SENSE && req->lun != s->lun) {
         DPRINTF("Unimplemented LUN %d\n", req->lun);
-        scsi_set_sense(s, SENSE_CODE(LUN_NOT_SUPPORTED));
+        scsi_req_build_sense(&r->req, SENSE_CODE(LUN_NOT_SUPPORTED));
         scsi_req_complete(&r->req, CHECK_CONDITION);
         return 0;
     }
@@ -533,8 +494,6 @@  static int scsi_generic_initfn(SCSIDevice *dev)
         }
     }
     DPRINTF("block size %d\n", s->qdev.blocksize);
-    s->driver_status = 0;
-    memset(s->sensebuf, 0, sizeof(s->sensebuf));
     bdrv_set_removable(s->bs, 0);
     return 0;
 }
@@ -553,7 +512,6 @@  static SCSIDeviceInfo scsi_generic_info = {
     .write_data   = scsi_write_data,
     .cancel_io    = scsi_cancel_io,
     .get_buf      = scsi_get_buf,
-    .get_sense    = scsi_get_sense,
     .qdev.props   = (Property[]) {
         DEFINE_BLOCK_PROPERTIES(SCSIGenericState, qdev.conf),
         DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/scsi.h b/hw/scsi.h
index 18d3643..c1cb987 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -27,6 +27,8 @@  typedef struct SCSISense {
     uint8_t ascq;
 } SCSISense;
 
+#define SCSI_SENSE_BUF_SIZE 96
+
 struct SCSIRequest {
     SCSIBus           *bus;
     SCSIDevice        *dev;
@@ -42,6 +44,8 @@  struct SCSIRequest {
         enum SCSIXferMode mode;
     } cmd;
     BlockDriverAIOCB  *aiocb;
+    uint8_t sense[SCSI_SENSE_BUF_SIZE];
+    uint32_t sense_len;
     bool enqueued;
     void *hba_private;
     QTAILQ_ENTRY(SCSIRequest) next;
@@ -53,6 +57,8 @@  struct SCSIDevice
     uint32_t id;
     BlockConf conf;
     SCSIDeviceInfo *info;
+    uint8_t sense[SCSI_SENSE_BUF_SIZE];
+    uint32_t sense_len;
     QTAILQ_HEAD(, SCSIRequest) requests;
     int blocksize;
     int type;
@@ -76,7 +82,6 @@  struct SCSIDeviceInfo {
     void (*write_data)(SCSIRequest *req);
     void (*cancel_io)(SCSIRequest *req);
     uint8_t *(*get_buf)(SCSIRequest *req);
-    int (*get_sense)(SCSIRequest *req, uint8_t *buf, int len);
 };
 
 struct SCSIBusOps {
@@ -137,7 +142,6 @@  extern const struct SCSISense sense_code_LUN_FAILURE;
 
 #define SENSE_CODE(x) sense_code_ ## x
 
-int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed);
 int scsi_sense_valid(SCSISense sense);
 
 SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
@@ -149,6 +153,7 @@  void scsi_req_free(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
 void scsi_req_unref(SCSIRequest *req);
 
+void scsi_req_build_sense(SCSIRequest *req, SCSISense sense);
 int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
 void scsi_req_print(SCSIRequest *req);
 void scsi_req_continue(SCSIRequest *req);
@@ -159,5 +164,6 @@  int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len);
 void scsi_req_abort(SCSIRequest *req, int status);
 void scsi_req_cancel(SCSIRequest *req);
 void scsi_device_purge_requests(SCSIDevice *sdev);
+int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed);
 
 #endif
diff --git a/trace-events b/trace-events
index 713f042..55023b0 100644
--- a/trace-events
+++ b/trace-events
@@ -248,6 +248,7 @@  disable scsi_req_continue(int target, int lun, int tag) "target %d lun %d tag %d
 disable scsi_req_parsed(int target, int lun, int tag, int cmd, int mode, int xfer) "target %d lun %d tag %d command %d dir %d length %d"
 disable scsi_req_parsed_lba(int target, int lun, int tag, int cmd, uint64_t lba) "target %d lun %d tag %d command %d lba %"PRIu64""
 disable scsi_req_parse_bad(int target, int lun, int tag, int cmd) "target %d lun %d tag %d command %d"
+disable scsi_req_build_sense(int target, int lun, int tag, int key, int asc, int ascq) "target %d lun %d tag %d key %#02x asc %#02x ascq %#02x"
 
 # vl.c
 disable vm_state_notify(int running, int reason) "running %d reason %d"