diff mbox

[10/16] scsi: Use 'SCSIRequest' directly

Message ID 20101118144728.9581CF90AB@ochil.suse.de
State New
Headers show

Commit Message

Hannes Reinecke Nov. 18, 2010, 2:47 p.m. UTC 
Rather than to access a SCSIRequest via an abstract 'tag' we can
as well use it directly and save us the lookup.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 hw/esp.c          |   20 ++++---
 hw/lsi53c895a.c   |   25 +++++----
 hw/scsi-bus.c     |   15 +-----
 hw/scsi-disk.c    |  147 ++++++++++++++++------------------------------------
 hw/scsi-generic.c |  119 ++++++++++++++-----------------------------
 hw/scsi.h         |   20 ++++---
 hw/usb-msd.c      |   23 +++++----
 7 files changed, 135 insertions(+), 234 deletions(-)

Comments

Gerd Hoffmann Nov. 18, 2010, 4:16 p.m. UTC | #1 
On 11/18/10 15:47, Hannes Reinecke wrote:
>
> Rather than to access a SCSIRequest via an abstract 'tag' we can
> as well use it directly and save us the lookup.

Hmm.  Looks like a few more request handling changes than a pure 
s/tag/req/ + zap lookups sneaked into that patch.  There are new get_req 
and put_req callbacks for example.  At minimum these changes must be 
documented in the commit message.  Even better splitted into separate 
patches.

cheers,
   Gerd
Hannes Reinecke Nov. 18, 2010, 4:33 p.m. UTC | #2 
On 11/18/2010 05:16 PM, Gerd Hoffmann wrote:
> On 11/18/10 15:47, Hannes Reinecke wrote:
>>
>> Rather than to access a SCSIRequest via an abstract 'tag' we can
>> as well use it directly and save us the lookup.
> 
> Hmm.  Looks like a few more request handling changes than a pure
> s/tag/req/ + zap lookups sneaked into that patch.  There are new get_req
> and put_req callbacks for example.  At minimum these changes must be
> documented in the commit message.  Even better splitted into separate
> patches.
> 
Not sure if it makes sense to split it up into several patches;
we need the ->get_req()/->put_req() callbacks to get the request in
the first place. And only then can we modify the other callbacks.
However, splitting them off into two patchsets would render the
first pretty much pointless.

But sure, I can do a better description.

Just tell me which direction you prefer.

Cheers,

Hannes
Gerd Hoffmann Nov. 19, 2010, 8:02 a.m. UTC | #3 
On 11/18/10 17:33, Hannes Reinecke wrote:
> Not sure if it makes sense to split it up into several patches;
> we need the ->get_req()/->put_req() callbacks to get the request in
> the first place.

Point.

> And only then can we modify the other callbacks.
> However, splitting them off into two patchsets would render the
> first pretty much pointless.
>
> But sure, I can do a better description.

Yes, please.

thanks,
   Gerd
Christoph Hellwig Nov. 19, 2010, 6:27 p.m. UTC | #4 
On Thu, Nov 18, 2010 at 03:47:28PM +0100, Hannes Reinecke wrote:
> 
> Rather than to access a SCSIRequest via an abstract 'tag' we can
> as well use it directly and save us the lookup.

The get_put/buf methods are a bit misnamed.  get/put generally implies
refcounting while they are simple alloc/free routines.  I'd suggest
renaming them to alloc_buf/free_buf.

Otherwise the patch looks very good to me.
diff mbox

Patch

