All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements
@ 2011-05-23 16:08 Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 01/24] scsi: add tracing of scsi requests Paolo Bonzini
                   ` (23 more replies)
  0 siblings, 24 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

This series includes the following improvements to the SCSI subsystem:

1) introduction of SCSIBusOps that generalize the existing
command_complete callback;

2) widespread use of the SCSIRequest abstraction, with simpler memory
management (refcounting) and with various common idioms converted into
simple C functions instead of duplicating them all over the place;

3) support for autosense.

Some patches are from Hannes Reinecke's megasas patchset posted last
November, forward ported and applied to the new vSCSI controller as
well.

I already planned the following two series too:

1) adding support for zerocopy.  Previous attempts were rejected
because they were applied to real devices (thus making for example an
IOMMU hard to impossible).  However, for PV devices zerocopy should be
uncontroversial---and it is a must to get competitive performance WRT
virtio-blk.  I'll use vmw-pvscsi for the first implementation and for
benchmarking.

2) adding support for multiple LUNs; see recently posted RFC for this.

After this I'll work on the virtio-scsi device model.

Testing:
- RHEL6.1 install complete to scsi-disk with lsi, from scsi-generic CD
- iozone run with lsi on scsi-disk target
- RHEL6.1 install to usb-msd from IDE CD is too slow, but it manages to
  format /boot in ~10 minutes with or without the patches
- RHEL6.1 install with vscsi with scsi-disk CD and installation disk
  hung at the same place with or without the series

esp is only compile tested.

Please review and apply.

v3->v4:
    renamed scsi_req_kick to scsi_req_continue.  Changed SCSIBusOps to
    pointer.  Removed scsi_req_free, merged it into scsi_req_unref.
    Added head comments for some functions.  Renamed arguments to
    HBA callbacks.  Added patches 23 and 24

v2->v3:
    included fixes for Jonathan Nieder's recently reported bug

v1->v2:
    rebased, added patch 21

Hannes Reinecke (4):
  scsi: Use 'SCSIRequest' directly
  scsi: Update sense code handling
  scsi: Implement 'get_sense' callback
  scsi-disk: add data direction checking

Paolo Bonzini (20):
  scsi: add tracing of scsi requests
  scsi-generic: Remove bogus double complete
  scsi: introduce scsi_req_data
  scsi: introduce SCSIBusOps
  scsi: reference-count requests
  lsi: extract lsi_find_by_tag
  scsi: commonize purging requests
  scsi: introduce scsi_req_abort
  scsi: introduce scsi_req_cancel
  scsi: use scsi_req_complete
  scsi: do not call send_command directly
  scsi: introduce scsi_req_new
  scsi: introduce scsi_req_continue
  scsi: introduce scsi_req_get_buf
  scsi: make write_data return void
  scsi-generic: Handle queue full
  esp: rename sense to status
  scsi: split command_complete callback in two
  scsi: rename arguments to the new callbacks
  scsi: ignore LUN field in the CDB

 hw/esp.c          |  119 ++++++++++++++----------
 hw/lsi53c895a.c   |  188 +++++++++++++++++++++++--------------
 hw/scsi-bus.c     |  214 +++++++++++++++++++++++++++++++++++++++----
 hw/scsi-disk.c    |  265 ++++++++++++++++++++++-------------------------------
 hw/scsi-generic.c |  220 +++++++++++++++++++++-----------------------
 hw/scsi.h         |   91 ++++++++++++++----
 hw/spapr_vscsi.c  |  185 +++++++++++++++++++++----------------
 hw/usb-msd.c      |  120 ++++++++++++++----------
 trace-events      |    8 ++
 9 files changed, 843 insertions(+), 567 deletions(-)

-- 
1.7.4.4

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 01/24] scsi: add tracing of scsi requests
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 02/24] scsi-generic: Remove bogus double complete Paolo Bonzini
                   ` (22 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-bus.c |    6 ++++++
 trace-events  |    6 ++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index ceeb4ec..0fd85fc 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -4,6 +4,7 @@
 #include "scsi-defs.h"
 #include "qdev.h"
 #include "blockdev.h"
+#include "trace.h"
 
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
 
@@ -141,6 +142,7 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
     req->lun = lun;
     req->status = -1;
     req->enqueued = true;
+    trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
     QTAILQ_INSERT_TAIL(&d->requests, req, next);
     return req;
 }
@@ -159,6 +161,7 @@ SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag)
 
 static void scsi_req_dequeue(SCSIRequest *req)
 {
+    trace_scsi_req_dequeue(req->dev->id, req->lun, req->tag);
     if (req->enqueued) {
         QTAILQ_REMOVE(&req->dev->requests, req, next);
         req->enqueued = false;
@@ -195,6 +198,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
         req->cmd.len = 12;
         break;
     default:
+        trace_scsi_req_parse_bad(req->dev->id, req->lun, req->tag, cmd[0]);
         return -1;
     }
 
@@ -392,6 +396,8 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf)
     memcpy(req->cmd.buf, buf, req->cmd.len);
     scsi_req_xfer_mode(req);
     req->cmd.lba = scsi_req_lba(req);
+    trace_scsi_req_parsed(req->dev->id, req->lun, req->tag, buf[0],
+                          req->cmd.mode, req->cmd.xfer, req->cmd.lba);
     return 0;
 }
 
diff --git a/trace-events b/trace-events
index a00b63c..55f89b4 100644
--- a/trace-events
+++ b/trace-events
@@ -205,6 +205,12 @@ disable usb_set_config(int addr, int config, int ret) "dev %d, config %d, ret %d
 disable usb_clear_device_feature(int addr, int feature, int ret) "dev %d, feature %d, ret %d"
 disable usb_set_device_feature(int addr, int feature, int ret) "dev %d, feature %d, ret %d"
 
+# hw/scsi-bus.c
+disable scsi_req_alloc(int target, int lun, int tag) "target %d lun %d tag %d"
+disable scsi_req_dequeue(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, uint64_t lba) "target %d lun %d tag %d command %d dir %d length %d lba %"PRIu64""
+disable scsi_req_parse_bad(int target, int lun, int tag, int cmd) "target %d lun %d tag %d command %d"
+
 # vl.c
 disable vm_state_notify(int running, int reason) "running %d reason %d"
 
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 02/24] scsi-generic: Remove bogus double complete
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 01/24] scsi: add tracing of scsi requests Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 03/24] scsi: introduce scsi_req_data Paolo Bonzini
                   ` (21 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch, David Gibson

scsi-generic scsi_read_complete() should not -both- call the client
complete callback with SCSI_REASON_DATA -and- call
scsi_command_complete().  The former will cause the client to queue a
new read or write request, while the later will free the request data
structure, thus causing the new read or write request to use a
freed/stale structure when it completes.

This patch fixes the bug, fixing a crash with scsi-generic & RHEL5.5
installer.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-generic.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 9be1cca..102f1da 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -172,9 +172,11 @@ 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);
-    if (len == 0)
+    if (len == 0) {
         scsi_command_complete(r, 0);
+    } else {
+        r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, len);
+    }
 }
 
 /* Read more data from scsi device into buffer.  */
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 03/24] scsi: introduce scsi_req_data
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 01/24] scsi: add tracing of scsi requests Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 02/24] scsi-generic: Remove bogus double complete Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 04/24] scsi: introduce SCSIBusOps Paolo Bonzini
                   ` (20 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

This abstracts calling the command_complete callback, reducing churn
in the following patches.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-bus.c     |   11 +++++++++++
 hw/scsi-disk.c    |    8 ++++----
 hw/scsi-generic.c |    6 +++---
 hw/scsi.h         |    1 +
 trace-events      |    1 +
 5 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 0fd85fc..191cbab 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -495,6 +495,17 @@ static const char *scsi_command_name(uint8_t cmd)
     return names[cmd];
 }
 
+/* Called by the devices when data is ready for the HBA.  The HBA should
+   start a DMA operation to read or fill the device's data buffer.
+   Once it completes, calling one of req->dev->info->read_data or
+   req->dev->info->write_data (depending on the direction of the
+   transfer) will restart I/O.  */
+void scsi_req_data(SCSIRequest *req, int len)
+{
+    trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
+    req->bus->complete(req->bus, SCSI_REASON_DATA, req->tag, len);
+}
+
 void scsi_req_print(SCSIRequest *req)
 {
     FILE *fp = stderr;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index b05e654..2b5dc2a 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -167,7 +167,7 @@ static void scsi_read_complete(void * opaque, int ret)
     n = r->iov.iov_len / 512;
     r->sector += n;
     r->sector_count -= n;
-    r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, r->iov.iov_len);
+    scsi_req_data(&r->req, r->iov.iov_len);
 }
 
 
@@ -179,7 +179,7 @@ static void scsi_read_request(SCSIDiskReq *r)
     if (r->sector_count == (uint32_t)-1) {
         DPRINTF("Read buf_len=%zd\n", r->iov.iov_len);
         r->sector_count = 0;
-        r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, r->iov.iov_len);
+        scsi_req_data(&r->req, r->iov.iov_len);
         return;
     }
     DPRINTF("Read sector_count=%d\n", r->sector_count);
@@ -242,7 +242,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
         vm_stop(VMSTOP_DISKFULL);
     } else {
         if (type == SCSI_REQ_STATUS_RETRY_READ) {
-            r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, 0);
+            scsi_req_data(&r->req, 0);
         }
         scsi_command_complete(r, CHECK_CONDITION,
                 HARDWARE_ERROR);
@@ -278,7 +278,7 @@ static void scsi_write_complete(void * opaque, int ret)
         }
         r->iov.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);
+        scsi_req_data(&r->req, len);
     }
 }
 
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 102f1da..e4f1f30 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -175,7 +175,7 @@ static void scsi_read_complete(void * opaque, int ret)
     if (len == 0) {
         scsi_command_complete(r, 0);
     } else {
-        r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, len);
+        scsi_req_data(&r->req, len);
     }
 }
 
@@ -212,7 +212,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);
+        scsi_req_data(&r->req, s->senselen);
         return;
     }
 
@@ -263,7 +263,7 @@ static int scsi_write_data(SCSIDevice *d, uint32_t tag)
 
     if (r->len == 0) {
         r->len = r->buflen;
-        r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, r->len);
+        scsi_req_data(&r->req, r->len);
         return 0;
     }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index d3b5d56..7c09f32 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -105,6 +105,7 @@ void scsi_req_free(SCSIRequest *req);
 
 int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
 void scsi_req_print(SCSIRequest *req);
+void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
 
 #endif
diff --git a/trace-events b/trace-events
index 55f89b4..98e7b2d 100644
--- a/trace-events
+++ b/trace-events
@@ -207,6 +207,7 @@ disable usb_set_device_feature(int addr, int feature, int ret) "dev %d, feature
 
 # hw/scsi-bus.c
 disable scsi_req_alloc(int target, int lun, int tag) "target %d lun %d tag %d"
+disable scsi_req_data(int target, int lun, int tag, int len) "target %d lun %d tag %d len %d"
 disable scsi_req_dequeue(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, uint64_t lba) "target %d lun %d tag %d command %d dir %d length %d lba %"PRIu64""
 disable scsi_req_parse_bad(int target, int lun, int tag, int cmd) "target %d lun %d tag %d command %d"
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 04/24] scsi: introduce SCSIBusOps
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (2 preceding siblings ...)
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 03/24] scsi: introduce scsi_req_data Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 19:35   ` Blue Swirl
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 05/24] scsi: reference-count requests Paolo Bonzini
                   ` (19 subsequent siblings)
  23 siblings, 1 reply; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

There are more operations than a SCSI bus can handle, besides completing
commands.  The current callback in fact is overloaded and can be called
with two different meanings already.  Another example, which this series
will introduce, is cleaning up after a request is cancelled.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 hw/esp.c          |    6 +++++-
 hw/lsi53c895a.c   |    6 +++++-
 hw/scsi-bus.c     |   12 ++++++------
 hw/scsi-generic.c |    2 +-
 hw/scsi.h         |   13 +++++++------
 hw/spapr_vscsi.c  |    6 +++++-
 hw/usb-msd.c      |    6 +++++-
 7 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index fa9d2a2..d8bba7a 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -714,6 +714,10 @@ void esp_init(target_phys_addr_t espaddr, int it_shift,
     *dma_enable = qdev_get_gpio_in(dev, 1);
 }
 
+static struct SCSIBusOps esp_scsi_ops = {
+    .complete = esp_command_complete
+};
+
 static int esp_init1(SysBusDevice *dev)
 {
     ESPState *s = FROM_SYSBUS(ESPState, dev);
@@ -728,7 +732,7 @@ static int esp_init1(SysBusDevice *dev)
 
     qdev_init_gpio_in(&dev->qdev, esp_gpio_demux, 2);
 
-    scsi_bus_new(&s->bus, &dev->qdev, 0, ESP_MAX_DEVS, esp_command_complete);
+    scsi_bus_new(&s->bus, &dev->qdev, 0, ESP_MAX_DEVS, &esp_scsi_ops);
     return scsi_bus_legacy_handle_cmdline(&s->bus);
 }
 
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 2ce38a9..ccea6ad 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -2205,6 +2205,10 @@ static int lsi_scsi_uninit(PCIDevice *d)
     return 0;
 }
 
+static struct SCSIBusOps lsi_scsi_ops = {
+    .complete = lsi_command_complete
+};
+
 static int lsi_scsi_init(PCIDevice *dev)
 {
     LSIState *s = DO_UPCAST(LSIState, dev, dev);
@@ -2241,7 +2245,7 @@ static int lsi_scsi_init(PCIDevice *dev)
                            PCI_BASE_ADDRESS_SPACE_MEMORY, lsi_ram_mapfunc);
     QTAILQ_INIT(&s->queue);
 
-    scsi_bus_new(&s->bus, &dev->qdev, 1, LSI_MAX_DEVS, lsi_command_complete);
+    scsi_bus_new(&s->bus, &dev->qdev, 1, LSI_MAX_DEVS, &lsi_scsi_ops);
     if (!dev->qdev.hotplugged) {
         return scsi_bus_legacy_handle_cmdline(&s->bus);
     }
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 191cbab..f21704f 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -21,13 +21,13 @@ static int next_scsi_bus;
 
 /* Create a scsi bus, and attach devices to it.  */
 void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
-                  scsi_completionfn complete)
+                  SCSIBusOps *ops)
 {
     qbus_create_inplace(&bus->qbus, &scsi_bus_info, host, NULL);
     bus->busnr = next_scsi_bus++;
     bus->tcq = tcq;
     bus->ndev = ndev;
-    bus->complete = complete;
+    bus->ops = ops;
     bus->qbus.allow_hotplug = 1;
 }
 
@@ -503,7 +503,7 @@ static const char *scsi_command_name(uint8_t cmd)
 void scsi_req_data(SCSIRequest *req, int len)
 {
     trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
-    req->bus->complete(req->bus, SCSI_REASON_DATA, req->tag, len);
+    req->bus->ops->complete(req->bus, SCSI_REASON_DATA, req->tag, len);
 }
 
 void scsi_req_print(SCSIRequest *req)
@@ -538,9 +538,9 @@ 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->bus->ops->complete(req->bus, SCSI_REASON_DONE,
+                            req->tag,
+                            req->status);
 }
 
 static char *scsibus_get_fw_dev_path(DeviceState *dev)
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index e4f1f30..f09458b 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -335,7 +335,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
         s->senselen = 7;
         s->driver_status = SG_ERR_DRIVER_SENSE;
         bus = scsi_bus_from_device(d);
-        bus->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
+        bus->ops->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
         return 0;
     }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index 7c09f32..20cf397 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -16,10 +16,9 @@ enum scsi_reason {
 };
 
 typedef struct SCSIBus SCSIBus;
+typedef struct SCSIBusOps SCSIBusOps;
 typedef struct SCSIDevice SCSIDevice;
 typedef struct SCSIDeviceInfo SCSIDeviceInfo;
-typedef void (*scsi_completionfn)(SCSIBus *bus, int reason, uint32_t tag,
-                                  uint32_t arg);
 
 enum SCSIXferMode {
     SCSI_XFER_NONE,      /*  TEST_UNIT_READY, ...            */
@@ -74,20 +73,22 @@ struct SCSIDeviceInfo {
     uint8_t *(*get_buf)(SCSIDevice *s, uint32_t tag);
 };
 
-typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
-              int unit);
+struct SCSIBusOps {
+    void (*complete)(SCSIBus *bus, int reason, uint32_t tag, uint32_t arg);
+};
+
 struct SCSIBus {
     BusState qbus;
     int busnr;
 
     int tcq, ndev;
-    scsi_completionfn complete;
+    SCSIBusOps *ops;
 
     SCSIDevice *devs[MAX_SCSI_DEVS];
 };
 
 void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
-                  scsi_completionfn complete);
+                  SCSIBusOps *ops);
 void scsi_qdev_register(SCSIDeviceInfo *info);
 
 static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 9928334..9f5c154 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -907,6 +907,10 @@ static int vscsi_do_crq(struct VIOsPAPRDevice *dev, uint8_t *crq_data)
     return 0;
 }
 
+static struct SCSIBusOps vscsi_scsi_ops = {
+    .complete = vscsi_command_complete
+};
+
 static int spapr_vscsi_init(VIOsPAPRDevice *dev)
 {
     VSCSIState *s = DO_UPCAST(VSCSIState, vdev, dev);
@@ -923,7 +927,7 @@ static int spapr_vscsi_init(VIOsPAPRDevice *dev)
     dev->crq.SendFunc = vscsi_do_crq;
 
     scsi_bus_new(&s->bus, &dev->qdev, 1, VSCSI_REQ_LIMIT,
-                 vscsi_command_complete);
+                 &vscsi_scsi_ops);
     if (!dev->qdev.hotplugged) {
         scsi_bus_legacy_handle_cmdline(&s->bus);
     }
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index bd1c3a4..7a07a12 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -487,6 +487,10 @@ static void usb_msd_password_cb(void *opaque, int err)
         qdev_unplug(&s->dev.qdev);
 }
 
+static struct SCSIBusOps usb_msd_scsi_ops = {
+    .complete = usb_msd_command_complete
+};
+
 static int usb_msd_initfn(USBDevice *dev)
 {
     MSDState *s = DO_UPCAST(MSDState, dev, dev);
@@ -516,7 +520,7 @@ static int usb_msd_initfn(USBDevice *dev)
     }
 
     usb_desc_init(dev);
