[Qemu-devel] [PATCH 54/55] scsi: push request restart to SCSIDevice

[Kevin Wolf]

From: Paolo Bonzini <address@hidden>

The request restart mechanism is generic and could be reused for
scsi-generic.  In the meanwhile, pushing it to SCSIDevice avoids
that scsi_dma_restart_bh looks at SCSIGenericReqs when working on
a scsi-block device.

The code is the same that is already in hw/scsi-disk.c, with
the type flags replaced by req->cmd.mode and a more generic way to
requeue SCSI_XFER_NONE commands.

I also added a missing call to qemu_del_vm_change_state_handler.

Signed-off-by: Paolo Bonzini <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
---
 hw/scsi-bus.c  |   56 +++++++++++++++++++++++++++++++++++++++++
 hw/scsi-disk.c |   76 +++++--------------------------------------------------
 hw/scsi.h      |    5 +++
 3 files changed, 68 insertions(+), 69 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index dfce5fb..e6ebbd5 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -8,6 +8,7 @@
 
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
 static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
+static void scsi_req_dequeue(SCSIRequest *req);
 static int scsi_build_sense(uint8_t *in_buf, int in_len,
                             uint8_t *buf, int len, bool fixed);
 
@@ -33,6 +34,53 @@ void scsi_bus_new(SCSIBus *bus, DeviceState *host, const 
SCSIBusInfo *info)
     bus->qbus.allow_hotplug = 1;
 }
 