diff --git a/hw/esp.c b/hw/esp.c
index 910fd31..a864af3 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -65,6 +65,7 @@  struct ESPState {
     uint32_t dma;
     SCSIBus bus;
     SCSIDevice *current_dev;
+    SCSIRequest *current_req;
     uint8_t cmdbuf[TI_BUFSZ];
     uint32_t cmdlen;
     uint32_t do_cmd;
@@ -209,7 +210,7 @@  static uint32_t get_cmd(ESPState *s, uint8_t *buf)
 
     if (s->current_dev) {
         /* Started a new command before the old one finished.  Cancel it.  */
-        s->current_dev->info->cancel_io(s->current_dev, 0);
+        s->current_dev->info->cancel_io(s->current_req);
         s->async_len = 0;
     }
 
@@ -232,7 +233,8 @@  static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
 
     DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
     lun = busid & 7;
-    datalen = s->current_dev->info->send_command(s->current_dev, 0, buf, lun);
+    s->current_req = s->current_dev->info->get_req(s->current_dev, 0, lun);
+    datalen = s->current_dev->info->send_command(s->current_req, buf);
     s->ti_size = datalen;
     if (datalen != 0) {
         s->rregs[ESP_RSTAT] = STAT_TC;
@@ -240,10 +242,10 @@  static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
         s->dma_counter = 0;
         if (datalen > 0) {
             s->rregs[ESP_RSTAT] |= STAT_DI;
-            s->current_dev->info->read_data(s->current_dev, 0);
+            s->current_dev->info->read_data(s->current_req);
         } else {
             s->rregs[ESP_RSTAT] |= STAT_DO;
-            s->current_dev->info->write_data(s->current_dev, 0);
+            s->current_dev->info->write_data(s->current_req);
         }
     }
     s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
@@ -372,9 +374,9 @@  static void esp_do_dma(ESPState *s)
     if (s->async_len == 0) {
         if (to_device) {
             // ti_size is negative
-            s->current_dev->info->write_data(s->current_dev, 0);
+            s->current_dev->info->write_data(s->current_req);
         } else {
-            s->current_dev->info->read_data(s->current_dev, 0);
+            s->current_dev->info->read_data(s->current_req);
             /* If there is still data to be read from the device then
                complete the DMA operation immediately.  Otherwise defer
                until the scsi layer has completed.  */
@@ -388,7 +390,7 @@  static void esp_do_dma(ESPState *s)
     }
 }
 
-static void esp_command_complete(SCSIBus *bus, int reason, uint32_t tag,
+static void esp_command_complete(SCSIBus *bus, int reason, SCSIRequest *req,
                                  uint32_t arg)
 {
     ESPState *s = DO_UPCAST(ESPState, busdev.qdev, bus->qbus.parent);
@@ -405,11 +407,13 @@  static void esp_command_complete(SCSIBus *bus, int reason, uint32_t tag,
         s->sense = arg;
         s->rregs[ESP_RSTAT] = STAT_ST;
         esp_dma_done(s);
+	req->dev->info->put_req(req);
+	s->current_req = NULL;
         s->current_dev = NULL;
     } else {
         DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
         s->async_len = arg;
-        s->async_buf = s->current_dev->info->get_buf(s->current_dev, 0);
+        s->async_buf = s->current_dev->info->get_buf(req);
         if (s->dma_left) {
             esp_do_dma(s);
         } else if (s->dma_counter != 0 && s->ti_size <= 0) {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 8246ee8..186b87a 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -174,6 +174,7 @@  do { fprintf(stderr, "lsi_scsi: error: " fmt , ## __VA_ARGS__);} while (0)
 #define LSI_TAG_VALID     (1 << 16)
 
 typedef struct lsi_request {
+    SCSIRequest *req;
     uint32_t tag;
     uint32_t dma_len;
     uint8_t *dma_buf;
@@ -569,7 +570,7 @@  static void lsi_do_dma(LSIState *s, int out)
     s->dbc -= count;
 
     if (s->current->dma_buf == NULL) {
-        s->current->dma_buf = dev->info->get_buf(dev, s->current->tag);
+        s->current->dma_buf = dev->info->get_buf(s->current->req);
     }
 
     /* ??? Set SFBR to first data byte.  */
@@ -583,10 +584,10 @@  static void lsi_do_dma(LSIState *s, int out)
         s->current->dma_buf = NULL;
         if (out) {
             /* Write the data.  */
-            dev->info->write_data(dev, s->current->tag);
+            dev->info->write_data(s->current->req);
         } else {
             /* Request any remaining data.  */
-            dev->info->read_data(dev, s->current->tag);
+            dev->info->read_data(s->current->req);
         }
     } else {
         s->current->dma_buf += count;
@@ -687,7 +688,7 @@  static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
 }
 
 /* Callback to indicate that the SCSI layer has completed a transfer.  */
-static void lsi_command_complete(SCSIBus *bus, int reason, uint32_t tag,
+static void lsi_command_complete(SCSIBus *bus, int reason, SCSIRequest *req,
                                  uint32_t arg)
 {
     LSIState *s = DO_UPCAST(LSIState, dev.qdev, bus->qbus.parent);
@@ -704,7 +705,8 @@  static void lsi_command_complete(SCSIBus *bus, int reason, uint32_t tag,
         } else {
             lsi_set_phase(s, PHASE_ST);
         }
-
+        req->dev->info->put_req(req);
+        s->current->req = NULL;
         qemu_free(s->current);
         s->current = NULL;
 
@@ -712,14 +714,14 @@  static void lsi_command_complete(SCSIBus *bus, int reason, uint32_t tag,
         return;
     }
 
-    if (s->waiting == 1 || !s->current || tag != s->current->tag ||
+    if (s->waiting == 1 || !s->current || req->tag != s->current->tag ||
         (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON))) {
-        if (lsi_queue_tag(s, tag, arg))
+        if (lsi_queue_tag(s, req->tag, arg))
             return;
     }
 
     /* host adapter (re)connected */
-    DPRINTF("Data ready tag=0x%x len=%d\n", tag, arg);
+    DPRINTF("Data ready tag=0x%x len=%d\n", req->tag, arg);
     s->current->dma_len = arg;
     s->command_complete = 1;
     if (!s->waiting)
@@ -755,14 +757,15 @@  static void lsi_do_command(LSIState *s)
     assert(s->current == NULL);
     s->current = qemu_mallocz(sizeof(lsi_request));
     s->current->tag = s->select_tag;
+    s->current->req = dev->info->get_req(dev, s->current->tag, s->current_lun);
 
-    n = dev->info->send_command(dev, s->current->tag, buf, s->current_lun);
+    n = dev->info->send_command(s->current->req, buf);
     if (n > 0) {
         lsi_set_phase(s, PHASE_DI);
-        dev->info->read_data(dev, s->current->tag);
+        dev->info->read_data(s->current->req);
     } else if (n < 0) {
         lsi_set_phase(s, PHASE_DO);
-        dev->info->write_data(dev, s->current->tag);
+        dev->info->write_data(s->current->req);
     }
 
     if (!s->command_complete) {
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index afdf0ad..bb88a56 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -138,18 +138,6 @@  SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
     return req;
 }
 
-SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag)
-{
-    SCSIRequest *req;
-
-    QTAILQ_FOREACH(req, &d->requests, next) {
-        if (req->tag == tag) {
-            return req;
-        }
-    }
-    return NULL;
-}
-
 static void scsi_req_dequeue(SCSIRequest *req)
 {
     if (req->enqueued) {
@@ -607,6 +595,5 @@  void scsi_req_complete(SCSIRequest *req)
     assert(req->status != -1);
     scsi_req_dequeue(req);
     req->bus->complete(req->bus, SCSI_REASON_DONE,
-                       req->tag,
-                       req->status);
+                       req, req->status);
 }
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 2d2e770..fa7b29f 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -81,23 +81,26 @@  struct SCSIDiskState
 static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
 static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf);
 
-static SCSIDiskReq *scsi_new_request(SCSIDiskState *s, uint32_t tag,
+static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
         uint32_t lun)
 {
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
     SCSIRequest *req;
     SCSIDiskReq *r;
 
-    req = scsi_req_alloc(sizeof(SCSIDiskReq), &s->qdev, tag, lun);
+    req = scsi_req_alloc(sizeof(SCSIDiskReq), d, tag, lun);
     r = DO_UPCAST(SCSIDiskReq, req, req);
     r->iov_buf = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE);
     r->iov = qemu_mallocz(sizeof(struct iovec));
     r->iov[0].iov_base = r->iov_buf;
     r->iov_num = 1;
-    return r;
+    return req;
 }
 
-static void scsi_remove_request(SCSIDiskReq *r)
+static void scsi_remove_request(SCSIRequest *req)
 {
+    SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
+
     qemu_vfree(r->iov);
     r->iov = NULL;
     qemu_vfree(r->iov_buf);
@@ -105,11 +108,6 @@  static void scsi_remove_request(SCSIDiskReq *r)
     scsi_req_free(&r->req);
 }
 
-static SCSIDiskReq *scsi_find_request(SCSIDiskState *s, uint32_t tag)
-{
-    return DO_UPCAST(SCSIDiskReq, req, scsi_req_find(&s->qdev, tag));
-}
-
 static void scsi_disk_clear_sense(SCSIDiskState *s)
 {
     memset(&s->sense, 0, sizeof(s->sense));
@@ -141,22 +139,17 @@  static void scsi_command_complete(SCSIDiskReq *r, int status, SCSISense sense)
             r->req.tag, status, sense.key, sense.asc, sense.ascq);
     scsi_req_set_status(r, status, sense);
     scsi_req_complete(&r->req);
-    scsi_remove_request(r);
 }
 
 /* Cancel a pending data transfer.  */
-static void scsi_cancel_io(SCSIDevice *d, uint32_t tag)
+static void scsi_cancel_io(SCSIRequest *req)
 {
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
-    SCSIDiskReq *r;
-    DPRINTF("Cancel tag=0x%x\n", tag);
-    r = scsi_find_request(s, tag);
-    if (r) {
-        if (r->req.aiocb)
-            bdrv_aio_cancel(r->req.aiocb);
-        r->req.aiocb = NULL;
-        scsi_remove_request(r);
-    }
+    SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
+
+    DPRINTF("Cancel tag=0x%x\n", req->tag);
+    if (r->req.aiocb)
+        bdrv_aio_cancel(r->req.aiocb);
+    r->req.aiocb = NULL;
 }
 
 static void scsi_read_complete(void * opaque, int ret)
@@ -175,19 +168,24 @@  static void scsi_read_complete(void * opaque, int ret)
 
     DPRINTF("Data ready tag=0x%x len=%zd\n", r->req.tag, iov_len);
 
-    r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, iov_len);
+    r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, &r->req, iov_len);
 }
 
 
-static void scsi_read_request(SCSIDiskReq *r)
+/* Read more data from scsi device into buffer.  */
+static void scsi_read_data(SCSIRequest *req)
 {
+    SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     uint32_t n;
 
+    /* No data transfer may already be in progress */
+    assert(r->req.aiocb == NULL);
+
     if (r->sector_count == (uint32_t)-1) {
         DPRINTF("Read buf_len=%zd\n", r->iov[0].iov_len);
         r->sector_count = 0;
-        r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag,
+        r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, &r->req,
                              r->iov[0].iov_len);
         return;
     }
@@ -216,29 +214,6 @@  static void scsi_read_request(SCSIDiskReq *r)
     }
 }
 