-    scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, usb_msd_command_complete);
+    scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, &usb_msd_scsi_ops);
     s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable);
     if (!s->scsi_dev) {
         return -1;
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 05/24] scsi: reference-count requests
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (3 preceding siblings ...)
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 04/24] scsi: introduce SCSIBusOps Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 06/24] lsi: extract lsi_find_by_tag Paolo Bonzini
                   ` (18 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

With the next patch, a device may hold SCSIRequest for an indefinite
time.  Split a rather big patch, and protect against access errors,
by reference counting them.

There is some ugliness in scsi_send_command implementation due to
the need to unref the request when it fails.  This will go away
with the next patches, which move the unref'ing to the devices.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-bus.c     |   29 ++++++++++++++++++++++-------
 hw/scsi-disk.c    |   21 +++++++++++++--------
 hw/scsi-generic.c |   24 ++++++++++++++++--------
 hw/scsi.h         |    5 +++++
 4 files changed, 56 insertions(+), 23 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index f21704f..74b9a74 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -136,6 +136,8 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
     SCSIRequest *req;
 
     req = qemu_mallocz(size);
+    /* Two references: one is passed back to the HBA, one is in d->requests.  */
+    req->refcount = 2;
     req->bus = scsi_bus_from_device(d);
     req->dev = d;
     req->tag = tag;
@@ -159,21 +161,16 @@ SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag)
     return NULL;
 }
 
-static void scsi_req_dequeue(SCSIRequest *req)
+void scsi_req_dequeue(SCSIRequest *req)
 {
     trace_scsi_req_dequeue(req->dev->id, req->lun, req->tag);
     if (req->enqueued) {
         QTAILQ_REMOVE(&req->dev->requests, req, next);
         req->enqueued = false;
+        scsi_req_unref(req);
     }
 }
 
-void scsi_req_free(SCSIRequest *req)
-{
-    scsi_req_dequeue(req);
-    qemu_free(req);
-}
-
 static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
 {
     switch (cmd[0] >> 5) {
@@ -495,6 +492,22 @@ static const char *scsi_command_name(uint8_t cmd)
     return names[cmd];
 }
 
+SCSIRequest *scsi_req_ref(SCSIRequest *req)
+{
+    req->refcount++;
+    return req;
+}
+
+void scsi_req_unref(SCSIRequest *req)
+{
+    if (--req->refcount == 0) {
+	if (req->dev->info->free_req) {
+            req->dev->info->free_req(req);
+	}
+        qemu_free(req);
+    }
+}
+
 /* Called by the devices when data is ready for the HBA.  The HBA should
    start a DMA operation to read or fill the device's data buffer.
    Once it completes, calling one of req->dev->info->read_data or
@@ -537,10 +550,12 @@ void scsi_req_print(SCSIRequest *req)
 void scsi_req_complete(SCSIRequest *req)
 {
     assert(req->status != -1);
+    scsi_req_ref(req);
     scsi_req_dequeue(req);
     req->bus->ops->complete(req->bus, SCSI_REASON_DONE,
                             req->tag,
                             req->status);
+    scsi_req_unref(req);
 }
 
 static char *scsibus_get_fw_dev_path(DeviceState *dev)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 2b5dc2a..d17f43e 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -95,10 +95,11 @@ static SCSIDiskReq *scsi_new_request(SCSIDiskState *s, uint32_t tag,
     return r;
 }
 
-static void scsi_remove_request(SCSIDiskReq *r)
+static void scsi_free_request(SCSIRequest *req)
 {
+    SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
+
     qemu_vfree(r->iov.iov_base);
-    scsi_req_free(&r->req);
 }
 
 static SCSIDiskReq *scsi_find_request(SCSIDiskState *s, uint32_t tag)
@@ -131,7 +132,6 @@ static void scsi_command_complete(SCSIDiskReq *r, int status, int sense)
             r->req.tag, status, sense);
     scsi_req_set_status(r, status, sense);
     scsi_req_complete(&r->req);
-    scsi_remove_request(r);
 }
 
 /* Cancel a pending data transfer.  */
@@ -145,7 +145,7 @@ static void scsi_cancel_io(SCSIDevice *d, uint32_t tag)
         if (r->req.aiocb)
             bdrv_aio_cancel(r->req.aiocb);
         r->req.aiocb = NULL;
-        scsi_remove_request(r);
+        scsi_req_dequeue(&r->req);
     }
 }
 
@@ -1030,7 +1030,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
                                  uint8_t *buf, int lun)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
-    uint32_t len;
+    int32_t len;
     int is_write;
     uint8_t command;
     uint8_t *outbuf;
@@ -1092,6 +1092,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     case REZERO_UNIT:
         rc = scsi_disk_emulate_command(r, outbuf);
         if (rc < 0) {
+            scsi_req_unref(&r->req);
             return 0;
         }
 
@@ -1178,9 +1179,11 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
         DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
     fail:
         scsi_command_complete(r, CHECK_CONDITION, ILLEGAL_REQUEST);
+        scsi_req_unref(&r->req);
         return 0;
     illegal_lba:
         scsi_command_complete(r, CHECK_CONDITION, HARDWARE_ERROR);
+        scsi_req_unref(&r->req);
         return 0;
     }
     if (r->sector_count == 0 && r->iov.iov_len == 0) {
@@ -1188,12 +1191,13 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     }
     len = r->sector_count * 512 + r->iov.iov_len;
     if (is_write) {
-        return -len;
+        len = -len;
     } else {
         if (!r->sector_count)
             r->sector_count = -1;
-        return len;
     }
+    scsi_req_unref(&r->req);
+    return len;
 }
 
 static void scsi_disk_purge_requests(SCSIDiskState *s)
@@ -1205,7 +1209,7 @@ static void scsi_disk_purge_requests(SCSIDiskState *s)
         if (r->req.aiocb) {
             bdrv_aio_cancel(r->req.aiocb);
         }
-        scsi_remove_request(r);
+        scsi_req_dequeue(&r->req);
     }
 }
 
@@ -1288,6 +1292,7 @@ static SCSIDeviceInfo scsi_disk_info = {
     .qdev.reset   = scsi_disk_reset,
     .init         = scsi_disk_initfn,
     .destroy      = scsi_destroy,
+    .free_req     = scsi_free_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 f09458b..7e4831c 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -74,10 +74,11 @@ static SCSIGenericReq *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lu
     return DO_UPCAST(SCSIGenericReq, req, req);
 }
 
-static void scsi_remove_request(SCSIGenericReq *r)
+static void scsi_free_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)
@@ -112,7 +113,6 @@ 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.  */
@@ -127,7 +127,7 @@ static void scsi_cancel_io(SCSIDevice *d, uint32_t tag)
         if (r->req.aiocb)
             bdrv_aio_cancel(r->req.aiocb);
         r->req.aiocb = NULL;
-        scsi_remove_request(r);
+        scsi_req_dequeue(&r->req);
     }
 }
 
@@ -320,6 +320,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     SCSIGenericReq *r;
     SCSIBus *bus;
     int ret;
+    int32_t len;
 
     if (cmd[0] != REQUEST_SENSE &&
         (lun != s->lun || (cmd[1] >> 5) != s->lun)) {
@@ -348,7 +349,8 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
 
     if (-1 == scsi_req_parse(&r->req, cmd)) {
         BADF("Unsupported command length, command %x\n", cmd[0]);
-        scsi_remove_request(r);
+        scsi_req_dequeue(&r->req);
+        scsi_req_unref(&r->req);
         return 0;
     }
     scsi_req_fixup(&r->req);
@@ -374,8 +376,10 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
         ret = execute_command(s->bs, r, SG_DXFER_NONE, scsi_command_complete);
         if (ret == -1) {
             scsi_command_complete(r, -EINVAL);
+            scsi_req_unref(&r->req);
             return 0;
         }
+        scsi_req_unref(&r->req);
         return 0;
     }
 
@@ -390,10 +394,13 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     r->len = r->req.cmd.xfer;
     if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
         r->len = 0;
-        return -r->req.cmd.xfer;
+        len = -r->req.cmd.xfer;
+    } else {
+        len = r->req.cmd.xfer;
     }
 
-    return r->req.cmd.xfer;
+    scsi_req_unref(&r->req);
+    return len;
 }
 
 static int get_blocksize(BlockDriverState *bdrv)
@@ -466,7 +473,7 @@ static void scsi_generic_purge_requests(SCSIGenericState *s)
         if (r->req.aiocb) {
             bdrv_aio_cancel(r->req.aiocb);
         }
-        scsi_remove_request(r);
+        scsi_req_dequeue(&r->req);
     }
 }
 
@@ -558,6 +565,7 @@ static SCSIDeviceInfo scsi_generic_info = {
     .qdev.reset   = scsi_generic_reset,
     .init         = scsi_generic_initfn,
     .destroy      = scsi_destroy,
+    .free_req     = scsi_free_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 20cf397..7d97dfa 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -29,6 +29,7 @@ enum SCSIXferMode {
 typedef struct SCSIRequest {
     SCSIBus           *bus;
     SCSIDevice        *dev;
+    uint32_t          refcount;
     uint32_t          tag;
     uint32_t          lun;
     uint32_t          status;
@@ -65,6 +66,7 @@ struct SCSIDeviceInfo {
     DeviceInfo qdev;
     scsi_qdev_initfn init;
     void (*destroy)(SCSIDevice *s);
+    void (*free_req)(SCSIRequest *req);
     int32_t (*send_command)(SCSIDevice *s, uint32_t tag, uint8_t *buf,
                             int lun);
     void (*read_data)(SCSIDevice *s, uint32_t tag);
@@ -103,6 +105,9 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
 SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
 SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag);
 void scsi_req_free(SCSIRequest *req);
+void scsi_req_dequeue(SCSIRequest *req);
+SCSIRequest *scsi_req_ref(SCSIRequest *req);
+void scsi_req_unref(SCSIRequest *req);
 
 int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
 void scsi_req_print(SCSIRequest *req);
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 06/24] lsi: extract lsi_find_by_tag
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (4 preceding siblings ...)
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 05/24] scsi: reference-count requests Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 07/24] scsi: Use 'SCSIRequest' directly Paolo Bonzini
                   ` (17 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/lsi53c895a.c |   63 +++++++++++++++++++++++++++++++++---------------------
 1 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index ccea6ad..3b67155 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -652,38 +652,51 @@ static void lsi_reselect(LSIState *s, lsi_request *p)
     }
 }
 
-/* Record that data is available for a queued command.  Returns zero if
-   the device was reselected, nonzero if the IO is deferred.  */
-static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
+static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t tag)
 {
     lsi_request *p;
 
     QTAILQ_FOREACH(p, &s->queue, next) {
         if (p->tag == tag) {
-            if (p->pending) {
-                BADF("Multiple IO pending for tag %d\n", tag);
-            }
-            p->pending = arg;
-            /* Reselect if waiting for it, or if reselection triggers an IRQ
-               and the bus is free.
-               Since no interrupt stacking is implemented in the emulation, it
-               is also required that there are no pending interrupts waiting
-               for service from the device driver. */
-            if (s->waiting == 1 ||
-                (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON) &&
-                 !(s->istat0 & (LSI_ISTAT0_SIP | LSI_ISTAT0_DIP)))) {
-                /* Reselect device.  */
-                lsi_reselect(s, p);
-                return 0;
-            } else {
-                DPRINTF("Queueing IO tag=0x%x\n", tag);
-                p->pending = arg;
-                return 1;
-            }
+            return p;
         }
     }