+static void scsi_dma_restart_bh(void *opaque)
+{
+    SCSIDevice *s = opaque;
+    SCSIRequest *req, *next;
+
+    qemu_bh_delete(s->bh);
+    s->bh = NULL;
+
+    QTAILQ_FOREACH_SAFE(req, &s->requests, next, next) {
+        scsi_req_ref(req);
+        if (req->retry) {
+            req->retry = false;
+            switch (req->cmd.mode) {
+            case SCSI_XFER_FROM_DEV:
+            case SCSI_XFER_TO_DEV:
+                scsi_req_continue(req);
+                break;
+            case SCSI_XFER_NONE:
+                scsi_req_dequeue(req);
+                scsi_req_enqueue(req);
+                break;
+            }
+        }
+        scsi_req_unref(req);
+    }
+}
+
+void scsi_req_retry(SCSIRequest *req)
+{
+    /* No need to save a reference, because scsi_dma_restart_bh just
+     * looks at the request list.  */
+    req->retry = true;
+}
+
+static void scsi_dma_restart_cb(void *opaque, int running, RunState state)
+{
+    SCSIDevice *s = opaque;
+
+    if (!running) {
+        return;
+    }
+    if (!s->bh) {
+        s->bh = qemu_bh_new(scsi_dma_restart_bh, s);
+        qemu_bh_schedule(s->bh);
+    }
+}
+
 static int scsi_qdev_init(DeviceState *qdev, DeviceInfo *base)
 {
     SCSIDevice *dev = DO_UPCAST(SCSIDevice, qdev, qdev);
@@ -83,6 +131,10 @@ static int scsi_qdev_init(DeviceState *qdev, DeviceInfo 
*base)
     dev->info = info;
     QTAILQ_INIT(&dev->requests);
     rc = dev->info->init(dev);
+    if (rc == 0) {
+        dev->vmsentry = qemu_add_vm_change_state_handler(scsi_dma_restart_cb,
+                                                         dev);
+    }
 
 err:
     return rc;
@@ -92,6 +144,9 @@ static int scsi_qdev_exit(DeviceState *qdev)
 {
     SCSIDevice *dev = DO_UPCAST(SCSIDevice, qdev, qdev);
 
+    if (dev->vmsentry) {
+        qemu_del_vm_change_state_handler(dev->vmsentry);
+    }
     if (dev->info->destroy) {
         dev->info->destroy(dev);
     }
@@ -586,6 +641,7 @@ int32_t scsi_req_enqueue(SCSIRequest *req)
 static void scsi_req_dequeue(SCSIRequest *req)
 {
     trace_scsi_req_dequeue(req->dev->id, req->lun, req->tag);
+    req->retry = false;
     if (req->enqueued) {
         QTAILQ_REMOVE(&req->dev->requests, req, next);
         req->enqueued = false;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index bb07737..352d41f 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -42,12 +42,6 @@ do { fprintf(stderr, "scsi-disk: " fmt , ## __VA_ARGS__); } 
while (0)
 #define SCSI_DMA_BUF_SIZE    131072
 #define SCSI_MAX_INQUIRY_LEN 256
 
-#define SCSI_REQ_STATUS_RETRY           0x01
-#define SCSI_REQ_STATUS_RETRY_TYPE_MASK 0x06
-#define SCSI_REQ_STATUS_RETRY_READ      0x00
-#define SCSI_REQ_STATUS_RETRY_WRITE     0x02
-#define SCSI_REQ_STATUS_RETRY_FLUSH     0x04
-
 typedef struct SCSIDiskState SCSIDiskState;
 
 typedef struct SCSIDiskReq {
@@ -58,7 +52,6 @@ typedef struct SCSIDiskReq {
     uint32_t buflen;
     struct iovec iov;
     QEMUIOVector qiov;
-    uint32_t status;
     BlockAcctCookie acct;
 } SCSIDiskReq;
 
@@ -75,8 +68,7 @@ struct SCSIDiskState
     bool tray_locked;
 };
 
-static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
-static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf);
+static int scsi_handle_rw_error(SCSIDiskReq *r, int error);
 
 static void scsi_free_request(SCSIRequest *req)
 {
@@ -102,7 +94,6 @@ static void scsi_cancel_io(SCSIRequest *req)
     SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
 
     DPRINTF("Cancel tag=0x%x\n", req->tag);
-    r->status &= ~(SCSI_REQ_STATUS_RETRY | SCSI_REQ_STATUS_RETRY_TYPE_MASK);
     if (r->req.aiocb) {
         bdrv_aio_cancel(r->req.aiocb);
 
@@ -139,7 +130,7 @@ static void scsi_read_complete(void * opaque, int ret)
     }
 
     if (ret) {
-        if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_READ)) {
+        if (scsi_handle_rw_error(r, -ret)) {
             goto done;
         }
     }
@@ -168,7 +159,7 @@ static void scsi_flush_complete(void * opaque, int ret)
     }
 
     if (ret < 0) {
-        if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_FLUSH)) {
+        if (scsi_handle_rw_error(r, -ret)) {
             goto done;
         }
     }
@@ -233,9 +224,9 @@ static void scsi_read_data(SCSIRequest *req)
  * scsi_handle_rw_error always manages its reference counts, independent
  * of the return value.
  */
-static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
+static int scsi_handle_rw_error(SCSIDiskReq *r, int error)
 {
-    int is_read = (type == SCSI_REQ_STATUS_RETRY_READ);
+    int is_read = (r->req.cmd.xfer == SCSI_XFER_FROM_DEV);
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     BlockErrorAction action = bdrv_get_on_error(s->qdev.conf.bs, is_read);
 
@@ -247,17 +238,10 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int 
error, int type)
     if ((error == ENOSPC && action == BLOCK_ERR_STOP_ENOSPC)
             || action == BLOCK_ERR_STOP_ANY) {
 
-        type &= SCSI_REQ_STATUS_RETRY_TYPE_MASK;
-        r->status |= SCSI_REQ_STATUS_RETRY | type;
-
         bdrv_mon_event(s->qdev.conf.bs, BDRV_ACTION_STOP, is_read);
         vm_stop(RUN_STATE_IO_ERROR);
         bdrv_iostatus_set_err(s->qdev.conf.bs, error);
-
-        /* No need to save a reference, because scsi_dma_restart_bh just
-         * looks at the request list.  If a request is canceled, the
-         * retry request is just dropped.
-         */
+        scsi_req_retry(&r->req);
     } else {
         switch (error) {
         case ENOMEDIUM:
@@ -290,7 +274,7 @@ static void scsi_write_complete(void * opaque, int ret)
     }
 
     if (ret) {
-        if (scsi_handle_rw_error(r, -ret, SCSI_REQ_STATUS_RETRY_WRITE)) {
+        if (scsi_handle_rw_error(r, -ret)) {
             goto done;
         }
     }
@@ -347,51 +331,6 @@ static void scsi_write_data(SCSIRequest *req)
     }
 }
 
-static void scsi_dma_restart_bh(void *opaque)
-{
-    SCSIDiskState *s = opaque;
-    SCSIRequest *req;
-    SCSIDiskReq *r;
-
-    qemu_bh_delete(s->bh);
-    s->bh = NULL;
-
-    QTAILQ_FOREACH(req, &s->qdev.requests, next) {
-        r = DO_UPCAST(SCSIDiskReq, req, req);
-        if (r->status & SCSI_REQ_STATUS_RETRY) {
-            int status = r->status;
-
-            r->status &=
-                ~(SCSI_REQ_STATUS_RETRY | SCSI_REQ_STATUS_RETRY_TYPE_MASK);
-
-            switch (status & SCSI_REQ_STATUS_RETRY_TYPE_MASK) {
-            case SCSI_REQ_STATUS_RETRY_READ:
-                scsi_read_data(&r->req);
-                break;
-            case SCSI_REQ_STATUS_RETRY_WRITE:
-                scsi_write_data(&r->req);
-                break;
-            case SCSI_REQ_STATUS_RETRY_FLUSH:
-                scsi_send_command(&r->req, r->req.cmd.buf);
-                break;
-            }
-        }
-    }
-}
-
-static void scsi_dma_restart_cb(void *opaque, int running, RunState state)
-{
-    SCSIDiskState *s = opaque;
-
-    if (!running) {
-        return;
-    }
-    if (!s->bh) {
-        s->bh = qemu_bh_new(scsi_dma_restart_bh, s);
-        qemu_bh_schedule(s->bh);
-    }
-}
-
 /* Return a pointer to the data buffer.  */
 static uint8_t *scsi_get_buf(SCSIRequest *req)
 {
@@ -1591,7 +1530,6 @@ static int scsi_initfn(SCSIDevice *dev)
     }
     bdrv_set_buffer_alignment(s->qdev.conf.bs, s->qdev.blocksize);
 
-    qemu_add_vm_change_state_handler(scsi_dma_restart_cb, s);
     bdrv_iostatus_enable(s->qdev.conf.bs);
     add_boot_device_path(s->qdev.conf.bootindex, &dev->qdev, ",0");
     return 0;
diff --git a/hw/scsi.h b/hw/scsi.h
index fcc3455..ff8fdd0 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -3,6 +3,7 @@
 
 #include "qdev.h"
 #include "block.h"
+#include "sysemu.h"
 
 #define MAX_SCSI_DEVS  255
 
@@ -52,6 +53,7 @@ struct SCSIRequest {
     uint32_t sense_len;
     bool enqueued;
     bool io_canceled;
+    bool retry;
     void *hba_private;
     QTAILQ_ENTRY(SCSIRequest) next;
 };
@@ -59,6 +61,8 @@ struct SCSIRequest {
 struct SCSIDevice
 {
     DeviceState qdev;
+    VMChangeStateEntry *vmsentry;
+    QEMUBH *bh;
     uint32_t id;
     BlockConf conf;
     SCSIDeviceInfo *info;
@@ -194,6 +198,7 @@ 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_req_retry(SCSIRequest *req);
 void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense);
 int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed);
 SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int target, int lun);
-- 
1.7.6.4