-/* Read more data from scsi device into buffer.  */
-static void scsi_read_data(SCSIDevice *d, uint32_t tag)
-{
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
-    SCSIDiskReq *r;
-
-    r = scsi_find_request(s, tag);
-    if (!r) {
-        SCSIBus *bus;
-
-        BADF("Bad read tag 0x%x\n", tag);
-        bus = scsi_bus_from_device(d);
-        s->sense = SENSE_CODE(I_T_NEXUS_LOSS);
-        bus->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
-        return;
-    }
-
-    /* No data transfer may already be in progress */
-    assert(r->req.aiocb == NULL);
-
-    scsi_read_request(r);
-}
-
 static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
 {
     int is_read = (type == SCSI_REQ_STATUS_RETRY_READ);
@@ -260,7 +235,7 @@  static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
         vm_stop(0);
     } else {
         if (type == SCSI_REQ_STATUS_RETRY_READ) {
-            r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, 0);
+            r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, &r->req, 0);
         }
         if (error == EBADR) {
                 scsi_command_complete(r, CHECK_CONDITION,
@@ -304,15 +279,21 @@  static void scsi_write_complete(void * opaque, int ret)
             r->iov[0].iov_len = len;
         }
         DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, len);
-        r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, len);
+        r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, &r->req, len);
     }
 }
 