-    BADF("IO with unknown tag %d\n", tag);
-    return 1;
+
+    return NULL;
+}
+
+/* Record that data is available for a queued command.  Returns zero if
+   the device was reselected, nonzero if the IO is deferred.  */
+static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
+{
+    lsi_request *p;
+
+    p = lsi_find_by_tag(s, tag);
+    if (!p) {
+        BADF("IO with unknown tag %d\n", tag);
+        return 1;
+    }
+
+    if (p->pending) {
+        BADF("Multiple IO pending for tag %d\n", tag);
+    }
+    p->pending = arg;
+    /* Reselect if waiting for it, or if reselection triggers an IRQ
+       and the bus is free.
+       Since no interrupt stacking is implemented in the emulation, it
+       is also required that there are no pending interrupts waiting
+       for service from the device driver. */
+    if (s->waiting == 1 ||
+        (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON) &&
+         !(s->istat0 & (LSI_ISTAT0_SIP | LSI_ISTAT0_DIP)))) {
+        /* Reselect device.  */
+        lsi_reselect(s, p);
+        return 0;
+    } else {
+        DPRINTF("Queueing IO tag=0x%x\n", tag);
+        p->pending = arg;
+        return 1;
+    }
 }
 
 /* Callback to indicate that the SCSI layer has completed a transfer.  */
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 07/24] scsi: Use 'SCSIRequest' directly
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (5 preceding siblings ...)
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 06/24] lsi: extract lsi_find_by_tag Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 08/24] scsi: commonize purging requests Paolo Bonzini
                   ` (16 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch, Hannes Reinecke

From: Hannes Reinecke <hare@suse.de>

Currently the SCSIRequest structure is abstracted away and cannot accessed
directly from the driver. This requires the handler to do a lookup on
an abstract 'tag' which identifies the SCSIRequest structure.

With this patch the SCSIRequest structure is exposed to the driver. This
allows use to use it directly as an argument to the SCSIDeviceInfo
callback functions and remove the lookup.

A new callback function 'alloc_req' is introduced matching 'free
req'; unref'ing to free up resources after use is moved into the
scsi_command_complete callbacks.

This temporarily introduces a leak of requests that are cancelled,
when they are removed from the queue and not from the driver.  This
is fixed later by introducing scsi_req_cancel.  That patch in turn
depends on this one, because the argument to scsi_req_cancel is a
SCSIRequest.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/esp.c          |   29 ++++++++-----
 hw/lsi53c895a.c   |   56 +++++++++++++++-----------
 hw/scsi-bus.c     |   25 ++++--------
 hw/scsi-disk.c    |  116 ++++++++++++++--------------------------------------
 hw/scsi-generic.c |  107 +++++++++++++++----------------------------------
 hw/scsi.h         |   21 +++++-----
 hw/spapr_vscsi.c  |   44 +++++++++++---------
 hw/usb-msd.c      |   27 +++++++-----
 8 files changed, 172 insertions(+), 253 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index d8bba7a..096f4dc 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;
     }
 
@@ -230,9 +231,10 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
     int32_t datalen;
     int lun;
 
-    DPRINTF("do_busid_cmd: busid 0x%x\n", 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->alloc_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,10 +390,9 @@ static void esp_do_dma(ESPState *s)
     }
 }
 
-static void esp_command_complete(SCSIBus *bus, int reason, uint32_t tag,
-                                 uint32_t arg)
+static void esp_command_complete(SCSIRequest *req, int reason, uint32_t arg)
 {
-    ESPState *s = DO_UPCAST(ESPState, busdev.qdev, bus->qbus.parent);
+    ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
 
     if (reason == SCSI_REASON_DONE) {
         DPRINTF("SCSI Command complete\n");
@@ -405,11 +406,15 @@ 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);
-        s->current_dev = NULL;
+	if (s->current_req) {
+            scsi_req_unref(s->current_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 3b67155..b9febae 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;
@@ -567,11 +568,9 @@ static void lsi_do_dma(LSIState *s, int out)
     s->csbc += count;
     s->dnad += count;
     s->dbc -= count;
-
-    if (s->current->dma_buf == NULL) {
-        s->current->dma_buf = dev->info->get_buf(dev, s->current->tag);
+     if (s->current->dma_buf == NULL) {
+        s->current->dma_buf = dev->info->get_buf(s->current->req);
     }
-
     /* ??? Set SFBR to first data byte.  */
     if (out) {
         cpu_physical_memory_read(addr, s->current->dma_buf, count);
@@ -583,10 +582,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;
@@ -698,12 +697,10 @@ static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
         return 1;
     }
 }
-
-/* Callback to indicate that the SCSI layer has completed a transfer.  */
-static void lsi_command_complete(SCSIBus *bus, int reason, uint32_t tag,
-                                 uint32_t arg)
+ /* Callback to indicate that the SCSI layer has completed a transfer.  */
+static void lsi_command_complete(SCSIRequest *req, int reason, uint32_t arg)
 {
-    LSIState *s = DO_UPCAST(LSIState, dev.qdev, bus->qbus.parent);
+    LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
     int out;
 
     out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
@@ -718,21 +715,24 @@ static void lsi_command_complete(SCSIBus *bus, int reason, uint32_t tag,
             lsi_set_phase(s, PHASE_ST);
         }
 
-        qemu_free(s->current);
-        s->current = NULL;
-
+        if (req == s->current->req) {
+            scsi_req_unref(s->current->req);
+            qemu_free(s->current);
+            s->current = NULL;
+	}
         lsi_resume_script(s);
         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)
@@ -768,14 +768,16 @@ 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->alloc_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) {
@@ -868,13 +870,15 @@ static void lsi_do_msgout(LSIState *s)
     int len;
     uint32_t current_tag;
     SCSIDevice *current_dev;
-    lsi_request *p, *p_next;
+    lsi_request *current_req, *p, *p_next;
     int id;
 
     if (s->current) {
         current_tag = s->current->tag;
+        current_req = s->current;
     } else {
         current_tag = s->select_tag;
+        current_req = lsi_find_by_tag(s, current_tag);
     }
     id = (current_tag >> 8) & 0xf;
     current_dev = s->bus.devs[id];
@@ -926,7 +930,9 @@ static void lsi_do_msgout(LSIState *s)
         case 0x0d:
             /* The ABORT TAG message clears the current I/O process only. */
             DPRINTF("MSG: ABORT TAG tag=0x%x\n", current_tag);
-            current_dev->info->cancel_io(current_dev, current_tag);
+            if (current_req) {
+                current_dev->info->cancel_io(current_req->req);
+            }
             lsi_disconnect(s);
             break;
         case 0x06:
@@ -949,7 +955,9 @@ static void lsi_do_msgout(LSIState *s)
             }
 
             /* clear the current I/O process */
-            current_dev->info->cancel_io(current_dev, current_tag);
+            if (s->current) {
+                current_dev->info->cancel_io(s->current->req);
+            }
 
             /* As the current implemented devices scsi_disk and scsi_generic
                only support one LUN, we don't need to keep track of LUNs.
@@ -961,7 +969,7 @@ static void lsi_do_msgout(LSIState *s)
             id = current_tag & 0x0000ff00;
             QTAILQ_FOREACH_SAFE(p, &s->queue, next, p_next) {
                 if ((p->tag & 0x0000ff00) == id) {
-                    current_dev->info->cancel_io(current_dev, p->tag);
+                    current_dev->info->cancel_io(p->req);
                     QTAILQ_REMOVE(&s->queue, p, next);
                 }
             }
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 74b9a74..94a5873 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -136,29 +136,22 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
     SCSIRequest *req;
 
     req = qemu_mallocz(size);
-    /* Two references: one is passed back to the HBA, one is in d->requests.  */
-    req->refcount = 2;
+    req->refcount = 1;
     req->bus = scsi_bus_from_device(d);
     req->dev = d;
     req->tag = tag;
     req->lun = lun;
     req->status = -1;
-    req->enqueued = true;
     trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
-    QTAILQ_INSERT_TAIL(&d->requests, req, next);
     return req;
 }
 
-SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag)
+void scsi_req_enqueue(SCSIRequest *req)
 {
-    SCSIRequest *req;
-
-    QTAILQ_FOREACH(req, &d->requests, next) {
-        if (req->tag == tag) {
-            return req;
-        }
-    }
-    return NULL;
+    assert(!req->enqueued);
+    scsi_req_ref(req);
+    req->enqueued = true;
+    QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
 }
 
 void scsi_req_dequeue(SCSIRequest *req)
@@ -516,7 +509,7 @@ void scsi_req_unref(SCSIRequest *req)
 void scsi_req_data(SCSIRequest *req, int len)
 {
     trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
-    req->bus->ops->complete(req->bus, SCSI_REASON_DATA, req->tag, len);
+    req->bus->ops->complete(req, SCSI_REASON_DATA, len);
 }
 
 void scsi_req_print(SCSIRequest *req)
@@ -552,9 +545,7 @@ void scsi_req_complete(SCSIRequest *req)
     assert(req->status != -1);
     scsi_req_ref(req);
     scsi_req_dequeue(req);
-    req->bus->ops->complete(req->bus, SCSI_REASON_DONE,
-                            req->tag,
-                            req->status);
+    req->bus->ops->complete(req, SCSI_REASON_DONE, req->status);
     scsi_req_unref(req);
 }
 
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index d17f43e..1fa5385 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -83,16 +83,17 @@ 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);
     r = DO_UPCAST(SCSIDiskReq, req, req);
     r->iov.iov_base = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE);
-    return r;
+    return req;
 }
 
 static void scsi_free_request(SCSIRequest *req)
@@ -102,11 +103,6 @@ static void scsi_free_request(SCSIRequest *req)
     qemu_vfree(r->iov.iov_base);
 }
 
-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));
@@ -135,18 +131,16 @@ static void scsi_command_complete(SCSIDiskReq *r, int status, int sense)
 }
 
 /* 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_req_dequeue(&r->req);
+    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;
+    scsi_req_dequeue(&r->req);
 }
 
 static void scsi_read_complete(void * opaque, int ret)
@@ -171,8 +165,10 @@ static void scsi_read_complete(void * opaque, int ret)
 }
 
 
-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;
 
@@ -204,23 +200,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) {
-        BADF("Bad read tag 0x%x\n", tag);
-        /* ??? This is the wrong error.  */
-        scsi_command_complete(r, CHECK_CONDITION, HARDWARE_ERROR);
-        return;
-    }
-
-    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);
@@ -282,8 +261,9 @@ static void scsi_write_complete(void * opaque, int ret)
     }
 }
 
-static void scsi_write_request(SCSIDiskReq *r)
+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;
 
@@ -302,24 +282,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) {
-        BADF("Bad write tag 0x%x\n", tag);
-        scsi_command_complete(r, CHECK_CONDITION, HARDWARE_ERROR);
-        return 1;
-    }
-
-    scsi_write_request(r);
 
     return 0;
 }
@@ -344,10 +306,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.iov_base);
@@ -373,16 +335,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 (uint8_t *)r->iov.iov_base;
 }
 
@@ -1026,26 +982,18 @@ 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);
     int32_t len;
     int is_write;
     uint8_t command;
     uint8_t *outbuf;
-    SCSIDiskReq *r;
     int rc;
 
+    scsi_req_enqueue(req);
     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);
     outbuf = (uint8_t *)r->iov.iov_base;
     is_write = 0;
     DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, tag, buf[0]);
@@ -1064,9 +1012,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)
             goto fail;
     }
@@ -1092,7 +1040,6 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     case REZERO_UNIT:
         rc = scsi_disk_emulate_command(r, outbuf);
         if (rc < 0) {
-            scsi_req_unref(&r->req);
             return 0;
         }
 
@@ -1102,7 +1049,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     case READ_10:
     case READ_12:
     case READ_16:
-        len = r->req.cmd.xfer / d->blocksize;
+        len = r->req.cmd.xfer / s->qdev.blocksize;
         DPRINTF("Read (sector %" PRId64 ", count %d)\n", r->req.cmd.lba, len);
         if (r->req.cmd.lba > s->max_lba)
             goto illegal_lba;
@@ -1116,7 +1063,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     case WRITE_VERIFY:
     case WRITE_VERIFY_12:
     case WRITE_VERIFY_16:
-        len = r->req.cmd.xfer / d->blocksize;
+        len = r->req.cmd.xfer / s->qdev.blocksize;
         DPRINTF("Write %s(sector %" PRId64 ", count %d)\n",
                 (command & 0xe) == 0xe ? "And Verify " : "",
                 r->req.cmd.lba, len);
@@ -1151,7 +1098,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
         }
         break;
     case WRITE_SAME_16:
-        len = r->req.cmd.xfer / d->blocksize;
+        len = r->req.cmd.xfer / s->qdev.blocksize;
 
         DPRINTF("WRITE SAME(16) (sector %" PRId64 ", count %d)\n",
                 r->req.cmd.lba, len);
@@ -1179,11 +1126,9 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
         DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
     fail:
         scsi_command_complete(r, CHECK_CONDITION, ILLEGAL_REQUEST);
-        scsi_req_unref(&r->req);
         return 0;
     illegal_lba:
         scsi_command_complete(r, CHECK_CONDITION, HARDWARE_ERROR);
-        scsi_req_unref(&r->req);
         return 0;
     }
     if (r->sector_count == 0 && r->iov.iov_len == 0) {
@@ -1196,7 +1141,6 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
         if (!r->sector_count)
             r->sector_count = -1;
     }
-    scsi_req_unref(&r->req);
     return len;
 }
 
@@ -1210,6 +1154,7 @@ static void scsi_disk_purge_requests(SCSIDiskState *s)
             bdrv_aio_cancel(r->req.aiocb);
         }
         scsi_req_dequeue(&r->req);
+        scsi_req_unref(&r->req);
     }
 }
 
@@ -1292,6 +1237,7 @@ static SCSIDeviceInfo scsi_disk_info = {
     .qdev.reset   = scsi_disk_reset,
     .init         = scsi_disk_initfn,
     .destroy      = scsi_destroy,
+    .alloc_req    = scsi_new_request,
     .free_req     = scsi_free_request,
     .send_command = scsi_send_command,
     .read_data    = scsi_read_data,
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 7e4831c..a4b2a69 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -66,12 +66,12 @@ struct SCSIGenericState
     uint8_t senselen;
 };
 
-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_free_request(SCSIRequest *req)
@@ -81,11 +81,6 @@ static void scsi_free_request(SCSIRequest *req)
     qemu_free(r->buf);
 }
 
-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)
 {
@@ -116,19 +111,16 @@ static void scsi_command_complete(void *opaque, int ret)
 }
 
 /* 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_req_dequeue(&r->req);
+    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;
+    scsi_req_dequeue(&r->req);
 }
 
 static int execute_command(BlockDriverState *bdrv,
@@ -180,21 +172,13 @@ static void scsi_read_complete(void * opaque, int ret)
 }
 
 /* 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;
+    SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
+    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, r->req.dev);
     int ret;
 
-    DPRINTF("scsi_read_data 0x%x\n", tag);
-    r = scsi_find_request(s, tag);
-    if (!r) {
-        BADF("Bad read tag 0x%x\n", tag);
-        /* ??? This is the wrong error.  */
-        scsi_command_complete(r, -EINVAL);
-        return;
-    }
-
+    DPRINTF("scsi_read_data 0x%x\n", req->tag);
     if (r->len == -1) {
         scsi_command_complete(r, 0);
         return;
@@ -246,21 +230,13 @@ 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) {
-        BADF("Bad write tag 0x%x\n", tag);
-        /* ??? This is the wrong error.  */
-        scsi_command_complete(r, -EINVAL);
-        return 0;
-    }
-
+    DPRINTF("scsi_write_data 0x%x\n", req->tag);
     if (r->len == 0) {
         r->len = r->buflen;
         scsi_req_data(&r->req, r->len);
@@ -277,15 +253,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;
 }
 
@@ -313,18 +284,17 @@ 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;
+    SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
+    SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
     SCSIBus *bus;
     int ret;
-    int32_t len;
 
+    scsi_req_enqueue(req);
     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);
 
         s->sensebuf[0] = 0x70;
         s->sensebuf[1] = 0x00;
@@ -335,18 +305,11 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
         s->sensebuf[6] = 0x00;
         s->senselen = 7;
         s->driver_status = SG_ERR_DRIVER_SENSE;
-        bus = scsi_bus_from_device(d);
-        bus->ops->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
+        bus = scsi_bus_from_device(&s->qdev);
+        bus->ops->complete(req, SCSI_REASON_DONE, CHECK_CONDITION);
         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_req_dequeue(&r->req);
@@ -376,10 +339,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
         ret = execute_command(s->bs, r, SG_DXFER_NONE, scsi_command_complete);
         if (ret == -1) {
             scsi_command_complete(r, -EINVAL);
-            scsi_req_unref(&r->req);
-            return 0;
         }
-        scsi_req_unref(&r->req);
         return 0;
     }
 
@@ -394,13 +354,10 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
     r->len = r->req.cmd.xfer;
     if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
         r->len = 0;
-        len = -r->req.cmd.xfer;
+        return -r->req.cmd.xfer;
     } else {
-        len = r->req.cmd.xfer;
+        return r->req.cmd.xfer;
     }
-
-    scsi_req_unref(&r->req);
-    return len;
 }
 
 static int get_blocksize(BlockDriverState *bdrv)
@@ -474,6 +431,7 @@ static void scsi_generic_purge_requests(SCSIGenericState *s)
             bdrv_aio_cancel(r->req.aiocb);
         }
         scsi_req_dequeue(&r->req);
+        scsi_req_unref(&r->req);
     }
 }
 
@@ -565,6 +523,7 @@ static SCSIDeviceInfo scsi_generic_info = {
     .qdev.reset   = scsi_generic_reset,
     .init         = scsi_generic_initfn,
     .destroy      = scsi_destroy,
+    .alloc_req    = scsi_new_request,
     .free_req     = scsi_free_request,
     .send_command = scsi_send_command,
     .read_data    = scsi_read_data,
diff --git a/hw/scsi.h b/hw/scsi.h
index 7d97dfa..3b6899c 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -19,6 +19,7 @@ typedef struct SCSIBus SCSIBus;
 typedef struct SCSIBusOps SCSIBusOps;
 typedef struct SCSIDevice SCSIDevice;
 typedef struct SCSIDeviceInfo SCSIDeviceInfo;
+typedef struct SCSIRequest SCSIRequest;
 
 enum SCSIXferMode {
     SCSI_XFER_NONE,      /*  TEST_UNIT_READY, ...            */
@@ -26,7 +27,7 @@ enum SCSIXferMode {
     SCSI_XFER_TO_DEV,    /*  WRITE, MODE_SELECT, ...         */
 };
 
-typedef struct SCSIRequest {
+struct SCSIRequest {
     SCSIBus           *bus;
     SCSIDevice        *dev;
     uint32_t          refcount;
@@ -43,7 +44,7 @@ typedef struct SCSIRequest {
     BlockDriverAIOCB  *aiocb;
     bool enqueued;
     QTAILQ_ENTRY(SCSIRequest) next;
-} SCSIRequest;
+};
 
 struct SCSIDevice
 {
@@ -66,17 +67,17 @@ struct SCSIDeviceInfo {
     DeviceInfo qdev;
     scsi_qdev_initfn init;
     void (*destroy)(SCSIDevice *s);
+    SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun);
     void (*free_req)(SCSIRequest *req);
-    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);
+    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);
 };
 
 struct SCSIBusOps {
-    void (*complete)(SCSIBus *bus, int reason, uint32_t tag, uint32_t arg);
+    void (*complete)(SCSIRequest *req, int reason, uint32_t arg);
 };
 
 struct SCSIBus {
@@ -103,7 +104,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
 int scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
 
 SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
-SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag);
+void scsi_req_enqueue(SCSIRequest *req);
 void scsi_req_free(SCSIRequest *req);
 void scsi_req_dequeue(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 9f5c154..cf2ed73 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -75,6 +75,7 @@ typedef struct vscsi_req {
 
     /* SCSI request tracking */
     SCSIDevice              *sdev;
+    SCSIRequest             *sreq;
     uint32_t                qtag; /* qemu tag != srp tag */
     int                     lun;
     int                     active;
@@ -123,11 +124,16 @@ static struct vscsi_req *vscsi_get_req(VSCSIState *s)
 
 static void vscsi_put_req(VSCSIState *s, vscsi_req *req)
 {
+    if (req->sreq != NULL) {
+        scsi_req_unref(req->sreq);
+    }
+    req->sreq = NULL;
     req->active = 0;
 }
 
-static vscsi_req *vscsi_find_req(VSCSIState *s, uint32_t tag)
+static vscsi_req *vscsi_find_req(VSCSIState *s, SCSIRequest *req)
 {
+    uint32_t tag = req->tag;
     if (tag >= VSCSI_REQ_LIMIT || !s->reqs[tag].active) {
         return NULL;
     }
@@ -453,11 +459,11 @@ static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
     cdb[4] = 96;
     cdb[5] = 0;
     req->sensing = 1;
-    n = sdev->info->send_command(sdev, req->qtag, cdb, req->lun);
+    n = sdev->info->send_command(req->sreq, cdb);
     dprintf("VSCSI: Queued request sense tag 0x%x\n", req->qtag);
     if (n < 0) {
         fprintf(stderr, "VSCSI: REQUEST_SENSE wants write data !?!?!?\n");
-        sdev->info->cancel_io(sdev, req->qtag);
+        sdev->info->cancel_io(req->sreq);
         vscsi_makeup_sense(s, req, HARDWARE_ERROR, 0, 0);
         vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
         vscsi_put_req(s, req);
@@ -465,24 +471,23 @@ static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
     } else if (n == 0) {
         return;
     }
-    sdev->info->read_data(sdev, req->qtag);
+    sdev->info->read_data(req->sreq);
 }
 
 /* Callback to indicate that the SCSI layer has completed a transfer.  */
-static void vscsi_command_complete(SCSIBus *bus, int reason, uint32_t tag,
-                                   uint32_t arg)
+static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
 {
-    VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, bus->qbus.parent);
-    vscsi_req *req = vscsi_find_req(s, tag);
+    VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
+    vscsi_req *req = vscsi_find_req(s, sreq);
     SCSIDevice *sdev;
     uint8_t *buf;
     int32_t res_in = 0, res_out = 0;
     int len, rc = 0;
 
     dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x arg=0x%x, req=%p\n",
-            reason, tag, arg, req);
+            reason, sreq->tag, arg, req);
     if (req == NULL) {
-        fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", tag);
+        fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
         return;
     }
     sdev = req->sdev;
@@ -493,7 +498,7 @@ static void vscsi_command_complete(SCSIBus *bus, int reason, uint32_t tag,
             vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
             vscsi_put_req(s, req);
         } else {
-            uint8_t *buf = sdev->info->get_buf(sdev, tag);
+            uint8_t *buf = sdev->info->get_buf(sreq);
 
             len = MIN(arg, SCSI_SENSE_BUF_SIZE);
             dprintf("VSCSI: Sense data, %d bytes:\n", len);
@@ -505,7 +510,7 @@ static void vscsi_command_complete(SCSIBus *bus, int reason, uint32_t tag,
                     buf[12], buf[13], buf[14], buf[15]);
             memcpy(req->sense, buf, len);
             req->senselen = len;
-            sdev->info->read_data(sdev, req->qtag);
+            sdev->info->read_data(sreq);
         }
         return;
     }
@@ -537,12 +542,12 @@ static void vscsi_command_complete(SCSIBus *bus, int reason, uint32_t tag,
      * to write for writes (ie, how much is to be DMA'd)
      */
     if (arg) {
-        buf = sdev->info->get_buf(sdev, tag);
+        buf = sdev->info->get_buf(sreq);
         rc = vscsi_srp_transfer_data(s, req, req->writing, buf, arg);
     }
     if (rc < 0) {
         fprintf(stderr, "VSCSI: RDMA error rc=%d!\n", rc);
-        sdev->info->cancel_io(sdev, req->qtag);
+        sdev->info->cancel_io(sreq);
         vscsi_makeup_sense(s, req, HARDWARE_ERROR, 0, 0);
         vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
         vscsi_put_req(s, req);
@@ -552,9 +557,9 @@ static void vscsi_command_complete(SCSIBus *bus, int reason, uint32_t tag,
     /* Start next chunk */
     req->data_len -= rc;
     if (req->writing) {
-        sdev->info->write_data(sdev, req->qtag);
+        sdev->info->write_data(sreq);
     } else {
-        sdev->info->read_data(sdev, req->qtag);
+        sdev->info->read_data(sreq);
     }
 }
 
@@ -644,7 +649,8 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
 
     req->sdev = sdev;
     req->lun = lun;
-    n = sdev->info->send_command(sdev, req->qtag, srp->cmd.cdb, lun);
+    req->sreq = sdev->info->alloc_req(sdev, req->qtag, lun);
+    n = sdev->info->send_command(req->sreq, srp->cmd.cdb);
 
     dprintf("VSCSI: Queued command tag 0x%x CMD 0x%x ID %d LUN %d ret: %d\n",
             req->qtag, srp->cmd.cdb[0], id, lun, n);
@@ -662,10 +668,10 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
     /* Get transfer direction and initiate transfer */
     if (n > 0) {
         req->data_len = n;
-        sdev->info->read_data(sdev, req->qtag);
+        sdev->info->read_data(req->sreq);
     } else if (n < 0) {
         req->data_len = -n;
-        sdev->info->write_data(sdev, req->qtag);
+        sdev->info->write_data(req->sreq);
     }
     /* Don't touch req here, it may have been recycled already */
 
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 7a07a12..38f97d0 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -48,6 +48,7 @@ typedef struct {
     uint32_t data_len;
     uint32_t residue;
     uint32_t tag;
+    SCSIRequest *req;
     SCSIBus bus;
     BlockConf conf;
     SCSIDevice *scsi_dev;
@@ -190,9 +191,9 @@ static void usb_msd_copy_data(MSDState *s)
     s->data_len -= len;
     if (s->scsi_len == 0 || s->data_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);
         }
     }
 }
@@ -211,14 +212,13 @@ static void usb_msd_send_status(MSDState *s, USBPacket *p)
     memcpy(p->data, &csw, len);
 }
 
-static void usb_msd_command_complete(SCSIBus *bus, int reason, uint32_t tag,
-                                     uint32_t arg)
+static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
 {
-    MSDState *s = DO_UPCAST(MSDState, dev.qdev, bus->qbus.parent);
+    MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->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);
@@ -245,10 +245,12 @@ static void usb_msd_command_complete(SCSIBus *bus, int reason, uint32_t tag,
         } else if (s->data_len == 0) {
             s->mode = USB_MSDM_CSW;
         }
+        scsi_req_unref(req);
+        s->req = NULL;
         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) {
@@ -316,7 +318,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;
 }
@@ -365,14 +367,15 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
                     s->tag, cbw.flags, cbw.cmd_len, s->data_len);
             s->residue = 0;
             s->scsi_len = 0;
-            s->scsi_dev->info->send_command(s->scsi_dev, s->tag, cbw.cmd, 0);
+            s->req = s->scsi_dev->info->alloc_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;
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 08/24] scsi: commonize purging requests
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (6 preceding siblings ...)
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 07/24] scsi: Use 'SCSIRequest' directly Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 09/24] scsi: introduce scsi_req_abort Paolo Bonzini
                   ` (15 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

The code for canceling requests upon reset is already the same.  Clean
it up and move it to scsi-bus.c.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-bus.c     |   12 ++++++++++++
 hw/scsi-disk.c    |   18 ++----------------
 hw/scsi-generic.c |   18 ++----------------
 hw/scsi.h         |    1 +
 4 files changed, 17 insertions(+), 32 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 94a5873..a3eb522 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -549,6 +549,18 @@ void scsi_req_complete(SCSIRequest *req)
     scsi_req_unref(req);
 }
 
+void scsi_device_purge_requests(SCSIDevice *sdev)
+{
+    SCSIRequest *req;
+
+    while (!QTAILQ_EMPTY(&sdev->requests)) {
+        req = QTAILQ_FIRST(&sdev->requests);
+        sdev->info->cancel_io(req);
+        scsi_req_dequeue(req);
+        scsi_req_unref(req);
+    }
+}
+
 static char *scsibus_get_fw_dev_path(DeviceState *dev)
 {
     SCSIDevice *d = (SCSIDevice*)dev;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 1fa5385..4b39f46 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1144,26 +1144,12 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
     return len;
 }
 
-static void scsi_disk_purge_requests(SCSIDiskState *s)
-{
-    SCSIDiskReq *r;
-
-    while (!QTAILQ_EMPTY(&s->qdev.requests)) {
-        r = DO_UPCAST(SCSIDiskReq, req, QTAILQ_FIRST(&s->qdev.requests));
-        if (r->req.aiocb) {
-            bdrv_aio_cancel(r->req.aiocb);
-        }
-        scsi_req_dequeue(&r->req);
-        scsi_req_unref(&r->req);
-    }
-}
-
 static void scsi_disk_reset(DeviceState *dev)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev.qdev, dev);
     uint64_t nb_sectors;
 
-    scsi_disk_purge_requests(s);
+    scsi_device_purge_requests(&s->qdev);
 
     bdrv_get_geometry(s->bs, &nb_sectors);
     nb_sectors /= s->cluster_size;
@@ -1177,7 +1163,7 @@ static void scsi_destroy(SCSIDevice *dev)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
 
-    scsi_disk_purge_requests(s);
+    scsi_device_purge_requests(&s->qdev);
     blockdev_mark_auto_del(s->qdev.conf.bs);
 }
 
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index a4b2a69..b0b6ae7 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -421,32 +421,18 @@ static int get_stream_blocksize(BlockDriverState *bdrv)
     return (buf[9] << 16) | (buf[10] << 8) | buf[11];
 }
 
-static void scsi_generic_purge_requests(SCSIGenericState *s)
-{
-    SCSIGenericReq *r;
-
-    while (!QTAILQ_EMPTY(&s->qdev.requests)) {
-        r = DO_UPCAST(SCSIGenericReq, req, QTAILQ_FIRST(&s->qdev.requests));
-        if (r->req.aiocb) {
-            bdrv_aio_cancel(r->req.aiocb);
-        }
-        scsi_req_dequeue(&r->req);
-        scsi_req_unref(&r->req);
-    }
-}
-
 static void scsi_generic_reset(DeviceState *dev)
 {
     SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev.qdev, dev);
 
-    scsi_generic_purge_requests(s);
+    scsi_device_purge_requests(&s->qdev);
 }
 
 static void scsi_destroy(SCSIDevice *d)
 {
     SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, d);
 
-    scsi_generic_purge_requests(s);
+    scsi_device_purge_requests(&s->qdev);
     blockdev_mark_auto_del(s->qdev.conf.bs);
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index 3b6899c..55a24eb 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -114,5 +114,6 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
 void scsi_req_print(SCSIRequest *req);
 void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
+void scsi_device_purge_requests(SCSIDevice *sdev);
 
 #endif
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 09/24] scsi: introduce scsi_req_abort
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (7 preceding siblings ...)
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 08/24] scsi: commonize purging requests Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 10/24] scsi: introduce scsi_req_cancel Paolo Bonzini
                   ` (14 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

This covers the case of canceling a request's I/O and still
completing it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-bus.c    |    9 +++++++++
 hw/scsi.h        |    1 +
 hw/spapr_vscsi.c |    8 ++------
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index a3eb522..b4ba8cd 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -549,6 +549,15 @@ void scsi_req_complete(SCSIRequest *req)
     scsi_req_unref(req);
 }
 
+void scsi_req_abort(SCSIRequest *req, int status)
+{
+    req->status = status;
+    if (req->dev && req->dev->info->cancel_io) {
+        req->dev->info->cancel_io(req);
+    }
+    scsi_req_complete(req);
+}
+
 void scsi_device_purge_requests(SCSIDevice *sdev)
 {
     SCSIRequest *req;
diff --git a/hw/scsi.h b/hw/scsi.h
index 55a24eb..e915ef8 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -114,6 +114,7 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
 void scsi_req_print(SCSIRequest *req);
 void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
+void scsi_req_abort(SCSIRequest *req, int status);
 void scsi_device_purge_requests(SCSIDevice *sdev);
 
 #endif
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index cf2ed73..678cd00 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -463,10 +463,8 @@ static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
     dprintf("VSCSI: Queued request sense tag 0x%x\n", req->qtag);
     if (n < 0) {
         fprintf(stderr, "VSCSI: REQUEST_SENSE wants write data !?!?!?\n");
-        sdev->info->cancel_io(req->sreq);
         vscsi_makeup_sense(s, req, HARDWARE_ERROR, 0, 0);
-        vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
-        vscsi_put_req(s, req);
+        scsi_req_abort(req->sreq, CHECK_CONDITION);
         return;
     } else if (n == 0) {
         return;
@@ -547,10 +545,8 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
     }
     if (rc < 0) {
         fprintf(stderr, "VSCSI: RDMA error rc=%d!\n", rc);
-        sdev->info->cancel_io(sreq);
         vscsi_makeup_sense(s, req, HARDWARE_ERROR, 0, 0);
-        vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
-        vscsi_put_req(s, req);
+        scsi_req_abort(req->sreq, CHECK_CONDITION);
         return;
     }
 
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 10/24] scsi: introduce scsi_req_cancel
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (8 preceding siblings ...)
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 09/24] scsi: introduce scsi_req_abort Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 11/24] scsi: use scsi_req_complete Paolo Bonzini
                   ` (13 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

This is for when the request must be dropped in the void,
but still memory should be freed.  To this end, the devices
register a second callback in SCSIBusOps.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/esp.c          |   16 ++++++++++++++--
 hw/lsi53c895a.c   |   30 +++++++++++++++++++++++++-----
 hw/scsi-bus.c     |   17 ++++++++++++++---
 hw/scsi-disk.c    |    1 -
 hw/scsi-generic.c |    1 -
 hw/scsi.h         |    2 ++
 hw/spapr_vscsi.c  |   11 ++++++++++-
 hw/usb-msd.c      |   19 +++++++++++++++----
 8 files changed, 80 insertions(+), 17 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 096f4dc..46157a8 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -188,6 +188,17 @@ static void esp_dma_enable(void *opaque, int irq, int level)
     }
 }
 
+static void esp_request_cancelled(SCSIRequest *req)
+{
+    ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
+
+    if (req == s->current_req) {
+        scsi_req_unref(s->current_req);
+        s->current_req = NULL;
+        s->current_dev = NULL;
+    }
+}
+
 static uint32_t get_cmd(ESPState *s, uint8_t *buf)
 {
     uint32_t dmalen;
@@ -210,7 +221,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_req);
+        scsi_req_cancel(s->current_req);
         s->async_len = 0;
     }
 
@@ -720,7 +731,8 @@ void esp_init(target_phys_addr_t espaddr, int it_shift,
 }
 
 static struct SCSIBusOps esp_scsi_ops = {
-    .complete = esp_command_complete
+    .complete = esp_command_complete,
+    .cancel = esp_request_cancelled
 };
 
 static int esp_init1(SysBusDevice *dev)
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index b9febae..fc2b352 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -664,6 +664,26 @@ static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t tag)
     return NULL;
 }
 
+static void lsi_request_cancelled(SCSIRequest *req)
+{
+    LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
+    lsi_request *p;
+
+    if (req == s->current->req) {
+        scsi_req_unref(req);
+        qemu_free(s->current);
+        s->current = NULL;
+        return;
+    }
+
+    p = lsi_find_by_tag(s, req->tag);
+    if (p) {
+        QTAILQ_REMOVE(&s->queue, p, next);
+        scsi_req_unref(req);
+        qemu_free(p);
+    }
+}
+
 /* Record that data is available for a queued command.  Returns zero if
    the device was reselected, nonzero if the IO is deferred.  */
 static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
@@ -931,7 +951,7 @@ static void lsi_do_msgout(LSIState *s)
             /* The ABORT TAG message clears the current I/O process only. */
             DPRINTF("MSG: ABORT TAG tag=0x%x\n", current_tag);
             if (current_req) {
-                current_dev->info->cancel_io(current_req->req);
+                scsi_req_cancel(current_req->req);
             }
             lsi_disconnect(s);
             break;
@@ -956,7 +976,7 @@ static void lsi_do_msgout(LSIState *s)
 
             /* clear the current I/O process */
             if (s->current) {
-                current_dev->info->cancel_io(s->current->req);
+                scsi_req_cancel(s->current->req);
             }
 
             /* As the current implemented devices scsi_disk and scsi_generic
@@ -969,8 +989,7 @@ static void lsi_do_msgout(LSIState *s)
             id = current_tag & 0x0000ff00;
             QTAILQ_FOREACH_SAFE(p, &s->queue, next, p_next) {
                 if ((p->tag & 0x0000ff00) == id) {
-                    current_dev->info->cancel_io(p->req);
-                    QTAILQ_REMOVE(&s->queue, p, next);
+                    scsi_req_cancel(p->req);
                 }
             }
 
@@ -2227,7 +2246,8 @@ static int lsi_scsi_uninit(PCIDevice *d)
 }
 
 static struct SCSIBusOps lsi_scsi_ops = {
-    .complete = lsi_command_complete
+    .complete = lsi_command_complete,
+    .cancel = lsi_request_cancelled
 };
 
 static int lsi_scsi_init(PCIDevice *dev)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index b4ba8cd..c0ab4a6 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -549,6 +549,19 @@ void scsi_req_complete(SCSIRequest *req)
     scsi_req_unref(req);
 }
 
+void scsi_req_cancel(SCSIRequest *req)
+{
+    if (req->dev && req->dev->info->cancel_io) {
+        req->dev->info->cancel_io(req);
+    }
+    scsi_req_ref(req);
+    scsi_req_dequeue(req);
+    if (req->bus->ops->cancel) {
+        req->bus->ops->cancel(req);
+    }
+    scsi_req_unref(req);
+}
+
 void scsi_req_abort(SCSIRequest *req, int status)
 {
     req->status = status;
@@ -564,9 +577,7 @@ void scsi_device_purge_requests(SCSIDevice *sdev)
 
     while (!QTAILQ_EMPTY(&sdev->requests)) {
         req = QTAILQ_FIRST(&sdev->requests);
-        sdev->info->cancel_io(req);
-        scsi_req_dequeue(req);
-        scsi_req_unref(req);
+        scsi_req_cancel(req);
     }
 }
 
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 4b39f46..1592c58 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -140,7 +140,6 @@ static void scsi_cancel_io(SCSIRequest *req)
         bdrv_aio_cancel(r->req.aiocb);
     }
     r->req.aiocb = NULL;
-    scsi_req_dequeue(&r->req);
 }
 
 static void scsi_read_complete(void * opaque, int ret)
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index b0b6ae7..16bf185 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -120,7 +120,6 @@ static void scsi_cancel_io(SCSIRequest *req)
         bdrv_aio_cancel(r->req.aiocb);
     }
     r->req.aiocb = NULL;
-    scsi_req_dequeue(&r->req);
 }
 
 static int execute_command(BlockDriverState *bdrv,
diff --git a/hw/scsi.h b/hw/scsi.h
index e915ef8..87a368a 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -78,6 +78,7 @@ struct SCSIDeviceInfo {
 
 struct SCSIBusOps {
     void (*complete)(SCSIRequest *req, int reason, uint32_t arg);
+    void (*cancel)(SCSIRequest *req);
 };
 
 struct SCSIBus {
@@ -115,6 +116,7 @@ void scsi_req_print(SCSIRequest *req);
 void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
 void scsi_req_abort(SCSIRequest *req, int status);
+void scsi_req_cancel(SCSIRequest *req);
 void scsi_device_purge_requests(SCSIDevice *sdev);
 
 #endif
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 678cd00..5b97a83 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -559,6 +559,14 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
     }
 }
 
+static void vscsi_request_cancelled(SCSIRequest *sreq)
+{
+    VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
+    vscsi_req *req = vscsi_find_req(s, sreq);
+
+    vscsi_put_req(s, req);
+}
+
 static void vscsi_process_login(VSCSIState *s, vscsi_req *req)
 {
     union viosrp_iu *iu = &req->iu;
@@ -910,7 +918,8 @@ static int vscsi_do_crq(struct VIOsPAPRDevice *dev, uint8_t *crq_data)
 }
 
 static struct SCSIBusOps vscsi_scsi_ops = {
-    .complete = vscsi_command_complete
+    .complete = vscsi_command_complete,
+    .cancel = vscsi_request_cancelled
 };
 
 static int spapr_vscsi_init(VIOsPAPRDevice *dev)
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 38f97d0..9b238e8 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -264,6 +264,18 @@ static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
     }
 }
 
+static void usb_msd_request_cancelled(SCSIRequest *req)
+{
+    MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
+
+    if (req == s->req) {
+        scsi_req_unref(s->req);
+        s->req = NULL;
+        s->packet = NULL;
+        s->scsi_len = 0;
+    }
+}
+
 static void usb_msd_handle_reset(USBDevice *dev)
 {
     MSDState *s = (MSDState *)dev;
@@ -318,9 +330,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->req);
-    s->packet = NULL;
-    s->scsi_len = 0;
+    scsi_req_cancel(s->req);
 }
 
 static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
@@ -491,7 +501,8 @@ static void usb_msd_password_cb(void *opaque, int err)
 }
 
 static struct SCSIBusOps usb_msd_scsi_ops = {
-    .complete = usb_msd_command_complete
+    .complete = usb_msd_command_complete,
+    .cancel = usb_msd_request_cancelled
 };
 
 static int usb_msd_initfn(USBDevice *dev)
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 11/24] scsi: use scsi_req_complete
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (9 preceding siblings ...)
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 10/24] scsi: introduce scsi_req_cancel Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 12/24] scsi: Update sense code handling Paolo Bonzini
                   ` (12 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-generic.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 16bf185..2b3ebd9 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -287,7 +287,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
 {
     SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
     SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
-    SCSIBus *bus;
     int ret;
 
     scsi_req_enqueue(req);
@@ -304,8 +303,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
         s->sensebuf[6] = 0x00;
         s->senselen = 7;
         s->driver_status = SG_ERR_DRIVER_SENSE;
-        bus = scsi_bus_from_device(&s->qdev);
-        bus->ops->complete(req, SCSI_REASON_DONE, CHECK_CONDITION);
+        r->req.status = CHECK_CONDITION;
+        scsi_req_complete(&r->req);
         return 0;
     }
 
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 12/24] scsi: Update sense code handling
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (10 preceding siblings ...)
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 11/24] scsi: use scsi_req_complete Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 13/24] scsi: do not call send_command directly Paolo Bonzini
                   ` (11 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch, Hannes Reinecke

From: Hannes Reinecke <hare@suse.de>

The SCSI spec has a quite detailed list of sense codes available.
It even mandates the use of specific ones for some failure cases.
The current implementation just has one type of generic error
which is actually a violation of the spec in certain cases.
This patch introduces various predefined sense codes to have the
sense code reporting more in line with the spec.

On top of Hannes's patch I fixed the reply to REQUEST SENSE commands
with DESC=0 and a small (<18) length.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-bus.c     |   91 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/scsi-disk.c    |   82 ++++++++++++++++++++++-------------------------
 hw/scsi-generic.c |   63 ++++++++++++++++++++++++-------------
 hw/scsi.h         |   39 ++++++++++++++++++++++-
 4 files changed, 208 insertions(+), 67 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index c0ab4a6..6d68862 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -154,7 +154,7 @@ void scsi_req_enqueue(SCSIRequest *req)
     QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
 }
 
-void scsi_req_dequeue(SCSIRequest *req)
+static void scsi_req_dequeue(SCSIRequest *req)
 {
     trace_scsi_req_dequeue(req->dev->id, req->lun, req->tag);
     if (req->enqueued) {
@@ -391,6 +391,95 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf)
     return 0;
 }
 
+/*
+ * Predefined sense codes
+ */
+
+/* No sense data available */
+const struct SCSISense sense_code_NO_SENSE = {
+    .key = NO_SENSE , .asc = 0x00 , .ascq = 0x00
+};
+
+/* LUN not ready, Manual intervention required */
+const struct SCSISense sense_code_LUN_NOT_READY = {
+    .key = NOT_READY, .asc = 0x04, .ascq = 0x03
+};
+
+/* LUN not ready, Medium not present */
+const struct SCSISense sense_code_NO_MEDIUM = {
+    .key = NOT_READY, .asc = 0x3a, .ascq = 0x00
+};
+
+/* Hardware error, internal target failure */
+const struct SCSISense sense_code_TARGET_FAILURE = {
+    .key = HARDWARE_ERROR, .asc = 0x44, .ascq = 0x00
+};
+
+/* Illegal request, invalid command operation code */
+const struct SCSISense sense_code_INVALID_OPCODE = {
+    .key = ILLEGAL_REQUEST, .asc = 0x20, .ascq = 0x00
+};
+
+/* Illegal request, LBA out of range */
+const struct SCSISense sense_code_LBA_OUT_OF_RANGE = {
+    .key = ILLEGAL_REQUEST, .asc = 0x21, .ascq = 0x00
+};
+
+/* Illegal request, Invalid field in CDB */
+const struct SCSISense sense_code_INVALID_FIELD = {
+    .key = ILLEGAL_REQUEST, .asc = 0x24, .ascq = 0x00
+};
+
+/* Illegal request, LUN not supported */
+const struct SCSISense sense_code_LUN_NOT_SUPPORTED = {
+    .key = ILLEGAL_REQUEST, .asc = 0x25, .ascq = 0x00
+};
+
+/* Command aborted, I/O process terminated */
+const struct SCSISense sense_code_IO_ERROR = {
+    .key = ABORTED_COMMAND, .asc = 0x00, .ascq = 0x06
+};
+
+/* Command aborted, I_T Nexus loss occurred */
+const struct SCSISense sense_code_I_T_NEXUS_LOSS = {
+    .key = ABORTED_COMMAND, .asc = 0x29, .ascq = 0x07
+};
+
+/* Command aborted, Logical Unit failure */
+const struct SCSISense sense_code_LUN_FAILURE = {
+    .key = ABORTED_COMMAND, .asc = 0x3e, .ascq = 0x01
+};
+
+/*
+ * scsi_build_sense
+ *
+ * Build a sense buffer
+ */
+int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed)
+{
+    if (!fixed && len < 8) {
+        return 0;
+    }
+
+    memset(buf, 0, len);
+    if (fixed) {
+        /* Return fixed format sense buffer */
+        buf[0] = 0xf0;
+        buf[2] = sense.key;
+        buf[7] = 7;
+        buf[12] = sense.asc;
+        buf[13] = sense.ascq;
+        return MIN(len, 18);
+    } else {
+        /* Return descriptor format sense buffer */
+        buf[0] = 0x72;
+        buf[1] = sense.key;
+        buf[2] = sense.asc;
+        buf[3] = sense.ascq;
+        return 8;
+    }
+}
+
 static const char *scsi_command_name(uint8_t cmd)
 {
     static const char *names[] = {
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 1592c58..d60668e 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -49,10 +49,6 @@ do { fprintf(stderr, "scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
 
 typedef struct SCSIDiskState SCSIDiskState;
 
-typedef struct SCSISense {
-    uint8_t key;
-} SCSISense;
-
 typedef struct SCSIDiskReq {
     SCSIRequest req;
     /* ??? We should probably keep track of whether the data transfer is
@@ -108,24 +104,19 @@ static void scsi_disk_clear_sense(SCSIDiskState *s)
     memset(&s->sense, 0, sizeof(s->sense));
 }
 
-static void scsi_disk_set_sense(SCSIDiskState *s, uint8_t key)
-{
-    s->sense.key = key;
-}
-
-static void scsi_req_set_status(SCSIDiskReq *r, int status, int sense_code)
+static void scsi_req_set_status(SCSIDiskReq *r, int status, SCSISense sense)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
 
     r->req.status = status;
-    scsi_disk_set_sense(s, sense_code);
+    s->sense = sense;
 }
 
 /* Helper function for command completion.  */
-static void scsi_command_complete(SCSIDiskReq *r, int status, int sense)
+static void scsi_command_complete(SCSIDiskReq *r, int status, SCSISense sense)
 {
-    DPRINTF("Command complete tag=0x%x status=%d sense=%d\n",
-            r->req.tag, status, sense);
+    DPRINTF("Command complete tag=0x%x status=%d sense=%d/%d/%d\n",
+            r->req.tag, status, sense.key, sense.asc, sense.ascq);
     scsi_req_set_status(r, status, sense);
     scsi_req_complete(&r->req);
 }
@@ -179,7 +170,7 @@ 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, NO_SENSE);
+        scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
         return;
     }
 
@@ -222,8 +213,13 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
         if (type == SCSI_REQ_STATUS_RETRY_READ) {
             scsi_req_data(&r->req, 0);
         }
-        scsi_command_complete(r, CHECK_CONDITION,
-                HARDWARE_ERROR);
+        if (error == ENOMEM) {
+            scsi_command_complete(r, CHECK_CONDITION,
+                                  SENSE_CODE(TARGET_FAILURE));
+        } else {
+            scsi_command_complete(r, CHECK_CONDITION,
+                                  SENSE_CODE(IO_ERROR));
+        }
         bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read);
     }
 
@@ -248,7 +244,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, NO_SENSE);
+        scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
     } else {
         len = r->sector_count * 512;
         if (len > SCSI_DMA_BUF_SIZE) {
@@ -275,7 +271,7 @@ static int scsi_write_data(SCSIRequest *req)
         r->req.aiocb = bdrv_aio_writev(s->bs, r->sector, &r->qiov, n,
                                    scsi_write_complete, r);
         if (r->req.aiocb == NULL) {
-            scsi_write_complete(r, -EIO);
+            scsi_write_complete(r, -ENOMEM);
         }
     } else {
         /* Invoke completion routine to fetch data from host.  */
@@ -313,7 +309,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, NO_SENSE);
+                    scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
                 }
             }
         }
@@ -812,19 +808,8 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
     case REQUEST_SENSE:
         if (req->cmd.xfer < 4)
             goto illegal_request;
-        memset(outbuf, 0, 4);
-        buflen = 4;
-        if (s->sense.key == NOT_READY && req->cmd.xfer >= 18) {
-            memset(outbuf, 0, 18);
-            buflen = 18;
-            outbuf[7] = 10;
-            /* asc 0x3a, ascq 0: Medium not present */
-            outbuf[12] = 0x3a;
-            outbuf[13] = 0;
-        }
-        outbuf[0] = 0xf0;
-        outbuf[1] = 0;
-        outbuf[2] = s->sense.key;
+        buflen = scsi_build_sense(s->sense, outbuf, req->cmd.xfer,
+                                  req->cmd.xfer > 13);
         scsi_disk_clear_sense(s);
         break;
     case INQUIRY:
@@ -962,17 +947,22 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
         }
         break;
     default:
-        goto illegal_request;
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
+        return -1;
     }
-    scsi_req_set_status(r, GOOD, NO_SENSE);
+    scsi_req_set_status(r, GOOD, SENSE_CODE(NO_SENSE));
     return buflen;
 
 not_ready:
-    scsi_command_complete(r, CHECK_CONDITION, NOT_READY);
+    if (!bdrv_is_inserted(s->bs)) {
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(NO_MEDIUM));
+    } else {
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(LUN_NOT_READY));
+    }
     return -1;
 
 illegal_request:
-    scsi_command_complete(r, CHECK_CONDITION, ILLEGAL_REQUEST);
+    scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_FIELD));
     return -1;
 }
 
@@ -999,7 +989,8 @@ 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);
-        goto fail;
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_OPCODE));
+        return 0;
     }
 #ifdef DEBUG_SCSI
     {
@@ -1014,8 +1005,11 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
     if (req->lun || buf[1] >> 5) {
         /* Only LUN 0 supported.  */
         DPRINTF("Unimplemented LUN %d\n", req->lun ? req->lun : buf[1] >> 5);
-        if (command != REQUEST_SENSE && command != INQUIRY)
-            goto fail;
+        if (command != REQUEST_SENSE && command != INQUIRY) {
+            scsi_command_complete(r, CHECK_CONDITION,
+                                  SENSE_CODE(LUN_NOT_SUPPORTED));
+            return 0;
+        }
     }
     switch (command) {
     case TEST_UNIT_READY:
@@ -1123,15 +1117,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));
+        return 0;
     fail:
-        scsi_command_complete(r, CHECK_CONDITION, ILLEGAL_REQUEST);
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(INVALID_FIELD));
         return 0;
     illegal_lba:
-        scsi_command_complete(r, CHECK_CONDITION, HARDWARE_ERROR);
+        scsi_command_complete(r, CHECK_CONDITION, SENSE_CODE(LBA_OUT_OF_RANGE));
         return 0;
     }
     if (r->sector_count == 0 && r->iov.iov_len == 0) {
-        scsi_command_complete(r, GOOD, NO_SENSE);
+        scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
     }
     len = r->sector_count * 512 + r->iov.iov_len;
     if (is_write) {
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 2b3ebd9..a9bbca6 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -66,6 +66,19 @@ struct SCSIGenericState
     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 SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
 {
     SCSIRequest *req;
@@ -91,9 +104,22 @@ static void scsi_command_complete(void *opaque, int ret)
     if (s->driver_status & SG_ERR_DRIVER_SENSE)
         s->senselen = r->io_header.sb_len_wr;
 
-    if (ret != 0)
-        r->req.status = BUSY;
-    else {
+    if (ret != 0) {
+        switch (ret) {
+        case -EINVAL:
+            r->req.status = CHECK_CONDITION;
+            scsi_set_sense(s, SENSE_CODE(INVALID_FIELD));
+            break;
+        case -ENOMEM:
+            r->req.status = CHECK_CONDITION;
+            scsi_set_sense(s, SENSE_CODE(TARGET_FAILURE));
+            break;
+        default:
+            r->req.status = CHECK_CONDITION;
+            scsi_set_sense(s, SENSE_CODE(IO_ERROR));
+            break;
+        }
+    } else {
         if (s->driver_status & SG_ERR_DRIVER_TIMEOUT) {
             r->req.status = BUSY;
             BADF("Driver Timeout\n");
@@ -143,7 +169,7 @@ static int execute_command(BlockDriverState *bdrv,
     r->req.aiocb = bdrv_aio_ioctl(bdrv, SG_IO, &r->io_header, complete, r);
     if (r->req.aiocb == NULL) {
         BADF("execute_command: read failed !\n");
-        return -1;
+        return -ENOMEM;
     }
 
     return 0;
@@ -196,12 +222,14 @@ static void scsi_read_data(SCSIRequest *req)
                 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);
         return;
     }
 
     ret = execute_command(s->bs, r, SG_DXFER_FROM_DEV, scsi_read_complete);
-    if (ret == -1) {
-        scsi_command_complete(r, -EINVAL);
+    if (ret < 0) {
+        scsi_command_complete(r, ret);
         return;
     }
 }
@@ -243,8 +271,8 @@ static int scsi_write_data(SCSIRequest *req)
     }
 
     ret = execute_command(s->bs, r, SG_DXFER_TO_DEV, scsi_write_complete);
-    if (ret == -1) {
-        scsi_command_complete(r, -EINVAL);
+    if (ret < 0) {
+        scsi_command_complete(r, ret);
         return 1;
     }
 
@@ -293,16 +321,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
     if (cmd[0] != REQUEST_SENSE &&
         (req->lun != s->lun || (cmd[1] >> 5) != s->lun)) {
         DPRINTF("Unimplemented LUN %d\n", req->lun ? req->lun : cmd[1] >> 5);
-
-        s->sensebuf[0] = 0x70;
-        s->sensebuf[1] = 0x00;
-        s->sensebuf[2] = ILLEGAL_REQUEST;
-        s->sensebuf[3] = 0x00;
-        s->sensebuf[4] = 0x00;
-        s->sensebuf[5] = 0x00;
-        s->sensebuf[6] = 0x00;
-        s->senselen = 7;
-        s->driver_status = SG_ERR_DRIVER_SENSE;
+        scsi_set_sense(s, SENSE_CODE(LUN_NOT_SUPPORTED));
         r->req.status = CHECK_CONDITION;
         scsi_req_complete(&r->req);
         return 0;
@@ -310,8 +329,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
 
     if (-1 == scsi_req_parse(&r->req, cmd)) {
         BADF("Unsupported command length, command %x\n", cmd[0]);
-        scsi_req_dequeue(&r->req);
-        scsi_req_unref(&r->req);
+        scsi_command_complete(r, -EINVAL);
         return 0;
     }
     scsi_req_fixup(&r->req);
@@ -335,8 +353,9 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
         r->buflen = 0;
         r->buf = NULL;
         ret = execute_command(s->bs, r, SG_DXFER_NONE, scsi_command_complete);
-        if (ret == -1) {
-            scsi_command_complete(r, -EINVAL);
+        if (ret < 0) {
+            scsi_command_complete(r, ret);
+            return 0;
         }
         return 0;
     }
diff --git a/hw/scsi.h b/hw/scsi.h
index 87a368a..028a7c9 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -27,6 +27,12 @@ enum SCSIXferMode {
     SCSI_XFER_TO_DEV,    /*  WRITE, MODE_SELECT, ...         */
 };
 
+typedef struct SCSISense {
+    uint8_t key;
+    uint8_t asc;
+    uint8_t ascq;
+} SCSISense;
+
 struct SCSIRequest {
     SCSIBus           *bus;
     SCSIDevice        *dev;
@@ -104,10 +110,41 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
                                       int unit, bool removable);
 int scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
 
+/*
+ * Predefined sense codes
+ */
+
+/* No sense data available */
+extern const struct SCSISense sense_code_NO_SENSE;
+/* LUN not ready, Manual intervention required */
+extern const struct SCSISense sense_code_LUN_NOT_READY;
+/* LUN not ready, Medium not present */
+extern const struct SCSISense sense_code_NO_MEDIUM;
+/* Hardware error, internal target failure */
+extern const struct SCSISense sense_code_TARGET_FAILURE;
+/* Illegal request, invalid command operation code */
+extern const struct SCSISense sense_code_INVALID_OPCODE;
+/* Illegal request, LBA out of range */
+extern const struct SCSISense sense_code_LBA_OUT_OF_RANGE;
+/* Illegal request, Invalid field in CDB */
+extern const struct SCSISense sense_code_INVALID_FIELD;
+/* Illegal request, LUN not supported */
+extern const struct SCSISense sense_code_LUN_NOT_SUPPORTED;
+/* Command aborted, I/O process terminated */
+extern const struct SCSISense sense_code_IO_ERROR;
+/* Command aborted, I_T Nexus loss occurred */
+extern const struct SCSISense sense_code_I_T_NEXUS_LOSS;
+/* Command aborted, Logical Unit failure */
+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, uint32_t lun);
 void scsi_req_enqueue(SCSIRequest *req);
 void scsi_req_free(SCSIRequest *req);
-void scsi_req_dequeue(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
 void scsi_req_unref(SCSIRequest *req);
 
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 13/24] scsi: do not call send_command directly
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (11 preceding siblings ...)
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 12/24] scsi: Update sense code handling Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 14/24] scsi: introduce scsi_req_new Paolo Bonzini
                   ` (10 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

Move the common part of scsi-disk.c and scsi-generic.c to the SCSI layer.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/esp.c          |    2 +-
 hw/lsi53c895a.c   |    2 +-
 hw/scsi-bus.c     |    3 ++-
 hw/scsi-disk.c    |    1 -
 hw/scsi-generic.c |    1 -
 hw/scsi.h         |    2 +-
 hw/spapr_vscsi.c  |    4 ++--
 hw/usb-msd.c      |    2 +-
 8 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 46157a8..3a65aed 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -245,7 +245,7 @@ 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;
     s->current_req = s->current_dev->info->alloc_req(s->current_dev, 0, lun);
-    datalen = s->current_dev->info->send_command(s->current_req, buf);
+    datalen = scsi_req_enqueue(s->current_req, buf);
     s->ti_size = datalen;
     if (datalen != 0) {
         s->rregs[ESP_RSTAT] = STAT_TC;
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index fc2b352..777b598 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -791,7 +791,7 @@ static void lsi_do_command(LSIState *s)
     s->current->req = dev->info->alloc_req(dev, s->current->tag,
                                            s->current_lun);
 
-    n = dev->info->send_command(s->current->req, buf);
+    n = scsi_req_enqueue(s->current->req, buf);
     if (n > 0) {
         lsi_set_phase(s, PHASE_DI);
         dev->info->read_data(s->current->req);
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 6d68862..a6dc706 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -146,12 +146,13 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
     return req;
 }
 
-void scsi_req_enqueue(SCSIRequest *req)
+int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
 {
     assert(!req->enqueued);
     scsi_req_ref(req);
     req->enqueued = true;
     QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
+    return req->dev->info->send_command(req, buf);
 }
 
 static void scsi_req_dequeue(SCSIRequest *req)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index d60668e..083097e 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -981,7 +981,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
     uint8_t *outbuf;
     int rc;
 
-    scsi_req_enqueue(req);
     command = buf[0];
     outbuf = (uint8_t *)r->iov.iov_base;
     is_write = 0;
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index a9bbca6..4cf8e3b 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -317,7 +317,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
     SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
     int ret;
 
-    scsi_req_enqueue(req);
     if (cmd[0] != REQUEST_SENSE &&
         (req->lun != s->lun || (cmd[1] >> 5) != s->lun)) {
         DPRINTF("Unimplemented LUN %d\n", req->lun ? req->lun : cmd[1] >> 5);
diff --git a/hw/scsi.h b/hw/scsi.h
index 028a7c9..36cb64c 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -143,7 +143,7 @@ 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, uint32_t lun);
-void scsi_req_enqueue(SCSIRequest *req);
+int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf);
 void scsi_req_free(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
 void scsi_req_unref(SCSIRequest *req);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 5b97a83..307a17f 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -459,7 +459,7 @@ static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
     cdb[4] = 96;
     cdb[5] = 0;
     req->sensing = 1;
-    n = sdev->info->send_command(req->sreq, cdb);
+    n = scsi_req_enqueue(req->sreq, cdb);
     dprintf("VSCSI: Queued request sense tag 0x%x\n", req->qtag);
     if (n < 0) {
         fprintf(stderr, "VSCSI: REQUEST_SENSE wants write data !?!?!?\n");
@@ -654,7 +654,7 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
     req->sdev = sdev;
     req->lun = lun;
     req->sreq = sdev->info->alloc_req(sdev, req->qtag, lun);
-    n = sdev->info->send_command(req->sreq, srp->cmd.cdb);
+    n = scsi_req_enqueue(req->sreq, srp->cmd.cdb);
 
     dprintf("VSCSI: Queued command tag 0x%x CMD 0x%x ID %d LUN %d ret: %d\n",
             req->qtag, srp->cmd.cdb[0], id, lun, n);
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 9b238e8..1375e82 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -378,7 +378,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
             s->residue = 0;
             s->scsi_len = 0;
             s->req = s->scsi_dev->info->alloc_req(s->scsi_dev, s->tag, 0);
-            s->scsi_dev->info->send_command(s->req, cbw.cmd);
+            scsi_req_enqueue(s->req, cbw.cmd);
             /* ??? Should check that USB and SCSI data transfer
                directions match.  */
             if (s->residue == 0) {
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 14/24] scsi: introduce scsi_req_new
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (12 preceding siblings ...)
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 13/24] scsi: do not call send_command directly Paolo Bonzini
@ 2011-05-23 16:08 ` Paolo Bonzini
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 15/24] scsi: introduce scsi_req_continue Paolo Bonzini
                   ` (9 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/esp.c         |    2 +-
 hw/lsi53c895a.c  |    3 +--
 hw/scsi-bus.c    |    5 +++++
 hw/scsi.h        |    1 +
 hw/spapr_vscsi.c |    2 +-
 hw/usb-msd.c     |    2 +-
 6 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 3a65aed..ad364b5 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -244,7 +244,7 @@ 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;
-    s->current_req = s->current_dev->info->alloc_req(s->current_dev, 0, lun);
+    s->current_req = scsi_req_new(s->current_dev, 0, lun);
     datalen = scsi_req_enqueue(s->current_req, buf);
     s->ti_size = datalen;
     if (datalen != 0) {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 777b598..608ad67 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -788,8 +788,7 @@ 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->alloc_req(dev, s->current->tag,
-                                           s->current_lun);
+    s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun);
 
     n = scsi_req_enqueue(s->current->req, buf);
     if (n > 0) {
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index a6dc706..17c8968 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -146,6 +146,11 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t l
     return req;
 }
 
+SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
+{
+    return d->info->alloc_req(d, tag, lun);
+}
+
 int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
 {
     assert(!req->enqueued);
diff --git a/hw/scsi.h b/hw/scsi.h
index 36cb64c..2d66ef4 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -143,6 +143,7 @@ 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, uint32_t lun);
+SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun);
 int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf);
 void scsi_req_free(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 307a17f..1bb4bf4 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -653,7 +653,7 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
 
     req->sdev = sdev;
     req->lun = lun;
-    req->sreq = sdev->info->alloc_req(sdev, req->qtag, lun);
+    req->sreq = scsi_req_new(sdev, req->qtag, lun);
     n = scsi_req_enqueue(req->sreq, srp->cmd.cdb);
 
     dprintf("VSCSI: Queued command tag 0x%x CMD 0x%x ID %d LUN %d ret: %d\n",
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 1375e82..c52e394 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -377,7 +377,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
                     s->tag, cbw.flags, cbw.cmd_len, s->data_len);
             s->residue = 0;
             s->scsi_len = 0;
-            s->req = s->scsi_dev->info->alloc_req(s->scsi_dev, s->tag, 0);
+            s->req = scsi_req_new(s->scsi_dev, s->tag, 0);
             scsi_req_enqueue(s->req, cbw.cmd);
             /* ??? Should check that USB and SCSI data transfer
                directions match.  */
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 15/24] scsi: introduce scsi_req_continue
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (13 preceding siblings ...)
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 14/24] scsi: introduce scsi_req_new Paolo Bonzini
@ 2011-05-23 16:09 ` Paolo Bonzini
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 16/24] scsi: introduce scsi_req_get_buf Paolo Bonzini
                   ` (8 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 hw/esp.c         |   26 ++++++++++----------------
 hw/lsi53c895a.c  |   22 ++++++++--------------
 hw/scsi-bus.c    |   16 +++++++++++++---
 hw/scsi.h        |    1 +
 hw/spapr_vscsi.c |   26 ++++++++++----------------
 hw/usb-msd.c     |   15 ++++-----------
 trace-events     |    1 +
 7 files changed, 47 insertions(+), 60 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index ad364b5..ced8945 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -253,11 +253,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_req);
         } else {
             s->rregs[ESP_RSTAT] |= STAT_DO;
-            s->current_dev->info->write_data(s->current_req);
         }
+        scsi_req_continue(s->current_req);
     }
     s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
     s->rregs[ESP_RSEQ] = SEQ_CD;
@@ -383,22 +382,17 @@ static void esp_do_dma(ESPState *s)
     else
         s->ti_size -= len;
     if (s->async_len == 0) {
-        if (to_device) {
-            // ti_size is negative
-            s->current_dev->info->write_data(s->current_req);
-        } else {
-            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.  */
-            if (s->dma_left == 0 && s->ti_size > 0) {
-                esp_dma_done(s);
-            }
+        scsi_req_continue(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.  */
+        if (to_device || s->dma_left != 0 || s->ti_size == 0) {
+            return;
         }
-    } else {
-        /* Partially filled a scsi buffer. Complete immediately.  */
-        esp_dma_done(s);
     }
+
+    /* Partially filled a scsi buffer. Complete immediately.  */
+    esp_dma_done(s);
 }
 
 static void esp_command_complete(SCSIRequest *req, int reason, uint32_t arg)
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 608ad67..3d5febb 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -580,13 +580,7 @@ static void lsi_do_dma(LSIState *s, int out)
     s->current->dma_len -= count;
     if (s->current->dma_len == 0) {
         s->current->dma_buf = NULL;
-        if (out) {
-            /* Write the data.  */
-            dev->info->write_data(s->current->req);
-        } else {
-            /* Request any remaining data.  */
-            dev->info->read_data(s->current->req);
-        }
+        scsi_req_continue(s->current->req);
     } else {
         s->current->dma_buf += count;
         lsi_resume_script(s);
@@ -791,14 +785,14 @@ static void lsi_do_command(LSIState *s)
     s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun);
 
     n = scsi_req_enqueue(s->current->req, buf);
-    if (n > 0) {
-        lsi_set_phase(s, PHASE_DI);
-        dev->info->read_data(s->current->req);
-    } else if (n < 0) {
-        lsi_set_phase(s, PHASE_DO);
-        dev->info->write_data(s->current->req);
+    if (n) {
+        if (n > 0) {
+            lsi_set_phase(s, PHASE_DI);
+        } else if (n < 0) {
+            lsi_set_phase(s, PHASE_DO);
+        }
+        scsi_req_continue(s->current->req);
     }
-
     if (!s->command_complete) {
         if (n) {
             /* Command did not complete immediately so disconnect.  */
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 17c8968..4541877 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -596,11 +596,21 @@ void scsi_req_unref(SCSIRequest *req)
     }
 }
 
+/* Tell the device that we finished processing this chunk of I/O.  It
+   will start the next chunk or complete the command.  */
+void scsi_req_continue(SCSIRequest *req)
+{
+    trace_scsi_req_continue(req->dev->id, req->lun, req->tag);
+    if (req->cmd.mode == SCSI_XFER_TO_DEV) {
+        req->dev->info->write_data(req);
+    } else {
+        req->dev->info->read_data(req);
+    }
+}
+
 /* Called by the devices when data is ready for the HBA.  The HBA should
    start a DMA operation to read or fill the device's data buffer.
-   Once it completes, calling one of req->dev->info->read_data or
-   req->dev->info->write_data (depending on the direction of the
-   transfer) will restart I/O.  */
+   Once it completes, calling scsi_req_continue will restart I/O.  */
 void scsi_req_data(SCSIRequest *req, int len)
 {
     trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
diff --git a/hw/scsi.h b/hw/scsi.h
index 2d66ef4..9038812 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -151,6 +151,7 @@ void scsi_req_unref(SCSIRequest *req);
 
 int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
 void scsi_req_print(SCSIRequest *req);
+void scsi_req_continue(SCSIRequest *req);
 void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
 void scsi_req_abort(SCSIRequest *req, int status);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 1bb4bf4..3f65a87 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -448,7 +448,6 @@ static int vscsi_preprocess_desc(vscsi_req *req)
 
 static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
 {
-    SCSIDevice *sdev = req->sdev;
     uint8_t *cdb = req->iu.srp.cmd.cdb;
     int n;
 
@@ -469,7 +468,7 @@ static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
     } else if (n == 0) {
         return;
     }
-    sdev->info->read_data(req->sreq);
+    scsi_req_continue(req->sreq);
 }
 
 /* Callback to indicate that the SCSI layer has completed a transfer.  */
@@ -508,7 +507,7 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
                     buf[12], buf[13], buf[14], buf[15]);
             memcpy(req->sense, buf, len);
             req->senselen = len;
-            sdev->info->read_data(sreq);
+            scsi_req_continue(req->sreq);
         }
         return;
     }
@@ -552,11 +551,7 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
 
     /* Start next chunk */
     req->data_len -= rc;
-    if (req->writing) {
-        sdev->info->write_data(sreq);
-    } else {
-        sdev->info->read_data(sreq);
-    }
+    scsi_req_continue(sreq);
 }
 
 static void vscsi_request_cancelled(SCSIRequest *sreq)
@@ -667,15 +662,14 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
 
         /* Preprocess RDMA descriptors */
         vscsi_preprocess_desc(req);
-    }
 
-    /* Get transfer direction and initiate transfer */
-    if (n > 0) {
-        req->data_len = n;
-        sdev->info->read_data(req->sreq);
-    } else if (n < 0) {
-        req->data_len = -n;
-        sdev->info->write_data(req->sreq);
+        /* Get transfer direction and initiate transfer */
+        if (n > 0) {
+            req->data_len = n;
+        } else if (n < 0) {
+            req->data_len = -n;
+        }
+        scsi_req_continue(req->sreq);
     }
     /* Don't touch req here, it may have been recycled already */
 
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index c52e394..5f84256 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -190,11 +190,7 @@ static void usb_msd_copy_data(MSDState *s)
     s->scsi_buf += len;
     s->data_len -= len;
     if (s->scsi_len == 0 || s->data_len == 0) {
-        if (s->mode == USB_MSDM_DATAIN) {
-            s->scsi_dev->info->read_data(s->req);
-        } else if (s->mode == USB_MSDM_DATAOUT) {
-            s->scsi_dev->info->write_data(s->req);
-        }
+        scsi_req_continue(s->req);
     }
 }
 
@@ -249,6 +245,7 @@ static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
         s->req = NULL;
         return;
     }
+    assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == SCSI_XFER_TO_DEV));
     s->scsi_len = arg;
     s->scsi_buf = s->scsi_dev->info->get_buf(req);
     if (p) {
@@ -381,12 +378,8 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
             scsi_req_enqueue(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->req);
-                } else if (s->mode == USB_MSDM_DATAOUT) {
-                    s->scsi_dev->info->write_data(s->req);
-                }
+            if (s->mode != USB_MSDM_CSW && s->residue == 0) {
+                scsi_req_continue(s->req);
             }
             ret = len;
             break;
diff --git a/trace-events b/trace-events
index 98e7b2d..4bdc46c 100644
--- a/trace-events
+++ b/trace-events
@@ -209,6 +209,7 @@ disable usb_set_device_feature(int addr, int feature, int ret) "dev %d, feature
 disable scsi_req_alloc(int target, int lun, int tag) "target %d lun %d tag %d"
 disable scsi_req_data(int target, int lun, int tag, int len) "target %d lun %d tag %d len %d"
 disable scsi_req_dequeue(int target, int lun, int tag) "target %d lun %d tag %d"
+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, uint64_t lba) "target %d lun %d tag %d command %d dir %d length %d lba %"PRIu64""
 disable scsi_req_parse_bad(int target, int lun, int tag, int cmd) "target %d lun %d tag %d command %d"
 
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 16/24] scsi: introduce scsi_req_get_buf
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (14 preceding siblings ...)
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 15/24] scsi: introduce scsi_req_continue Paolo Bonzini
@ 2011-05-23 16:09 ` Paolo Bonzini
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 17/24] scsi: Implement 'get_sense' callback Paolo Bonzini
                   ` (7 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

... and remove some SCSIDevice variables or fields that now become unused.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/esp.c         |    2 +-
 hw/lsi53c895a.c  |    2 +-
 hw/scsi-bus.c    |    5 +++++
 hw/scsi.h        |    1 +
 hw/spapr_vscsi.c |    8 ++------
 hw/usb-msd.c     |    2 +-
 6 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index ced8945..381a8fb 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -419,7 +419,7 @@ static void esp_command_complete(SCSIRequest *req, int reason, uint32_t arg)
     } 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(req);
+        s->async_buf = scsi_req_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 3d5febb..a6c7c5b 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -569,7 +569,7 @@ static void lsi_do_dma(LSIState *s, int out)
     s->dnad += count;
     s->dbc -= count;
      if (s->current->dma_buf == NULL) {
-        s->current->dma_buf = dev->info->get_buf(s->current->req);
+        s->current->dma_buf = scsi_req_get_buf(s->current->req);
     }
     /* ??? Set SFBR to first data byte.  */
     if (out) {
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 4541877..c6b7ee5 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -151,6 +151,11 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
     return d->info->alloc_req(d, tag, lun);
 }
 
+uint8_t *scsi_req_get_buf(SCSIRequest *req)
+{
+    return req->dev->info->get_buf(req);
+}
+
 int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
 {
     assert(!req->enqueued);
diff --git a/hw/scsi.h b/hw/scsi.h
index 9038812..a4bdede 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -154,6 +154,7 @@ void scsi_req_print(SCSIRequest *req);
 void scsi_req_continue(SCSIRequest *req);
 void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
+uint8_t *scsi_req_get_buf(SCSIRequest *req);
 void scsi_req_abort(SCSIRequest *req, int status);
 void scsi_req_cancel(SCSIRequest *req);
 void scsi_device_purge_requests(SCSIDevice *sdev);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 3f65a87..29ac8ef 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -74,7 +74,6 @@ typedef struct vscsi_req {
     union viosrp_iu         iu;
 
     /* SCSI request tracking */
-    SCSIDevice              *sdev;
     SCSIRequest             *sreq;
     uint32_t                qtag; /* qemu tag != srp tag */
     int                     lun;
@@ -476,7 +475,6 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
 {
     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
     vscsi_req *req = vscsi_find_req(s, sreq);
-    SCSIDevice *sdev;
     uint8_t *buf;
     int32_t res_in = 0, res_out = 0;
     int len, rc = 0;
@@ -487,7 +485,6 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
         fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
         return;
     }
-    sdev = req->sdev;
 
     if (req->sensing) {
         if (reason == SCSI_REASON_DONE) {
@@ -495,7 +492,7 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
             vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
             vscsi_put_req(s, req);
         } else {
-            uint8_t *buf = sdev->info->get_buf(sreq);
+            uint8_t *buf = scsi_req_get_buf(sreq);
 
             len = MIN(arg, SCSI_SENSE_BUF_SIZE);
             dprintf("VSCSI: Sense data, %d bytes:\n", len);
@@ -539,7 +536,7 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
      * to write for writes (ie, how much is to be DMA'd)
      */
     if (arg) {
-        buf = sdev->info->get_buf(sreq);
+        buf = scsi_req_get_buf(sreq);
         rc = vscsi_srp_transfer_data(s, req, req->writing, buf, arg);
     }
     if (rc < 0) {
@@ -646,7 +643,6 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
         } return 1;
     }
 
-    req->sdev = sdev;
     req->lun = lun;
     req->sreq = scsi_req_new(sdev, req->qtag, lun);
     n = scsi_req_enqueue(req->sreq, srp->cmd.cdb);
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 5f84256..27a37fa 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -247,7 +247,7 @@ static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
     }
     assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == SCSI_XFER_TO_DEV));
     s->scsi_len = arg;
-    s->scsi_buf = s->scsi_dev->info->get_buf(req);
+    s->scsi_buf = scsi_req_get_buf(req);
     if (p) {
         usb_msd_copy_data(s);
         if (s->usb_len == 0) {
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 17/24] scsi: Implement 'get_sense' callback
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (15 preceding siblings ...)
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 16/24] scsi: introduce scsi_req_get_buf Paolo Bonzini
@ 2011-05-23 16:09 ` Paolo Bonzini
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 18/24] scsi-disk: add data direction checking Paolo Bonzini
                   ` (6 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch, Hannes Reinecke

From: Hannes Reinecke <hare@suse.de>

The get_sense callback copies existing sense information into
the provided buffer. This is required if sense information
should be transferred together with the command response.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-bus.c     |    9 +++++++++
 hw/scsi-disk.c    |    9 +++++++++
 hw/scsi-generic.c |   18 ++++++++++++++++++
 hw/scsi.h         |    2 ++
 hw/spapr_vscsi.c  |   10 +++++++++-
 5 files changed, 47 insertions(+), 1 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index c6b7ee5..d4fd61d 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -156,6 +156,15 @@ uint8_t *scsi_req_get_buf(SCSIRequest *req)
     return req->dev->info->get_buf(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 {
+        return 0;
+    }
+}
+
 int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
 {
     assert(!req->enqueued);
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 083097e..b9c8fff 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -337,6 +337,14 @@ 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);
@@ -1224,6 +1232,7 @@ 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_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),
         DEFINE_PROP_STRING("ver",  SCSIDiskState, version),
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 4cf8e3b..a131431 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -79,6 +79,23 @@ static void scsi_clear_sense(SCSIGenericState *s)
     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)
 {
     SCSIRequest *req;
@@ -532,6 +549,7 @@ 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 a4bdede..53efa8e 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -80,6 +80,7 @@ struct SCSIDeviceInfo {
     int (*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 {
@@ -155,6 +156,7 @@ void scsi_req_continue(SCSIRequest *req);
 void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
 uint8_t *scsi_req_get_buf(SCSIRequest *req);
+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);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 29ac8ef..f0e08e3 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -450,6 +450,15 @@ static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
     uint8_t *cdb = req->iu.srp.cmd.cdb;
     int n;
 
+    n = scsi_req_get_sense(req->sreq, req->sense, sizeof(req->sense));
+    if (n) {
+        req->senselen = n;
+        vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
+        vscsi_put_req(s, req);
+        return;
+    }
+
+    dprintf("VSCSI: Got CHECK_CONDITION, requesting sense...\n");
     cdb[0] = 3;
     cdb[1] = 0;
     cdb[2] = 0;
@@ -522,7 +531,6 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
             }
             vscsi_send_rsp(s, req, 0, res_in, res_out);
         } else if (arg == CHECK_CONDITION) {
-            dprintf("VSCSI: Got CHECK_CONDITION, requesting sense...\n");
             vscsi_send_request_sense(s, req);
             return;
         } else {
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 18/24] scsi-disk: add data direction checking
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (16 preceding siblings ...)
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 17/24] scsi: Implement 'get_sense' callback Paolo Bonzini
@ 2011-05-23 16:09 ` Paolo Bonzini
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 19/24] scsi: make write_data return void Paolo Bonzini
                   ` (5 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch, Hannes Reinecke

From: Hannes Reinecke <hare@suse.de>

scsi_req_parse() already provides for a data direction setting,
so we should be using it to check for correct direction.
And we should return the sense code 'INVALID FIELD IN CDB'
in these cases.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-disk.c |   35 ++++++++++++++++++++++++-----------
 1 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index b9c8fff..7d122c4 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -51,8 +51,6 @@ typedef struct SCSIDiskState SCSIDiskState;
 
 typedef struct SCSIDiskReq {
     SCSIRequest req;
-    /* ??? We should probably keep track of whether the data transfer is
-       a read or a write.  Currently we rely on the host getting it right.  */
     /* Both sector and sector_count are in terms of qemu 512 byte blocks.  */
     uint64_t sector;
     uint32_t sector_count;
@@ -177,6 +175,12 @@ static void scsi_read_data(SCSIRequest *req)
     /* No data transfer may already be in progress */
     assert(r->req.aiocb == NULL);
 
+    if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
+        DPRINTF("Data transfer direction invalid\n");
+        scsi_read_complete(r, -EINVAL);
+        return;
+    }
+
     n = r->sector_count;
     if (n > SCSI_DMA_BUF_SIZE / 512)
         n = SCSI_DMA_BUF_SIZE / 512;
@@ -213,16 +217,22 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
         if (type == SCSI_REQ_STATUS_RETRY_READ) {
             scsi_req_data(&r->req, 0);
         }
-        if (error == ENOMEM) {
+        switch (error) {
+        case ENOMEM:
             scsi_command_complete(r, CHECK_CONDITION,
                                   SENSE_CODE(TARGET_FAILURE));
-        } else {
+            break;
+        case EINVAL:
+            scsi_command_complete(r, CHECK_CONDITION,
+                                  SENSE_CODE(INVALID_FIELD));
+            break;
+        default:
             scsi_command_complete(r, CHECK_CONDITION,
                                   SENSE_CODE(IO_ERROR));
+            break;
         }
         bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read);
     }
-
     return 1;
 }
 
@@ -265,6 +275,12 @@ static int scsi_write_data(SCSIRequest *req)
     /* No data transfer may already be in progress */
     assert(r->req.aiocb == NULL);
 
+    if (r->req.cmd.mode != SCSI_XFER_TO_DEV) {
+        DPRINTF("Data transfer direction invalid\n");
+        scsi_write_complete(r, -EINVAL);
+        return 0;
+    }
+
     n = r->iov.iov_len / 512;
     if (n) {
         qemu_iovec_init_external(&r->qiov, &r->iov, 1);
@@ -984,14 +1000,12 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
     SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
     int32_t len;
-    int is_write;
     uint8_t command;
     uint8_t *outbuf;
     int rc;
 
     command = buf[0];
     outbuf = (uint8_t *)r->iov.iov_base;
-    is_write = 0;
     DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, tag, buf[0]);
 
     if (scsi_req_parse(&r->req, buf) != 0) {
@@ -1071,7 +1085,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
             goto illegal_lba;
         r->sector = r->req.cmd.lba * s->cluster_size;
         r->sector_count = len * s->cluster_size;
-        is_write = 1;
         break;
     case MODE_SELECT:
         DPRINTF("Mode Select(6) (len %lu)\n", (long)r->req.cmd.xfer);
@@ -1137,13 +1150,13 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
         scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
     }
     len = r->sector_count * 512 + r->iov.iov_len;
-    if (is_write) {
-        len = -len;
+    if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
+        return -len;
     } else {
         if (!r->sector_count)
             r->sector_count = -1;
+        return len;
     }
-    return len;
 }
 
 static void scsi_disk_reset(DeviceState *dev)
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 19/24] scsi: make write_data return void
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (17 preceding siblings ...)
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 18/24] scsi-disk: add data direction checking Paolo Bonzini
@ 2011-05-23 16:09 ` Paolo Bonzini
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 20/24] scsi-generic: Handle queue full Paolo Bonzini
                   ` (4 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

The return value is unused anyway.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-disk.c    |    6 ++----
 hw/scsi-generic.c |    7 ++-----
 hw/scsi.h         |    2 +-
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 7d122c4..77d0151 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -266,7 +266,7 @@ static void scsi_write_complete(void * opaque, int ret)
     }
 }
 
-static int scsi_write_data(SCSIRequest *req)
+static void scsi_write_data(SCSIRequest *req)
 {
     SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
@@ -278,7 +278,7 @@ static int scsi_write_data(SCSIRequest *req)
     if (r->req.cmd.mode != SCSI_XFER_TO_DEV) {
         DPRINTF("Data transfer direction invalid\n");
         scsi_write_complete(r, -EINVAL);
-        return 0;
+        return;
     }
 
     n = r->iov.iov_len / 512;
@@ -293,8 +293,6 @@ static int scsi_write_data(SCSIRequest *req)
         /* Invoke completion routine to fetch data from host.  */
         scsi_write_complete(r, 0);
     }
-
-    return 0;
 }
 
 static void scsi_dma_restart_bh(void *opaque)
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index a131431..53114e5 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -274,7 +274,7 @@ 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(SCSIRequest *req)
+static void scsi_write_data(SCSIRequest *req)
 {
     SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
     SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
@@ -284,16 +284,13 @@ static int scsi_write_data(SCSIRequest *req)
     if (r->len == 0) {
         r->len = r->buflen;
         scsi_req_data(&r->req, r->len);
-        return 0;
+        return;
     }
 
     ret = execute_command(s->bs, r, SG_DXFER_TO_DEV, scsi_write_complete);
     if (ret < 0) {
         scsi_command_complete(r, ret);
-        return 1;
     }
-
-    return 0;
 }
 
 /* Return a pointer to the data buffer.  */
diff --git a/hw/scsi.h b/hw/scsi.h
index 53efa8e..fa9c9fa 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -77,7 +77,7 @@ struct SCSIDeviceInfo {
     void (*free_req)(SCSIRequest *req);
     int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
     void (*read_data)(SCSIRequest *req);
-    int (*write_data)(SCSIRequest *req);
+    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);
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 20/24] scsi-generic: Handle queue full
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (18 preceding siblings ...)
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 19/24] scsi: make write_data return void Paolo Bonzini
@ 2011-05-23 16:09 ` Paolo Bonzini
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 21/24] esp: rename sense to status Paolo Bonzini
                   ` (3 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch, Hannes Reinecke

The sg driver currently has a hardcoded limit of commands it
can handle simultaneously. When this limit is reached the
driver will return -EDOM. So we need to capture this to
enable proper return values here.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-generic.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 53114e5..30845c5 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -123,6 +123,9 @@ static void scsi_command_complete(void *opaque, int ret)
 
     if (ret != 0) {
         switch (ret) {
+        case -EDOM:
+            r->req.status = TASK_SET_FULL;
+            break;
         case -EINVAL:
             r->req.status = CHECK_CONDITION;
             scsi_set_sense(s, SENSE_CODE(INVALID_FIELD));
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 21/24] esp: rename sense to status
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (19 preceding siblings ...)
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 20/24] scsi-generic: Handle queue full Paolo Bonzini
@ 2011-05-23 16:09 ` Paolo Bonzini
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 22/24] scsi: split command_complete callback in two Paolo Bonzini
                   ` (2 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

This mirrors the LSI patch that was recently committed.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 hw/esp.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 381a8fb..0b80529 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -61,7 +61,7 @@ struct ESPState {
     int32_t ti_size;
     uint32_t ti_rptr, ti_wptr;
     uint8_t ti_buf[TI_BUFSZ];
-    uint32_t sense;
+    uint32_t status;
     uint32_t dma;
     SCSIBus bus;
     SCSIDevice *current_dev;
@@ -318,8 +318,8 @@ static void handle_satn_stop(ESPState *s)
 
 static void write_response(ESPState *s)
 {
-    DPRINTF("Transfer status (sense=%d)\n", s->sense);
-    s->ti_buf[0] = s->sense;
+    DPRINTF("Transfer status (status=%d)\n", s->status);
+    s->ti_buf[0] = s->status;
     s->ti_buf[1] = 0;
     if (s->dma) {
         s->dma_memory_write(s->dma_opaque, s->ti_buf, 2);
@@ -408,7 +408,7 @@ static void esp_command_complete(SCSIRequest *req, int reason, uint32_t arg)
         s->async_len = 0;
         if (arg)
             DPRINTF("Command failed\n");
-        s->sense = arg;
+        s->status = arg;
         s->rregs[ESP_RSTAT] = STAT_ST;
         esp_dma_done(s);
 	if (s->current_req) {
@@ -688,7 +688,7 @@ static const VMStateDescription vmstate_esp = {
         VMSTATE_UINT32(ti_rptr, ESPState),
         VMSTATE_UINT32(ti_wptr, ESPState),
         VMSTATE_BUFFER(ti_buf, ESPState),
-        VMSTATE_UINT32(sense, ESPState),
+        VMSTATE_UINT32(status, ESPState),
         VMSTATE_UINT32(dma, ESPState),
         VMSTATE_BUFFER(cmdbuf, ESPState),
         VMSTATE_UINT32(cmdlen, ESPState),
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 22/24] scsi: split command_complete callback in two
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (20 preceding siblings ...)
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 21/24] esp: rename sense to status Paolo Bonzini
@ 2011-05-23 16:09 ` Paolo Bonzini
  2011-05-23 19:38   ` Blue Swirl
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 23/24] scsi: rename arguments to the new callbacks Paolo Bonzini
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 24/24] scsi: ignore LUN field in the CDB Paolo Bonzini
  23 siblings, 1 reply; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 hw/esp.c         |   60 +++++++++++++++++---------------
 hw/lsi53c895a.c  |   48 +++++++++++++++-----------
 hw/scsi-bus.c    |    4 +-
 hw/scsi.h        |    9 +----
 hw/spapr_vscsi.c |  101 ++++++++++++++++++++++++++++++------------------------
 hw/usb-msd.c     |   69 +++++++++++++++++++++----------------
 6 files changed, 159 insertions(+), 132 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 0b80529..32ed327 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -395,38 +395,41 @@ static void esp_do_dma(ESPState *s)
     esp_dma_done(s);
 }
 
-static void esp_command_complete(SCSIRequest *req, int reason, uint32_t arg)
+static void esp_command_complete(SCSIRequest *req, uint32_t arg)
 {
     ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
 
-    if (reason == SCSI_REASON_DONE) {
-        DPRINTF("SCSI Command complete\n");
-        if (s->ti_size != 0)
-            DPRINTF("SCSI command completed unexpectedly\n");
-        s->ti_size = 0;
-        s->dma_left = 0;
-        s->async_len = 0;
-        if (arg)
-            DPRINTF("Command failed\n");
-        s->status = arg;
-        s->rregs[ESP_RSTAT] = STAT_ST;
+    DPRINTF("SCSI Command complete\n");
+    if (s->ti_size != 0)
+        DPRINTF("SCSI command completed unexpectedly\n");
+    s->ti_size = 0;
+    s->dma_left = 0;
+    s->async_len = 0;
+    if (arg)
+        DPRINTF("Command failed\n");
+    s->status = arg;
+    s->rregs[ESP_RSTAT] = STAT_ST;
+    esp_dma_done(s);
+    if (s->current_req) {
+        scsi_req_unref(s->current_req);
+        s->current_req = NULL;
+        s->current_dev = NULL;
+    }
+}
+
+static void esp_transfer_data(SCSIRequest *req, uint32_t arg)
+{
+    ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
+
+    DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
+    s->async_len = arg;
+    s->async_buf = scsi_req_get_buf(req);
+    if (s->dma_left) {
+        esp_do_dma(s);
+    } else if (s->dma_counter != 0 && s->ti_size <= 0) {
+        /* If this was the last part of a DMA transfer then the
+           completion interrupt is deferred to here.  */
         esp_dma_done(s);
-	if (s->current_req) {
-            scsi_req_unref(s->current_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 = scsi_req_get_buf(req);
-        if (s->dma_left) {
-            esp_do_dma(s);
-        } else if (s->dma_counter != 0 && s->ti_size <= 0) {
-            /* If this was the last part of a DMA transfer then the
-               completion interrupt is deferred to here.  */
-            esp_dma_done(s);
-        }
     }
 }
 
@@ -725,6 +728,7 @@ void esp_init(target_phys_addr_t espaddr, int it_shift,
 }
 
 static struct SCSIBusOps esp_scsi_ops = {
+    .transfer_data = esp_transfer_data,
     .complete = esp_command_complete,
     .cancel = esp_request_cancelled
 };
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index a6c7c5b..62fe191 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -711,40 +711,47 @@ static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
         return 1;
     }
 }
- /* Callback to indicate that the SCSI layer has completed a transfer.  */
-static void lsi_command_complete(SCSIRequest *req, int reason, uint32_t arg)
+
+ /* Callback to indicate that the SCSI layer has completed a command.  */
+static void lsi_command_complete(SCSIRequest *req, uint32_t arg)
 {
     LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
     int out;
 
     out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
-    if (reason == SCSI_REASON_DONE) {
-        DPRINTF("Command complete status=%d\n", (int)arg);
-        s->status = arg;
-        s->command_complete = 2;
-        if (s->waiting && s->dbc != 0) {
-            /* Raise phase mismatch for short transfers.  */
-            lsi_bad_phase(s, out, PHASE_ST);
-        } else {
-            lsi_set_phase(s, PHASE_ST);
-        }
+    DPRINTF("Command complete status=%d\n", (int)arg);
+    s->status = arg;
+    s->command_complete = 2;
+    if (s->waiting && s->dbc != 0) {
+        /* Raise phase mismatch for short transfers.  */
+        lsi_bad_phase(s, out, PHASE_ST);
+    } else {
+        lsi_set_phase(s, PHASE_ST);
+    }
 
-        if (req == s->current->req) {
-            scsi_req_unref(s->current->req);
-            qemu_free(s->current);
-            s->current = NULL;
-	}
-        lsi_resume_script(s);
-        return;
+    if (req == s->current->req) {
+        scsi_req_unref(s->current->req);
+        qemu_free(s->current);
+        s->current = NULL;
     }
+    lsi_resume_script(s);
+}
+
+ /* Callback to indicate that the SCSI layer has completed a transfer.  */
+static void lsi_transfer_data(SCSIRequest *req, uint32_t arg)
+{
+    LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
+    int out;
 
     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, req->tag, arg)) {
             return;
-        }
+	}
     }
 
+    out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
+
     /* host adapter (re)connected */
     DPRINTF("Data ready tag=0x%x len=%d\n", req->tag, arg);
     s->current->dma_len = arg;
@@ -2239,6 +2246,7 @@ static int lsi_scsi_uninit(PCIDevice *d)
 }
 
 static struct SCSIBusOps lsi_scsi_ops = {
+    .transfer_data = lsi_transfer_data,
     .complete = lsi_command_complete,
     .cancel = lsi_request_cancelled
 };
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index d4fd61d..086a998 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -628,7 +628,7 @@ void scsi_req_continue(SCSIRequest *req)
 void scsi_req_data(SCSIRequest *req, int len)
 {
     trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
-    req->bus->ops->complete(req, SCSI_REASON_DATA, len);
+    req->bus->ops->transfer_data(req, len);
 }
 
 void scsi_req_print(SCSIRequest *req)
@@ -664,7 +664,7 @@ void scsi_req_complete(SCSIRequest *req)
     assert(req->status != -1);
     scsi_req_ref(req);
     scsi_req_dequeue(req);
-    req->bus->ops->complete(req, SCSI_REASON_DONE, req->status);
+    req->bus->ops->complete(req, req->status);
     scsi_req_unref(req);
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index fa9c9fa..70f9e89 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -9,12 +9,6 @@
 
 #define SCSI_CMD_BUF_SIZE     16
 
-/* scsi-disk.c */
-enum scsi_reason {
-    SCSI_REASON_DONE, /* Command complete.  */
-    SCSI_REASON_DATA  /* Transfer complete, more data required.  */
-};
-
 typedef struct SCSIBus SCSIBus;
 typedef struct SCSIBusOps SCSIBusOps;
 typedef struct SCSIDevice SCSIDevice;
@@ -84,7 +78,8 @@ struct SCSIDeviceInfo {
 };
 
 struct SCSIBusOps {
-    void (*complete)(SCSIRequest *req, int reason, uint32_t arg);
+    void (*transfer_data)(SCSIRequest *req, uint32_t arg);
+    void (*complete)(SCSIRequest *req, uint32_t arg);
     void (*cancel)(SCSIRequest *req);
 };
 
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index f0e08e3..d84e10d 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -480,63 +480,34 @@ static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
 }
 
 /* Callback to indicate that the SCSI layer has completed a transfer.  */
-static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
+static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t arg)
 {
     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
     vscsi_req *req = vscsi_find_req(s, sreq);
     uint8_t *buf;
-    int32_t res_in = 0, res_out = 0;
     int len, rc = 0;
 
-    dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x arg=0x%x, req=%p\n",
-            reason, sreq->tag, arg, req);
+    dprintf("VSCSI: SCSI xfer complete tag=0x%x arg=0x%x, req=%p\n",
+            sreq->tag, arg, req);
     if (req == NULL) {
         fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
         return;
     }
 
     if (req->sensing) {
-        if (reason == SCSI_REASON_DONE) {
-            dprintf("VSCSI: Sense done !\n");
-            vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
-            vscsi_put_req(s, req);
-        } else {
-            uint8_t *buf = scsi_req_get_buf(sreq);
-
-            len = MIN(arg, SCSI_SENSE_BUF_SIZE);
-            dprintf("VSCSI: Sense data, %d bytes:\n", len);
-            dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
-                    buf[0], buf[1], buf[2], buf[3],
-                    buf[4], buf[5], buf[6], buf[7]);
-            dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
-                    buf[8], buf[9], buf[10], buf[11],
-                    buf[12], buf[13], buf[14], buf[15]);
-            memcpy(req->sense, buf, len);
-            req->senselen = len;
-            scsi_req_continue(req->sreq);
-        }
-        return;
-    }
-
-    if (reason == SCSI_REASON_DONE) {
-        dprintf("VSCSI: Command complete err=%d\n", arg);
-        if (arg == 0) {
-            /* We handle overflows, not underflows for normal commands,
-             * but hopefully nobody cares
-             */
-            if (req->writing) {
-                res_out = req->data_len;
-            } else {
-                res_in = req->data_len;
-            }
-            vscsi_send_rsp(s, req, 0, res_in, res_out);
-        } else if (arg == CHECK_CONDITION) {
-            vscsi_send_request_sense(s, req);
-            return;
-        } else {
-            vscsi_send_rsp(s, req, arg, 0, 0);
-        }
-        vscsi_put_req(s, req);
+        uint8_t *buf = scsi_req_get_buf(sreq);
+
+        len = MIN(arg, SCSI_SENSE_BUF_SIZE);
+        dprintf("VSCSI: Sense data, %d bytes:\n", len);
+        dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
+                buf[0], buf[1], buf[2], buf[3],
+                buf[4], buf[5], buf[6], buf[7]);
+        dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
+                buf[8], buf[9], buf[10], buf[11],
+                buf[12], buf[13], buf[14], buf[15]);
+        memcpy(req->sense, buf, len);
+        req->senselen = len;
+        scsi_req_continue(req->sreq);
         return;
     }
 
@@ -559,6 +530,45 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
     scsi_req_continue(sreq);
 }
 
+/* Callback to indicate that the SCSI layer has completed a transfer.  */
+static void vscsi_command_complete(SCSIRequest *sreq, uint32_t arg)
+{
+    VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
+    vscsi_req *req = vscsi_find_req(s, sreq);
+    int32_t res_in = 0, res_out = 0;
+
+    dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x arg=0x%x, req=%p\n",
+            reason, sreq->tag, arg, req);
+    if (req == NULL) {
+        fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
+        return;
+    }
+
+    if (!req->sensing && arg == CHECK_CONDITION) {
+        vscsi_send_request_sense(s, req);
+        return;
+    }
+
+    if (req->sensing) {
+        dprintf("VSCSI: Sense done !\n");
+        arg = CHECK_CONDITION;
+    } else {
+        dprintf("VSCSI: Command complete err=%d\n", arg);
+        if (arg == 0) {
+            /* We handle overflows, not underflows for normal commands,
+             * but hopefully nobody cares
+             */
+            if (req->writing) {
+                res_out = req->data_len;
+            } else {
+                res_in = req->data_len;
+            }
+        }
+    }
+    vscsi_send_rsp(s, req, 0, res_in, res_out);
+    vscsi_put_req(s, req);
+}
+
 static void vscsi_request_cancelled(SCSIRequest *sreq)
 {
     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
@@ -916,6 +926,7 @@ static int vscsi_do_crq(struct VIOsPAPRDevice *dev, uint8_t *crq_data)
 }
 
 static struct SCSIBusOps vscsi_scsi_ops = {
+    .transfer_data = vscsi_transfer_data,
     .complete = vscsi_command_complete,
     .cancel = vscsi_request_cancelled
 };
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 27a37fa..c808815 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -208,7 +208,7 @@ static void usb_msd_send_status(MSDState *s, USBPacket *p)
     memcpy(p->data, &csw, len);
 }
 
-static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
+static void usb_msd_transfer_data(SCSIRequest *req, uint32_t arg)
 {
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
     USBPacket *p = s->packet;
@@ -216,35 +216,7 @@ static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
     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);
-        s->residue = s->data_len;
-        s->result = arg != 0;
-        if (s->packet) {
-            if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
-                /* A deferred packet with no write data remaining must be
-                   the status read packet.  */
-                usb_msd_send_status(s, p);
-                s->mode = USB_MSDM_CBW;
-            } else {
-                if (s->data_len) {
-                    s->data_len -= s->usb_len;
-                    if (s->mode == USB_MSDM_DATAIN)
-                        memset(s->usb_buf, 0, s->usb_len);
-                    s->usb_len = 0;
-                }
-                if (s->data_len == 0)
-                    s->mode = USB_MSDM_CSW;
-            }
-            s->packet = NULL;
-            usb_packet_complete(&s->dev, p);
-        } else if (s->data_len == 0) {
-            s->mode = USB_MSDM_CSW;
-        }
-        scsi_req_unref(req);
-        s->req = NULL;
-        return;
-    }
+
     assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == SCSI_XFER_TO_DEV));
     s->scsi_len = arg;
     s->scsi_buf = scsi_req_get_buf(req);
@@ -261,6 +233,42 @@ static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
     }
 }
 
+static void usb_msd_command_complete(SCSIRequest *req, uint32_t arg)
+{
+    MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
+    USBPacket *p = s->packet;
+
+    if (req->tag != s->tag) {
+        fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
+    }
+    DPRINTF("Command complete %d\n", arg);
+    s->residue = s->data_len;
+    s->result = arg != 0;
+    if (s->packet) {
+        if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
+            /* A deferred packet with no write data remaining must be
+               the status read packet.  */
+            usb_msd_send_status(s, p);
+            s->mode = USB_MSDM_CBW;
+        } else {
+            if (s->data_len) {
+                s->data_len -= s->usb_len;
+                if (s->mode == USB_MSDM_DATAIN)
+                    memset(s->usb_buf, 0, s->usb_len);
+                s->usb_len = 0;
+            }
+            if (s->data_len == 0)
+                s->mode = USB_MSDM_CSW;
+        }
+        s->packet = NULL;
+        usb_packet_complete(&s->dev, p);
+    } else if (s->data_len == 0) {
+        s->mode = USB_MSDM_CSW;
+    }
+    scsi_req_unref(req);
+    s->req = NULL;
+}
+
 static void usb_msd_request_cancelled(SCSIRequest *req)
 {
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
@@ -494,6 +502,7 @@ static void usb_msd_password_cb(void *opaque, int err)
 }
 
 static struct SCSIBusOps usb_msd_scsi_ops = {
+    .transfer_data = usb_msd_transfer_data,
     .complete = usb_msd_command_complete,
     .cancel = usb_msd_request_cancelled
 };
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 23/24] scsi: rename arguments to the new callbacks
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (21 preceding siblings ...)
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 22/24] scsi: split command_complete callback in two Paolo Bonzini
@ 2011-05-23 16:09 ` Paolo Bonzini
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 24/24] scsi: ignore LUN field in the CDB Paolo Bonzini
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 hw/esp.c         |   10 +++++-----
 hw/lsi53c895a.c  |   20 ++++++++++----------
 hw/spapr_vscsi.c |   29 +++++++++++++----------------
 hw/usb-msd.c     |   10 +++++-----
 4 files changed, 33 insertions(+), 36 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 32ed327..a914d19 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -395,7 +395,7 @@ static void esp_do_dma(ESPState *s)
     esp_dma_done(s);
 }
 