-static void scsi_write_request(SCSIDiskReq *r)
+/* Write data to a scsi device.  Returns nonzero on failure.
+   The transfer may complete asynchronously.  */
+static int scsi_write_data(SCSIRequest *req)
 {
+    SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     uint32_t n;
 
+    /* No data transfer may already be in progress */
+    assert(r->req.aiocb == NULL);
+
     n = scsi_req_iov_len(r) / 512;
     if (n) {
         qemu_iovec_init_external(&r->qiov, r->iov, r->iov_num);
@@ -325,31 +306,6 @@  static void scsi_write_request(SCSIDiskReq *r)
         /* Invoke completion routine to fetch data from host.  */
         scsi_write_complete(r, 0);
     }
-}
-
-/* Write data to a scsi device.  Returns nonzero on failure.
-   The transfer may complete asynchronously.  */
-static int scsi_write_data(SCSIDevice *d, uint32_t tag)
-{
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
-    SCSIDiskReq *r;
-
-    DPRINTF("Write data tag=0x%x\n", tag);
-    r = scsi_find_request(s, tag);
-    if (!r) {
-        SCSIBus *bus;
-
-        BADF("Bad write tag 0x%x\n", tag);
-        bus = scsi_bus_from_device(d);
-        s->sense = SENSE_CODE(I_T_NEXUS_LOSS);
-        bus->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
-        return 1;
-    }
-
-    /* No data transfer may already be in progress */
-    assert(r->req.aiocb == NULL);
-
-    scsi_write_request(r);
 
     return 0;
 }