-static void esp_command_complete(SCSIRequest *req, uint32_t arg)
+static void esp_command_complete(SCSIRequest *req, uint32_t status)
 {
     ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
 
@@ -405,9 +405,9 @@ static void esp_command_complete(SCSIRequest *req, uint32_t arg)
     s->ti_size = 0;
     s->dma_left = 0;
     s->async_len = 0;
-    if (arg)
+    if (status)
         DPRINTF("Command failed\n");
-    s->status = arg;
+    s->status = status;
     s->rregs[ESP_RSTAT] = STAT_ST;
     esp_dma_done(s);
     if (s->current_req) {
@@ -417,12 +417,12 @@ static void esp_command_complete(SCSIRequest *req, uint32_t arg)
     }
 }
 
-static void esp_transfer_data(SCSIRequest *req, uint32_t arg)
+static void esp_transfer_data(SCSIRequest *req, uint32_t len)
 {
     ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
 
     DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
-    s->async_len = arg;
+    s->async_len = len;
     s->async_buf = scsi_req_get_buf(req);
     if (s->dma_left) {
         esp_do_dma(s);
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 62fe191..8ad7e6c 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -680,7 +680,7 @@ static void lsi_request_cancelled(SCSIRequest *req)
 
 /* Record that data is available for a queued command.  Returns zero if
    the device was reselected, nonzero if the IO is deferred.  */
-static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
+static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t len)
 {
     lsi_request *p;
 
@@ -693,7 +693,7 @@ static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
     if (p->pending) {
         BADF("Multiple IO pending for tag %d\n", tag);
     }
-    p->pending = arg;
+    p->pending = len;
     /* Reselect if waiting for it, or if reselection triggers an IRQ
        and the bus is free.
        Since no interrupt stacking is implemented in the emulation, it
@@ -707,20 +707,20 @@ static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
         return 0;
     } else {
         DPRINTF("Queueing IO tag=0x%x\n", tag);
-        p->pending = arg;
+        p->pending = len;
         return 1;
     }
 }
 
  /* Callback to indicate that the SCSI layer has completed a command.  */
-static void lsi_command_complete(SCSIRequest *req, uint32_t arg)
+static void lsi_command_complete(SCSIRequest *req, uint32_t status)
 {
     LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
     int out;
 
     out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
-    DPRINTF("Command complete status=%d\n", (int)arg);
-    s->status = arg;
+    DPRINTF("Command complete status=%d\n", (int)status);
+    s->status = status;
     s->command_complete = 2;
     if (s->waiting && s->dbc != 0) {
         /* Raise phase mismatch for short transfers.  */
@@ -738,14 +738,14 @@ static void lsi_command_complete(SCSIRequest *req, uint32_t arg)
 }
 
  /* Callback to indicate that the SCSI layer has completed a transfer.  */
-static void lsi_transfer_data(SCSIRequest *req, uint32_t arg)
+static void lsi_transfer_data(SCSIRequest *req, uint32_t len)
 {
     LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
     int out;
 
     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, req->tag, arg)) {
+        if (lsi_queue_tag(s, req->tag, len)) {
             return;
 	}
     }
@@ -753,8 +753,8 @@ static void lsi_transfer_data(SCSIRequest *req, uint32_t arg)
     out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
 
     /* host adapter (re)connected */
-    DPRINTF("Data ready tag=0x%x len=%d\n", req->tag, arg);
-    s->current->dma_len = arg;
+    DPRINTF("Data ready tag=0x%x len=%d\n", req->tag, len);
+    s->current->dma_len = len;
     s->command_complete = 1;
     if (!s->waiting)
         return;
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index d84e10d..cdccf7a 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -480,15 +480,15 @@ static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
 }
 
 /* Callback to indicate that the SCSI layer has completed a transfer.  */
-static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t arg)
+static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t len)
 {
     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
     vscsi_req *req = vscsi_find_req(s, sreq);
     uint8_t *buf;
-    int len, rc = 0;
+    int rc = 0;
 
-    dprintf("VSCSI: SCSI xfer complete tag=0x%x arg=0x%x, req=%p\n",
-            sreq->tag, arg, req);
+    dprintf("VSCSI: SCSI xfer complete tag=0x%x len=0x%x, req=%p\n",
+            sreq->tag, len, req);
     if (req == NULL) {
         fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
         return;
@@ -497,7 +497,7 @@ static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t arg)
     if (req->sensing) {
         uint8_t *buf = scsi_req_get_buf(sreq);
 
-        len = MIN(arg, SCSI_SENSE_BUF_SIZE);
+        len = MIN(len, SCSI_SENSE_BUF_SIZE);
         dprintf("VSCSI: Sense data, %d bytes:\n", len);
         dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
                 buf[0], buf[1], buf[2], buf[3],
@@ -511,12 +511,9 @@ static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t arg)
         return;
     }
 
-    /* "arg" is how much we have read for reads and how much we want
-     * to write for writes (ie, how much is to be DMA'd)
-     */
-    if (arg) {
+    if (len) {
         buf = scsi_req_get_buf(sreq);
-        rc = vscsi_srp_transfer_data(s, req, req->writing, buf, arg);
+        rc = vscsi_srp_transfer_data(s, req, req->writing, buf, len);
     }
     if (rc < 0) {
         fprintf(stderr, "VSCSI: RDMA error rc=%d!\n", rc);
@@ -531,30 +528,30 @@ static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t arg)
 }
 
 /* Callback to indicate that the SCSI layer has completed a transfer.  */
-static void vscsi_command_complete(SCSIRequest *sreq, uint32_t arg)
+static void vscsi_command_complete(SCSIRequest *sreq, uint32_t status)
 {
     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
     vscsi_req *req = vscsi_find_req(s, sreq);
     int32_t res_in = 0, res_out = 0;
 
-    dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x arg=0x%x, req=%p\n",
-            reason, sreq->tag, arg, req);
+    dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x status=0x%x, req=%p\n",
+            reason, sreq->tag, status, req);
     if (req == NULL) {
         fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
         return;
     }
 
-    if (!req->sensing && arg == CHECK_CONDITION) {
+    if (!req->sensing && status == CHECK_CONDITION) {
         vscsi_send_request_sense(s, req);
         return;
     }
 
     if (req->sensing) {
         dprintf("VSCSI: Sense done !\n");
-        arg = CHECK_CONDITION;
+        status = CHECK_CONDITION;
     } else {
-        dprintf("VSCSI: Command complete err=%d\n", arg);
-        if (arg == 0) {
+        dprintf("VSCSI: Command complete err=%d\n", status);
+        if (status == 0) {
             /* We handle overflows, not underflows for normal commands,
              * but hopefully nobody cares
              */
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index c808815..9fa8eff 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -208,7 +208,7 @@ static void usb_msd_send_status(MSDState *s, USBPacket *p)
     memcpy(p->data, &csw, len);
 }
 
-static void usb_msd_transfer_data(SCSIRequest *req, uint32_t arg)
+static void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
 {
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
     USBPacket *p = s->packet;
@@ -218,7 +218,7 @@ static void usb_msd_transfer_data(SCSIRequest *req, uint32_t arg)
     }
 
     assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == SCSI_XFER_TO_DEV));
-    s->scsi_len = arg;
+    s->scsi_len = len;
     s->scsi_buf = scsi_req_get_buf(req);
     if (p) {
         usb_msd_copy_data(s);
@@ -233,7 +233,7 @@ static void usb_msd_transfer_data(SCSIRequest *req, uint32_t arg)
     }
 }
 
-static void usb_msd_command_complete(SCSIRequest *req, uint32_t arg)
+static void usb_msd_command_complete(SCSIRequest *req, uint32_t status)
 {
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
     USBPacket *p = s->packet;
@@ -241,9 +241,9 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t arg)
     if (req->tag != s->tag) {
         fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
     }
-    DPRINTF("Command complete %d\n", arg);
+    DPRINTF("Command complete %d\n", status);
     s->residue = s->data_len;
-    s->result = arg != 0;
+    s->result = status != 0;
     if (s->packet) {
         if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
             /* A deferred packet with no write data remaining must be
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PATCH v4 24/24] scsi: ignore LUN field in the CDB
  2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
                   ` (22 preceding siblings ...)
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 23/24] scsi: rename arguments to the new callbacks Paolo Bonzini
@ 2011-05-23 16:09 ` Paolo Bonzini
  23 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-23 16:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: hch

The LUN field in the CDB is a historical relic.  Ignore it as reserved,
which is what modern SCSI specifications actually say.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 hw/scsi-disk.c    |    6 +++---
 hw/scsi-generic.c |    5 ++---
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 77d0151..b90bc97 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -515,7 +515,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
 
     memset(outbuf, 0, buflen);
 
-    if (req->lun || req->cmd.buf[1] >> 5) {
+    if (req->lun) {
         outbuf[0] = 0x7f;	/* LUN not supported */
         return buflen;
     }
@@ -1021,9 +1021,9 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
     }
 #endif
 
-    if (req->lun || buf[1] >> 5) {
+    if (req->lun) {
         /* Only LUN 0 supported.  */
-        DPRINTF("Unimplemented LUN %d\n", req->lun ? req->lun : buf[1] >> 5);
+        DPRINTF("Unimplemented LUN %d\n", req->lun);
         if (command != REQUEST_SENSE && command != INQUIRY) {
             scsi_command_complete(r, CHECK_CONDITION,
                                   SENSE_CODE(LUN_NOT_SUPPORTED));
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 30845c5..3e720dc 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -334,9 +334,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
     SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
     int ret;
 
-    if (cmd[0] != REQUEST_SENSE &&
-        (req->lun != s->lun || (cmd[1] >> 5) != s->lun)) {
-        DPRINTF("Unimplemented LUN %d\n", req->lun ? req->lun : cmd[1] >> 5);
+    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));
         r->req.status = CHECK_CONDITION;
         scsi_req_complete(&r->req);
-- 
1.7.4.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH v4 04/24] scsi: introduce SCSIBusOps
  2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 04/24] scsi: introduce SCSIBusOps Paolo Bonzini