@@ -374,10 +330,10 @@  static void scsi_dma_restart_bh(void *opaque)
 
             switch (status & SCSI_REQ_STATUS_RETRY_TYPE_MASK) {
             case SCSI_REQ_STATUS_RETRY_READ:
-                scsi_read_request(r);
+                scsi_read_data(&r->req);
                 break;
             case SCSI_REQ_STATUS_RETRY_WRITE:
-                scsi_write_request(r);
+                scsi_write_data(&r->req);
                 break;
             case SCSI_REQ_STATUS_RETRY_FLUSH:
                 ret = scsi_disk_emulate_command(r, r->iov[0].iov_base);
@@ -403,16 +359,10 @@  static void scsi_dma_restart_cb(void *opaque, int running, int reason)
 }
 
 /* Return a pointer to the data buffer.  */
-static uint8_t *scsi_get_buf(SCSIDevice *d, uint32_t tag)
+static uint8_t *scsi_get_buf(SCSIRequest *req)
 {
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
-    SCSIDiskReq *r;
+    SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
 
-    r = scsi_find_request(s, tag);
-    if (!r) {
-        BADF("Bad buffer tag 0x%x\n", tag);
-        return NULL;
-    }
     return r->iov_buf;
 }
 
@@ -1025,24 +975,15 @@  illegal_request:
    (eg. disk reads), negative for transfers to the device (eg. disk writes),
    and zero if the command does not transfer any data.  */
 
-static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
-                                 uint8_t *buf, int lun)
+static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
 {
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+    SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
     ssize_t len = 0;
     int is_write;
     uint8_t command;
-    SCSIDiskReq *r;
 
     command = buf[0];
-    r = scsi_find_request(s, tag);
-    if (r) {
-        BADF("Tag 0x%x already in use\n", tag);
-        scsi_cancel_io(d, tag);
-    }
-    /* ??? Tags are not unique for different luns.  We only implement a
-       single lun, so this should not matter.  */
-    r = scsi_new_request(s, tag, lun);
     is_write = 0;
     DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, tag, buf[0]);
 
@@ -1061,9 +1002,9 @@  static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     }
 #endif
 
-    if (lun || buf[1] >> 5) {
+    if (req->lun || buf[1] >> 5) {
         /* Only LUN 0 supported.  */
-        DPRINTF("Unimplemented LUN %d\n", lun ? lun : buf[1] >> 5);
+        DPRINTF("Unimplemented LUN %d\n", req->lun ? req->lun : buf[1] >> 5);
         if (command != REQUEST_SENSE && command != INQUIRY) {
             scsi_command_complete(r, CHECK_CONDITION,
                                   SENSE_CODE(LUN_NOT_SUPPORTED));
@@ -1101,7 +1042,7 @@  static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     case READ_10:
     case READ_12:
     case READ_16:
-        r->sector_count = r->req.cmd.xfer / d->blocksize * s->cluster_size;
+        r->sector_count = r->req.cmd.xfer / s->qdev.blocksize * s->cluster_size;
         DPRINTF("Read (sector %" PRId64 ", blocks %d)\n", r->req.cmd.lba,
                 r->sector_count);
         if (r->req.cmd.lba > s->max_lba) {
@@ -1117,7 +1058,7 @@  static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     case WRITE_VERIFY:
     case WRITE_VERIFY_12:
     case WRITE_VERIFY_16:
-        r->sector_count = r->req.cmd.xfer / d->blocksize * s->cluster_size;
+        r->sector_count = r->req.cmd.xfer / s->qdev.blocksize * s->cluster_size;
         DPRINTF("Write %s(sector %" PRId64 ", blocks %d)\n",
                 (command & 0xe) == 0xe ? "And Verify " : "",
                 r->req.cmd.lba, r->sector_count);
@@ -1185,7 +1126,7 @@  static void scsi_disk_purge_requests(SCSIDiskState *s)
         if (r->req.aiocb) {
             bdrv_aio_cancel(r->req.aiocb);
         }
-        scsi_remove_request(r);
+        scsi_remove_request(&r->req);
     }
 }
 
@@ -1266,6 +1207,8 @@  static SCSIDeviceInfo scsi_disk_info = {
     .qdev.reset   = scsi_disk_reset,
     .init         = scsi_disk_initfn,
     .destroy      = scsi_destroy,
+    .get_req      = scsi_new_request,
+    .put_req      = scsi_remove_request,
     .send_command = scsi_send_command,
     .read_data    = scsi_read_data,
     .write_data   = scsi_write_data,
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 6311232..de37d78 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -83,25 +83,22 @@  static void scsi_clear_sense(SCSIGenericState *s)
     s->driver_status = 0;
 }
 
-static SCSIGenericReq *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
+static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
 {
     SCSIRequest *req;
 
     req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun);
-    return DO_UPCAST(SCSIGenericReq, req, req);
+    return req;
 }
 
-static void scsi_remove_request(SCSIGenericReq *r)
+static void scsi_remove_request(SCSIRequest *req)
 {
+    SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
+
     qemu_free(r->buf);
     scsi_req_free(&r->req);
 }
 
-static SCSIGenericReq *scsi_find_request(SCSIGenericState *s, uint32_t tag)
-{
-    return DO_UPCAST(SCSIGenericReq, req, scsi_req_find(&s->qdev, tag));
-}
-
 /* Helper function for command completion.  */
 static void scsi_command_complete(void *opaque, int ret)
 {
@@ -114,8 +111,11 @@  static void scsi_command_complete(void *opaque, int ret)
 
     if (ret != 0) {
         switch(ret) {
+            case ENODEV:
+                s->senselen = scsi_set_sense(s, SENSE_CODE(LUN_NOT_SUPPORTED));
+                break;
             case EINVAL:
-                s->senselen = scsi_set_sense(s, SENSE_CODE(TARGET_FAILURE));
+                s->senselen = scsi_set_sense(s, SENSE_CODE(INVALID_FIELD));
                 break;
             case EBADR:
                 s->senselen = scsi_set_sense(s, SENSE_CODE(TARGET_FAILURE));
@@ -142,23 +142,17 @@  static void scsi_command_complete(void *opaque, int ret)
             r, r->req.tag, r->req.status);
 
     scsi_req_complete(&r->req);
-    scsi_remove_request(r);
 }
 
 /* Cancel a pending data transfer.  */
-static void scsi_cancel_io(SCSIDevice *d, uint32_t tag)
+static void scsi_cancel_io(SCSIRequest *req)
 {
-    DPRINTF("scsi_cancel_io 0x%x\n", tag);
-    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, d);
-    SCSIGenericReq *r;
-    DPRINTF("Cancel tag=0x%x\n", tag);
-    r = scsi_find_request(s, tag);
-    if (r) {
-        if (r->req.aiocb)
-            bdrv_aio_cancel(r->req.aiocb);
-        r->req.aiocb = NULL;
-        scsi_remove_request(r);
-    }
+    SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
+
+    DPRINTF("Cancel tag=0x%x\n", req->tag);
+    if (r->req.aiocb)
+        bdrv_aio_cancel(r->req.aiocb);
+    r->req.aiocb = NULL;
 }
 
 static int execute_command(BlockDriverState *bdrv,
@@ -202,30 +196,19 @@  static void scsi_read_complete(void * opaque, int ret)
     DPRINTF("Data ready tag=0x%x len=%d\n", r->req.tag, len);
 
     r->len = -1;
-    r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, len);
+    r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, &r->req, len);
     if (len == 0)
         scsi_command_complete(r, 0);
 }
 
 /* Read more data from scsi device into buffer.  */