@ 2011-05-23 19:35   ` Blue Swirl
  0 siblings, 0 replies; 28+ messages in thread
From: Blue Swirl @ 2011-05-23 19:35 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, hch

On Mon, May 23, 2011 at 7:08 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> There are more operations than a SCSI bus can handle, besides completing
> commands.  The current callback in fact is overloaded and can be called
> with two different meanings already.  Another example, which this series
> will introduce, is cleaning up after a request is cancelled.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> ---
>  hw/esp.c          |    6 +++++-
>  hw/lsi53c895a.c   |    6 +++++-
>  hw/scsi-bus.c     |   12 ++++++------
>  hw/scsi-generic.c |    2 +-
>  hw/scsi.h         |   13 +++++++------
>  hw/spapr_vscsi.c  |    6 +++++-
>  hw/usb-msd.c      |    6 +++++-
>  7 files changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/hw/esp.c b/hw/esp.c
> index fa9d2a2..d8bba7a 100644
> --- a/hw/esp.c
> +++ b/hw/esp.c
> @@ -714,6 +714,10 @@ void esp_init(target_phys_addr_t espaddr, int it_shift,
>     *dma_enable = qdev_get_gpio_in(dev, 1);
>  }
>
> +static struct SCSIBusOps esp_scsi_ops = {

It looks like structs like this could be made 'const'.

> +    .complete = esp_command_complete
> +};
> +
>  static int esp_init1(SysBusDevice *dev)
>  {
>     ESPState *s = FROM_SYSBUS(ESPState, dev);
> @@ -728,7 +732,7 @@ static int esp_init1(SysBusDevice *dev)
>
>     qdev_init_gpio_in(&dev->qdev, esp_gpio_demux, 2);
>
> -    scsi_bus_new(&s->bus, &dev->qdev, 0, ESP_MAX_DEVS, esp_command_complete);
> +    scsi_bus_new(&s->bus, &dev->qdev, 0, ESP_MAX_DEVS, &esp_scsi_ops);
>     return scsi_bus_legacy_handle_cmdline(&s->bus);
>  }
>
> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> index 2ce38a9..ccea6ad 100644
> --- a/hw/lsi53c895a.c
> +++ b/hw/lsi53c895a.c
> @@ -2205,6 +2205,10 @@ static int lsi_scsi_uninit(PCIDevice *d)
>     return 0;
>  }
>
> +static struct SCSIBusOps lsi_scsi_ops = {
> +    .complete = lsi_command_complete
> +};
> +
>  static int lsi_scsi_init(PCIDevice *dev)
>  {
>     LSIState *s = DO_UPCAST(LSIState, dev, dev);
> @@ -2241,7 +2245,7 @@ static int lsi_scsi_init(PCIDevice *dev)
>                            PCI_BASE_ADDRESS_SPACE_MEMORY, lsi_ram_mapfunc);
>     QTAILQ_INIT(&s->queue);
>
> -    scsi_bus_new(&s->bus, &dev->qdev, 1, LSI_MAX_DEVS, lsi_command_complete);
> +    scsi_bus_new(&s->bus, &dev->qdev, 1, LSI_MAX_DEVS, &lsi_scsi_ops);
>     if (!dev->qdev.hotplugged) {
>         return scsi_bus_legacy_handle_cmdline(&s->bus);
>     }
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index 191cbab..f21704f 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -21,13 +21,13 @@ static int next_scsi_bus;
>
>  /* Create a scsi bus, and attach devices to it.  */
>  void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
> -                  scsi_completionfn complete)
> +                  SCSIBusOps *ops)
>  {
>     qbus_create_inplace(&bus->qbus, &scsi_bus_info, host, NULL);
>     bus->busnr = next_scsi_bus++;
>     bus->tcq = tcq;
>     bus->ndev = ndev;
> -    bus->complete = complete;
> +    bus->ops = ops;
>     bus->qbus.allow_hotplug = 1;
>  }
>
> @@ -503,7 +503,7 @@ static const char *scsi_command_name(uint8_t cmd)
>  void scsi_req_data(SCSIRequest *req, int len)
>  {
>     trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
> -    req->bus->complete(req->bus, SCSI_REASON_DATA, req->tag, len);
> +    req->bus->ops->complete(req->bus, SCSI_REASON_DATA, req->tag, len);
>  }
>
>  void scsi_req_print(SCSIRequest *req)
> @@ -538,9 +538,9 @@ 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->bus->ops->complete(req->bus, SCSI_REASON_DONE,
> +                            req->tag,
> +                            req->status);
>  }
>
>  static char *scsibus_get_fw_dev_path(DeviceState *dev)
> diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
> index e4f1f30..f09458b 100644
> --- a/hw/scsi-generic.c
> +++ b/hw/scsi-generic.c
> @@ -335,7 +335,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
>         s->senselen = 7;
>         s->driver_status = SG_ERR_DRIVER_SENSE;
>         bus = scsi_bus_from_device(d);
> -        bus->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
> +        bus->ops->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
>         return 0;
>     }
>
> diff --git a/hw/scsi.h b/hw/scsi.h
> index 7c09f32..20cf397 100644
> --- a/hw/scsi.h
> +++ b/hw/scsi.h
> @@ -16,10 +16,9 @@ enum scsi_reason {
>  };
>
>  typedef struct SCSIBus SCSIBus;
> +typedef struct SCSIBusOps SCSIBusOps;
>  typedef struct SCSIDevice SCSIDevice;
>  typedef struct SCSIDeviceInfo SCSIDeviceInfo;
> -typedef void (*scsi_completionfn)(SCSIBus *bus, int reason, uint32_t tag,
> -                                  uint32_t arg);
>
>  enum SCSIXferMode {
>     SCSI_XFER_NONE,      /*  TEST_UNIT_READY, ...            */
> @@ -74,20 +73,22 @@ struct SCSIDeviceInfo {
>     uint8_t *(*get_buf)(SCSIDevice *s, uint32_t tag);
>  };
>
> -typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
> -              int unit);
> +struct SCSIBusOps {
> +    void (*complete)(SCSIBus *bus, int reason, uint32_t tag, uint32_t arg);
> +};
> +
>  struct SCSIBus {
>     BusState qbus;
>     int busnr;
>
>     int tcq, ndev;
> -    scsi_completionfn complete;
> +    SCSIBusOps *ops;
>
>     SCSIDevice *devs[MAX_SCSI_DEVS];
>  };
>
>  void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
> -                  scsi_completionfn complete);
> +                  SCSIBusOps *ops);
>  void scsi_qdev_register(SCSIDeviceInfo *info);
>
>  static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
> diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
> index 9928334..9f5c154 100644
> --- a/hw/spapr_vscsi.c
> +++ b/hw/spapr_vscsi.c
> @@ -907,6 +907,10 @@ static int vscsi_do_crq(struct VIOsPAPRDevice *dev, uint8_t *crq_data)
>     return 0;
>  }
>
> +static struct SCSIBusOps vscsi_scsi_ops = {
> +    .complete = vscsi_command_complete
> +};
> +
>  static int spapr_vscsi_init(VIOsPAPRDevice *dev)
>  {
>     VSCSIState *s = DO_UPCAST(VSCSIState, vdev, dev);
> @@ -923,7 +927,7 @@ static int spapr_vscsi_init(VIOsPAPRDevice *dev)
>     dev->crq.SendFunc = vscsi_do_crq;
>
>     scsi_bus_new(&s->bus, &dev->qdev, 1, VSCSI_REQ_LIMIT,
> -                 vscsi_command_complete);
> +                 &vscsi_scsi_ops);
>     if (!dev->qdev.hotplugged) {
>         scsi_bus_legacy_handle_cmdline(&s->bus);
>     }
> diff --git a/hw/usb-msd.c b/hw/usb-msd.c
> index bd1c3a4..7a07a12 100644
> --- a/hw/usb-msd.c
> +++ b/hw/usb-msd.c
> @@ -487,6 +487,10 @@ static void usb_msd_password_cb(void *opaque, int err)
>         qdev_unplug(&s->dev.qdev);
>  }
>
> +static struct SCSIBusOps usb_msd_scsi_ops = {
> +    .complete = usb_msd_command_complete
> +};
> +
>  static int usb_msd_initfn(USBDevice *dev)
>  {
>     MSDState *s = DO_UPCAST(MSDState, dev, dev);
> @@ -516,7 +520,7 @@ static int usb_msd_initfn(USBDevice *dev)
>     }
>
>     usb_desc_init(dev);
> -    scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, usb_msd_command_complete);
> +    scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, &usb_msd_scsi_ops);
>     s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable);
>     if (!s->scsi_dev) {
>         return -1;
> --
> 1.7.4.4
>
>
>
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH v4 22/24] scsi: split command_complete callback in two
  2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 22/24] scsi: split command_complete callback in two Paolo Bonzini
@ 2011-05-23 19:38   ` Blue Swirl
  2011-05-24  7:32     ` Paolo Bonzini
  0 siblings, 1 reply; 28+ messages in thread
From: Blue Swirl @ 2011-05-23 19:38 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, hch

On Mon, May 23, 2011 at 7:09 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> ---
>  hw/esp.c         |   60 +++++++++++++++++---------------
>  hw/lsi53c895a.c  |   48 +++++++++++++++-----------
>  hw/scsi-bus.c    |    4 +-
>  hw/scsi.h        |    9 +----
>  hw/spapr_vscsi.c |  101 ++++++++++++++++++++++++++++++------------------------
>  hw/usb-msd.c     |   69 +++++++++++++++++++++----------------
>  6 files changed, 159 insertions(+), 132 deletions(-)
>
> diff --git a/hw/esp.c b/hw/esp.c
> index 0b80529..32ed327 100644
> --- a/hw/esp.c
> +++ b/hw/esp.c
> @@ -395,38 +395,41 @@ static void esp_do_dma(ESPState *s)
>     esp_dma_done(s);
>  }
>
> -static void esp_command_complete(SCSIRequest *req, int reason, uint32_t arg)
> +static void esp_command_complete(SCSIRequest *req, uint32_t arg)
>  {
>     ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
>
> -    if (reason == SCSI_REASON_DONE) {
> -        DPRINTF("SCSI Command complete\n");
> -        if (s->ti_size != 0)
> -            DPRINTF("SCSI command completed unexpectedly\n");
> -        s->ti_size = 0;
> -        s->dma_left = 0;
> -        s->async_len = 0;
> -        if (arg)
> -            DPRINTF("Command failed\n");
> -        s->status = arg;
> -        s->rregs[ESP_RSTAT] = STAT_ST;
> +    DPRINTF("SCSI Command complete\n");
> +    if (s->ti_size != 0)

While touching the code, please add braces.

> +        DPRINTF("SCSI command completed unexpectedly\n");
> +    s->ti_size = 0;
> +    s->dma_left = 0;
> +    s->async_len = 0;
> +    if (arg)
> +        DPRINTF("Command failed\n");
> +    s->status = arg;
> +    s->rregs[ESP_RSTAT] = STAT_ST;
> +    esp_dma_done(s);
> +    if (s->current_req) {
> +        scsi_req_unref(s->current_req);
> +        s->current_req = NULL;
> +        s->current_dev = NULL;
> +    }
> +}
> +
> +static void esp_transfer_data(SCSIRequest *req, uint32_t arg)
> +{
> +    ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
> +
> +    DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
> +    s->async_len = arg;
> +    s->async_buf = scsi_req_get_buf(req);
> +    if (s->dma_left) {
> +        esp_do_dma(s);
> +    } else if (s->dma_counter != 0 && s->ti_size <= 0) {
> +        /* If this was the last part of a DMA transfer then the
> +           completion interrupt is deferred to here.  */
>         esp_dma_done(s);
> -       if (s->current_req) {
> -            scsi_req_unref(s->current_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 = scsi_req_get_buf(req);
> -        if (s->dma_left) {
> -            esp_do_dma(s);
> -        } else if (s->dma_counter != 0 && s->ti_size <= 0) {
> -            /* If this was the last part of a DMA transfer then the
> -               completion interrupt is deferred to here.  */
> -            esp_dma_done(s);
> -        }
>     }
>  }
>
> @@ -725,6 +728,7 @@ void esp_init(target_phys_addr_t espaddr, int it_shift,
>  }
>
>  static struct SCSIBusOps esp_scsi_ops = {
> +    .transfer_data = esp_transfer_data,
>     .complete = esp_command_complete,
>     .cancel = esp_request_cancelled
>  };
> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> index a6c7c5b..62fe191 100644
> --- a/hw/lsi53c895a.c
> +++ b/hw/lsi53c895a.c
> @@ -711,40 +711,47 @@ static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
>         return 1;
>     }
>  }
> - /* Callback to indicate that the SCSI layer has completed a transfer.  */
> -static void lsi_command_complete(SCSIRequest *req, int reason, uint32_t arg)
> +
> + /* Callback to indicate that the SCSI layer has completed a command.  */
> +static void lsi_command_complete(SCSIRequest *req, uint32_t arg)
>  {
>     LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
>     int out;
>
>     out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
> -    if (reason == SCSI_REASON_DONE) {
> -        DPRINTF("Command complete status=%d\n", (int)arg);
> -        s->status = arg;
> -        s->command_complete = 2;
> -        if (s->waiting && s->dbc != 0) {
> -            /* Raise phase mismatch for short transfers.  */
> -            lsi_bad_phase(s, out, PHASE_ST);
> -        } else {
> -            lsi_set_phase(s, PHASE_ST);
> -        }
> +    DPRINTF("Command complete status=%d\n", (int)arg);
> +    s->status = arg;
> +    s->command_complete = 2;
> +    if (s->waiting && s->dbc != 0) {
> +        /* Raise phase mismatch for short transfers.  */
> +        lsi_bad_phase(s, out, PHASE_ST);
> +    } else {
> +        lsi_set_phase(s, PHASE_ST);
> +    }
>
> -        if (req == s->current->req) {
> -            scsi_req_unref(s->current->req);
> -            qemu_free(s->current);
> -            s->current = NULL;
> -       }
> -        lsi_resume_script(s);
> -        return;
> +    if (req == s->current->req) {
> +        scsi_req_unref(s->current->req);
> +        qemu_free(s->current);
> +        s->current = NULL;
>     }
> +    lsi_resume_script(s);
> +}
> +
> + /* Callback to indicate that the SCSI layer has completed a transfer.  */
> +static void lsi_transfer_data(SCSIRequest *req, uint32_t arg)
> +{
> +    LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
> +    int out;
>
>     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, req->tag, arg)) {
>             return;
> -        }
> +       }
>     }
>
> +    out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
> +
>     /* host adapter (re)connected */
>     DPRINTF("Data ready tag=0x%x len=%d\n", req->tag, arg);
>     s->current->dma_len = arg;
> @@ -2239,6 +2246,7 @@ static int lsi_scsi_uninit(PCIDevice *d)
>  }
>
>  static struct SCSIBusOps lsi_scsi_ops = {
> +    .transfer_data = lsi_transfer_data,
>     .complete = lsi_command_complete,
>     .cancel = lsi_request_cancelled
>  };
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index d4fd61d..086a998 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -628,7 +628,7 @@ void scsi_req_continue(SCSIRequest *req)
>  void scsi_req_data(SCSIRequest *req, int len)
>  {
>     trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
> -    req->bus->ops->complete(req, SCSI_REASON_DATA, len);
> +    req->bus->ops->transfer_data(req, len);
>  }
>
>  void scsi_req_print(SCSIRequest *req)
> @@ -664,7 +664,7 @@ void scsi_req_complete(SCSIRequest *req)
>     assert(req->status != -1);
>     scsi_req_ref(req);
>     scsi_req_dequeue(req);
> -    req->bus->ops->complete(req, SCSI_REASON_DONE, req->status);
> +    req->bus->ops->complete(req, req->status);
>     scsi_req_unref(req);
>  }
>
> diff --git a/hw/scsi.h b/hw/scsi.h
> index fa9c9fa..70f9e89 100644
> --- a/hw/scsi.h
> +++ b/hw/scsi.h
> @@ -9,12 +9,6 @@
>
>  #define SCSI_CMD_BUF_SIZE     16
>
> -/* scsi-disk.c */
> -enum scsi_reason {
> -    SCSI_REASON_DONE, /* Command complete.  */
> -    SCSI_REASON_DATA  /* Transfer complete, more data required.  */
> -};
> -
>  typedef struct SCSIBus SCSIBus;
>  typedef struct SCSIBusOps SCSIBusOps;
>  typedef struct SCSIDevice SCSIDevice;
> @@ -84,7 +78,8 @@ struct SCSIDeviceInfo {
>  };
>
>  struct SCSIBusOps {
> -    void (*complete)(SCSIRequest *req, int reason, uint32_t arg);
> +    void (*transfer_data)(SCSIRequest *req, uint32_t arg);
> +    void (*complete)(SCSIRequest *req, uint32_t arg);
>     void (*cancel)(SCSIRequest *req);
>  };
>
> diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
> index f0e08e3..d84e10d 100644
> --- a/hw/spapr_vscsi.c
> +++ b/hw/spapr_vscsi.c
> @@ -480,63 +480,34 @@ static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
>  }
>
>  /* Callback to indicate that the SCSI layer has completed a transfer.  */
> -static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
> +static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t arg)
>  {
>     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
>     vscsi_req *req = vscsi_find_req(s, sreq);
>     uint8_t *buf;
> -    int32_t res_in = 0, res_out = 0;
>     int len, rc = 0;
>
> -    dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x arg=0x%x, req=%p\n",
> -            reason, sreq->tag, arg, req);
> +    dprintf("VSCSI: SCSI xfer complete tag=0x%x arg=0x%x, req=%p\n",
> +            sreq->tag, arg, req);
>     if (req == NULL) {
>         fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
>         return;
>     }
>
>     if (req->sensing) {
> -        if (reason == SCSI_REASON_DONE) {
> -            dprintf("VSCSI: Sense done !\n");
> -            vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
> -            vscsi_put_req(s, req);
> -        } else {
> -            uint8_t *buf = scsi_req_get_buf(sreq);
> -
> -            len = MIN(arg, SCSI_SENSE_BUF_SIZE);
> -            dprintf("VSCSI: Sense data, %d bytes:\n", len);
> -            dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
> -                    buf[0], buf[1], buf[2], buf[3],
> -                    buf[4], buf[5], buf[6], buf[7]);
> -            dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
> -                    buf[8], buf[9], buf[10], buf[11],
> -                    buf[12], buf[13], buf[14], buf[15]);
> -            memcpy(req->sense, buf, len);
> -            req->senselen = len;
> -            scsi_req_continue(req->sreq);
> -        }
> -        return;
> -    }
> -
> -    if (reason == SCSI_REASON_DONE) {
> -        dprintf("VSCSI: Command complete err=%d\n", arg);
> -        if (arg == 0) {
> -            /* We handle overflows, not underflows for normal commands,
> -             * but hopefully nobody cares
> -             */
> -            if (req->writing) {
> -                res_out = req->data_len;
> -            } else {
> -                res_in = req->data_len;
> -            }
> -            vscsi_send_rsp(s, req, 0, res_in, res_out);
> -        } else if (arg == CHECK_CONDITION) {
> -            vscsi_send_request_sense(s, req);
> -            return;
> -        } else {
> -            vscsi_send_rsp(s, req, arg, 0, 0);
> -        }
> -        vscsi_put_req(s, req);
> +        uint8_t *buf = scsi_req_get_buf(sreq);
> +
> +        len = MIN(arg, SCSI_SENSE_BUF_SIZE);
> +        dprintf("VSCSI: Sense data, %d bytes:\n", len);
> +        dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
> +                buf[0], buf[1], buf[2], buf[3],
> +                buf[4], buf[5], buf[6], buf[7]);
> +        dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
> +                buf[8], buf[9], buf[10], buf[11],
> +                buf[12], buf[13], buf[14], buf[15]);
> +        memcpy(req->sense, buf, len);
> +        req->senselen = len;
> +        scsi_req_continue(req->sreq);
>         return;
>     }
>
> @@ -559,6 +530,45 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
>     scsi_req_continue(sreq);
>  }
>
> +/* Callback to indicate that the SCSI layer has completed a transfer.  */
> +static void vscsi_command_complete(SCSIRequest *sreq, uint32_t arg)
> +{
> +    VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
> +    vscsi_req *req = vscsi_find_req(s, sreq);
> +    int32_t res_in = 0, res_out = 0;
> +
> +    dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x arg=0x%x, req=%p\n",
> +            reason, sreq->tag, arg, req);
> +    if (req == NULL) {
> +        fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
> +        return;
> +    }
> +
> +    if (!req->sensing && arg == CHECK_CONDITION) {
> +        vscsi_send_request_sense(s, req);
> +        return;
> +    }
> +
> +    if (req->sensing) {
> +        dprintf("VSCSI: Sense done !\n");
> +        arg = CHECK_CONDITION;
> +    } else {
> +        dprintf("VSCSI: Command complete err=%d\n", arg);
> +        if (arg == 0) {
> +            /* We handle overflows, not underflows for normal commands,
> +             * but hopefully nobody cares
> +             */
> +            if (req->writing) {
> +                res_out = req->data_len;
> +            } else {
> +                res_in = req->data_len;
> +            }
> +        }
> +    }
> +    vscsi_send_rsp(s, req, 0, res_in, res_out);
> +    vscsi_put_req(s, req);
> +}
> +
>  static void vscsi_request_cancelled(SCSIRequest *sreq)
>  {
>     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
> @@ -916,6 +926,7 @@ static int vscsi_do_crq(struct VIOsPAPRDevice *dev, uint8_t *crq_data)
>  }
>
>  static struct SCSIBusOps vscsi_scsi_ops = {
> +    .transfer_data = vscsi_transfer_data,
>     .complete = vscsi_command_complete,
>     .cancel = vscsi_request_cancelled
>  };
> diff --git a/hw/usb-msd.c b/hw/usb-msd.c
> index 27a37fa..c808815 100644
> --- a/hw/usb-msd.c
> +++ b/hw/usb-msd.c
> @@ -208,7 +208,7 @@ static void usb_msd_send_status(MSDState *s, USBPacket *p)
>     memcpy(p->data, &csw, len);
>  }
>
> -static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
> +static void usb_msd_transfer_data(SCSIRequest *req, uint32_t arg)
>  {
>     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
>     USBPacket *p = s->packet;
> @@ -216,35 +216,7 @@ static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
>     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);
> -        s->residue = s->data_len;
> -        s->result = arg != 0;
> -        if (s->packet) {
> -            if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
> -                /* A deferred packet with no write data remaining must be
> -                   the status read packet.  */
> -                usb_msd_send_status(s, p);
> -                s->mode = USB_MSDM_CBW;
> -            } else {
> -                if (s->data_len) {
> -                    s->data_len -= s->usb_len;
> -                    if (s->mode == USB_MSDM_DATAIN)
> -                        memset(s->usb_buf, 0, s->usb_len);
> -                    s->usb_len = 0;
> -                }
> -                if (s->data_len == 0)
> -                    s->mode = USB_MSDM_CSW;
> -            }
> -            s->packet = NULL;
> -            usb_packet_complete(&s->dev, p);
> -        } else if (s->data_len == 0) {
> -            s->mode = USB_MSDM_CSW;
> -        }
> -        scsi_req_unref(req);
> -        s->req = NULL;
> -        return;
> -    }
> +
>     assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == SCSI_XFER_TO_DEV));
>     s->scsi_len = arg;
>     s->scsi_buf = scsi_req_get_buf(req);
> @@ -261,6 +233,42 @@ static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
>     }
>  }
>
> +static void usb_msd_command_complete(SCSIRequest *req, uint32_t arg)
> +{
> +    MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
> +    USBPacket *p = s->packet;
> +
> +    if (req->tag != s->tag) {
> +        fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
> +    }
> +    DPRINTF("Command complete %d\n", arg);
> +    s->residue = s->data_len;
> +    s->result = arg != 0;
> +    if (s->packet) {
> +        if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
> +            /* A deferred packet with no write data remaining must be
> +               the status read packet.  */
> +            usb_msd_send_status(s, p);
> +            s->mode = USB_MSDM_CBW;
> +        } else {
> +            if (s->data_len) {
> +                s->data_len -= s->usb_len;
> +                if (s->mode == USB_MSDM_DATAIN)
> +                    memset(s->usb_buf, 0, s->usb_len);
> +                s->usb_len = 0;
> +            }
> +            if (s->data_len == 0)
> +                s->mode = USB_MSDM_CSW;
> +        }
> +        s->packet = NULL;
> +        usb_packet_complete(&s->dev, p);
> +    } else if (s->data_len == 0) {
> +        s->mode = USB_MSDM_CSW;
> +    }
> +    scsi_req_unref(req);
> +    s->req = NULL;
> +}
> +
>  static void usb_msd_request_cancelled(SCSIRequest *req)
>  {
>     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
> @@ -494,6 +502,7 @@ static void usb_msd_password_cb(void *opaque, int err)
>  }
>
>  static struct SCSIBusOps usb_msd_scsi_ops = {
> +    .transfer_data = usb_msd_transfer_data,
>     .complete = usb_msd_command_complete,
>     .cancel = usb_msd_request_cancelled
>  };
> --
> 1.7.4.4
>
>
>
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PATCH v4 22/24] scsi: split command_complete callback in two
  2011-05-23 19:38   ` Blue Swirl
@ 2011-05-24  7:32     ` Paolo Bonzini
  0 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2011-05-24  7:32 UTC (permalink / raw)
  To: Blue Swirl; +Cc: qemu-devel, hch

On 05/23/2011 09:38 PM, Blue Swirl wrote:
> While touching the code, please add braces.

Done.  However, both changes you requested touch several patches, so I 
cannot just send follow-ups for the two you replied to.  Is it ok to 
just send a pull request for v5?

Paolo

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2011-05-24  7:32 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-23 16:08 [Qemu-devel] [PATCH v4 00/24] SCSI subsystem improvements Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 01/24] scsi: add tracing of scsi requests Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 02/24] scsi-generic: Remove bogus double complete Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 03/24] scsi: introduce scsi_req_data Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 04/24] scsi: introduce SCSIBusOps Paolo Bonzini
2011-05-23 19:35   ` Blue Swirl
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 05/24] scsi: reference-count requests Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 06/24] lsi: extract lsi_find_by_tag Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 07/24] scsi: Use 'SCSIRequest' directly Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 08/24] scsi: commonize purging requests Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 09/24] scsi: introduce scsi_req_abort Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 10/24] scsi: introduce scsi_req_cancel Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 11/24] scsi: use scsi_req_complete Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 12/24] scsi: Update sense code handling Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 13/24] scsi: do not call send_command directly Paolo Bonzini
2011-05-23 16:08 ` [Qemu-devel] [PATCH v4 14/24] scsi: introduce scsi_req_new Paolo Bonzini
2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 15/24] scsi: introduce scsi_req_continue Paolo Bonzini
2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 16/24] scsi: introduce scsi_req_get_buf Paolo Bonzini
2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 17/24] scsi: Implement 'get_sense' callback Paolo Bonzini
2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 18/24] scsi-disk: add data direction checking Paolo Bonzini
2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 19/24] scsi: make write_data return void Paolo Bonzini
2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 20/24] scsi-generic: Handle queue full Paolo Bonzini
2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 21/24] esp: rename sense to status Paolo Bonzini
2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 22/24] scsi: split command_complete callback in two Paolo Bonzini
2011-05-23 19:38   ` Blue Swirl
2011-05-24  7:32     ` Paolo Bonzini
2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 23/24] scsi: rename arguments to the new callbacks Paolo Bonzini
2011-05-23 16:09 ` [Qemu-devel] [PATCH v4 24/24] scsi: ignore LUN field in the CDB Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.