-static void scsi_read_data(SCSIDevice *d, uint32_t tag)
+static void scsi_read_data(SCSIRequest *req)
 {
-    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, d);
-    SCSIGenericReq *r;
+    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
+    SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
     int ret;
 
-    DPRINTF("scsi_read_data 0x%x\n", tag);
-    r = scsi_find_request(s, tag);
-    if (!r) {
-        SCSIBus *bus;
-
-        BADF("Bad read tag 0x%x\n", tag);
-        scsi_set_sense(s, SENSE_CODE(I_T_NEXUS_LOSS));
-        bus = scsi_bus_from_device(d);
-        bus->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
-        return;
-    }
-
+    DPRINTF("scsi_read_data 0x%x\n", req->tag);
     if (r->len == -1) {
         scsi_command_complete(r, 0);
         return;
@@ -243,7 +226,7 @@  static void scsi_read_data(SCSIDevice *d, uint32_t tag)
         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]);
-        r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, s->senselen);
+        r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, &r->req, s->senselen);
         /* Clear sensebuf after REQUEST_SENSE */
         scsi_clear_sense(s);
         return;
@@ -279,27 +262,17 @@  static void scsi_write_complete(void * opaque, int ret)
 
 /* Write data to a scsi device.  Returns nonzero on failure.
    The transfer may complete asynchronously.  */
-static int scsi_write_data(SCSIDevice *d, uint32_t tag)
+static int scsi_write_data(SCSIRequest *req)
 {
-    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, d);
-    SCSIGenericReq *r;
+    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
+    SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
     int ret;
 
-    DPRINTF("scsi_write_data 0x%x\n", tag);
-    r = scsi_find_request(s, tag);
-    if (!r) {
-        SCSIBus *bus;
-
-        BADF("Bad write tag 0x%x\n", tag);
-        scsi_set_sense(s, SENSE_CODE(I_T_NEXUS_LOSS));
-        bus = scsi_bus_from_device(d);
-        bus->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
-        return 0;
-    }
+    DPRINTF("scsi_write_data 0x%x\n", req->tag);
 
     if (r->len == 0) {
         r->len = r->buflen;
-        r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, r->len);
+        r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, &r->req, r->len);
         return 0;
     }
 
@@ -313,15 +286,10 @@  static int scsi_write_data(SCSIDevice *d, uint32_t tag)
 }
 
 /* Return a pointer to the data buffer.  */
-static uint8_t *scsi_get_buf(SCSIDevice *d, uint32_t tag)
+static uint8_t *scsi_get_buf(SCSIRequest *req)
 {
-    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, d);
-    SCSIGenericReq *r;
-    r = scsi_find_request(s, tag);
-    if (!r) {
-        BADF("Bad buffer tag 0x%x\n", tag);
-        return NULL;
-    }
+    SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
+
     return r->buf;
 }
 
@@ -349,31 +317,20 @@  static void scsi_req_fixup(SCSIRequest *req)
    (eg. disk reads), negative for transfers to the device (eg. disk writes),
    and zero if the command does not transfer any data.  */
 
-static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
-                                 uint8_t *cmd, int lun)
+static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
 {
-    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, d);
-    SCSIGenericReq *r;
-    SCSIBus *bus;
+    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
+    SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
     int ret;
 
     if (cmd[0] != REQUEST_SENSE &&
-        (lun != s->lun || (cmd[1] >> 5) != s->lun)) {
-        DPRINTF("Unimplemented LUN %d\n", lun ? lun : cmd[1] >> 5);
+        (req->lun != s->lun || (cmd[1] >> 5) != s->lun)) {
+        DPRINTF("Unimplemented LUN %d\n", req->lun ? req->lun : cmd[1] >> 5);
 
-        scsi_set_sense(s, SENSE_CODE(LUN_NOT_SUPPORTED));
-        bus = scsi_bus_from_device(d);
-        bus->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
+        scsi_command_complete(r, -ENODEV);
         return 0;
     }
 
-    r = scsi_find_request(s, tag);
-    if (r) {
-        BADF("Tag 0x%x already in use %p\n", tag, r);
-        scsi_cancel_io(d, tag);
-    }
-    r = scsi_new_request(d, tag, lun);
-
     if (-1 == scsi_req_parse(&r->req, cmd)) {
         BADF("Unsupported command length, command %x\n", cmd[0]);
         scsi_command_complete(r, -EINVAL);
@@ -494,7 +451,7 @@  static void scsi_generic_purge_requests(SCSIGenericState *s)
         if (r->req.aiocb) {
             bdrv_aio_cancel(r->req.aiocb);
         }
-        scsi_remove_request(r);
+        scsi_remove_request(&r->req);
     }
 }
 
@@ -586,6 +543,8 @@  static SCSIDeviceInfo scsi_generic_info = {
     .qdev.reset   = scsi_generic_reset,
     .init         = scsi_generic_initfn,
     .destroy      = scsi_destroy,
+    .get_req      = scsi_new_request,
+    .put_req      = scsi_remove_request,
     .send_command = scsi_send_command,
     .read_data    = scsi_read_data,
     .write_data   = scsi_write_data,
diff --git a/hw/scsi.h b/hw/scsi.h
index 1196122..fbef736 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -17,7 +17,8 @@  enum scsi_reason {
 typedef struct SCSIBus SCSIBus;
 typedef struct SCSIDevice SCSIDevice;
 typedef struct SCSIDeviceInfo SCSIDeviceInfo;
-typedef void (*scsi_completionfn)(SCSIBus *bus, int reason, uint32_t tag,
+typedef struct SCSIRequest SCSIRequest;
+typedef void (*scsi_completionfn)(SCSIBus *bus, int reason, SCSIRequest *req,
                                   uint32_t arg);
 
 enum SCSIXferMode {
@@ -32,7 +33,7 @@  typedef struct SCSISense {
     uint8_t ascq;
 } SCSISense;
 
-typedef struct SCSIRequest {
+struct SCSIRequest {
     SCSIBus           *bus;
     SCSIDevice        *dev;
     uint32_t          tag;
@@ -48,7 +49,7 @@  typedef struct SCSIRequest {
     BlockDriverAIOCB  *aiocb;
     bool enqueued;
     QTAILQ_ENTRY(SCSIRequest) next;
-} SCSIRequest;
+};
 
 struct SCSIDevice
 {
@@ -71,12 +72,13 @@  struct SCSIDeviceInfo {
     DeviceInfo qdev;
     scsi_qdev_initfn init;
     void (*destroy)(SCSIDevice *s);
-    int32_t (*send_command)(SCSIDevice *s, uint32_t tag, uint8_t *buf,
-                            int lun);
-    void (*read_data)(SCSIDevice *s, uint32_t tag);
-    int (*write_data)(SCSIDevice *s, uint32_t tag);
-    void (*cancel_io)(SCSIDevice *s, uint32_t tag);
-    uint8_t *(*get_buf)(SCSIDevice *s, uint32_t tag);
+    SCSIRequest *(*get_req)(SCSIDevice *s, uint32_t tag, uint32_t lun);
+    void (*put_req)(SCSIRequest *req);
+    int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
+    void (*read_data)(SCSIRequest *req);
+    int (*write_data)(SCSIRequest *req);
+    void (*cancel_io)(SCSIRequest *req);
+    uint8_t *(*get_buf)(SCSIRequest *req);
 };
 
 typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 0a95d8d..ad7d9a7 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -47,6 +47,7 @@  typedef struct {
     uint32_t data_len;
     uint32_t residue;
     uint32_t tag;
+    SCSIRequest *req;
     SCSIBus bus;
     BlockConf conf;
     SCSIDevice *scsi_dev;
@@ -155,9 +156,9 @@  static void usb_msd_copy_data(MSDState *s)
     s->data_len -= len;
     if (s->scsi_len == 0) {
         if (s->mode == USB_MSDM_DATAIN) {
-            s->scsi_dev->info->read_data(s->scsi_dev, s->tag);
+            s->scsi_dev->info->read_data(s->req);
         } else if (s->mode == USB_MSDM_DATAOUT) {
-            s->scsi_dev->info->write_data(s->scsi_dev, s->tag);
+            s->scsi_dev->info->write_data(s->req);
         }
     }
 }
@@ -173,14 +174,14 @@  static void usb_msd_send_status(MSDState *s)
     memcpy(s->usb_buf, &csw, 13);
 }
 
-static void usb_msd_command_complete(SCSIBus *bus, int reason, uint32_t tag,
+static void usb_msd_command_complete(SCSIBus *bus, int reason, SCSIRequest *req,
                                      uint32_t arg)
 {
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, bus->qbus.parent);
     USBPacket *p = s->packet;
 
-    if (tag != s->tag) {
-        fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", tag);
+    if (req->tag != s->tag) {
+        fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
     }
     if (reason == SCSI_REASON_DONE) {
         DPRINTF("Command complete %d\n", arg);
@@ -207,10 +208,11 @@  static void usb_msd_command_complete(SCSIBus *bus, int reason, uint32_t tag,
         } else if (s->data_len == 0) {
             s->mode = USB_MSDM_CSW;
         }
+        s->scsi_dev->info->put_req(req);
         return;
     }
     s->scsi_len = arg;
-    s->scsi_buf = s->scsi_dev->info->get_buf(s->scsi_dev, tag);
+    s->scsi_buf = s->scsi_dev->info->get_buf(req);
     if (p) {
         usb_msd_copy_data(s);
         if (s->usb_len == 0) {
@@ -348,7 +350,7 @@  static int usb_msd_handle_control(USBDevice *dev, int request, int value,
 static void usb_msd_cancel_io(USBPacket *p, void *opaque)
 {
     MSDState *s = opaque;
-    s->scsi_dev->info->cancel_io(s->scsi_dev, s->tag);
+    s->scsi_dev->info->cancel_io(s->req);
     s->packet = NULL;
     s->scsi_len = 0;
 }
@@ -396,14 +398,15 @@  static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
             DPRINTF("Command tag 0x%x flags %08x len %d data %d\n",
                     s->tag, cbw.flags, cbw.cmd_len, s->data_len);
             s->residue = 0;
-            s->scsi_dev->info->send_command(s->scsi_dev, s->tag, cbw.cmd, 0);
+            s->req = s->scsi_dev->info->get_req(s->scsi_dev, s->tag, 0);
+            s->scsi_dev->info->send_command(s->req, cbw.cmd);
             /* ??? Should check that USB and SCSI data transfer
                directions match.  */
             if (s->residue == 0) {
                 if (s->mode == USB_MSDM_DATAIN) {
-                    s->scsi_dev->info->read_data(s->scsi_dev, s->tag);
+                    s->scsi_dev->info->read_data(s->req);
                 } else if (s->mode == USB_MSDM_DATAOUT) {
-                    s->scsi_dev->info->write_data(s->scsi_dev, s->tag);
+                    s->scsi_dev->info->write_data(s->req);
                 }
             }
             ret = len;