[Qemu-devel] [RFC PATCH 9/9] scsi: Cancel request asynchronously

[Fam Zheng <famz@redhat.com>]

We are blocking the whole VM, which means that an irresponsive storage
backend will hang the whole guest. Let's switch to bdrv_aio_cancel_async
to improve this.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/scsi/scsi-bus.c     |  6 ++----
 hw/scsi/scsi-disk.c    | 41 ++++++++++-------------------------------
 hw/scsi/scsi-generic.c | 21 ++++++---------------
 3 files changed, 18 insertions(+), 50 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 6f4462b..59ec9f9 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1615,7 +1615,6 @@ void scsi_req_continue(SCSIRequest *req)
 {
     if (req->io_canceled) {
         trace_scsi_req_continue_canceled(req->dev->id, req->lun, req->tag);
-        return;
     }
     trace_scsi_req_continue(req->dev->id, req->lun, req->tag);
     if (req->cmd.mode == SCSI_XFER_TO_DEV) {
@@ -1633,7 +1632,6 @@ void scsi_req_data(SCSIRequest *req, int len)
     uint8_t *buf;
     if (req->io_canceled) {
         trace_scsi_req_data_canceled(req->dev->id, req->lun, req->tag, len);
-        return;
     }
     trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
     assert(req->cmd.mode != SCSI_XFER_NONE);
@@ -1721,7 +1719,7 @@ void scsi_req_complete(SCSIRequest *req, int status)
 void scsi_req_cancel(SCSIRequest *req)
 {
     trace_scsi_req_cancel(req->dev->id, req->lun, req->tag);
-    if (!req->enqueued) {
+    if (req->io_canceled) {
         return;
     }
     scsi_req_ref(req);
@@ -1738,7 +1736,7 @@ void scsi_req_cancel(SCSIRequest *req)
 
 void scsi_req_abort(SCSIRequest *req, int status)
 {
-    if (!req->enqueued) {
+    if (req->io_canceled) {
         return;
     }
     scsi_req_ref(req);
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index d55521d..46b1e53 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -112,14 +112,9 @@ static void scsi_cancel_io(SCSIRequest *req)
 
     DPRINTF("Cancel tag=0x%x\n", req->tag);
     if (r->req.aiocb) {
-        bdrv_aio_cancel(r->req.aiocb);
-
-        /* This reference was left in by scsi_*_data.  We take ownership of
-         * it the moment scsi_req_cancel is called, independent of whether
-         * bdrv_aio_cancel completes the request or not.  */
-        scsi_req_unref(&r->req);
+        assert(req->io_canceled);
+        bdrv_aio_cancel_async(r->req.aiocb);
     }
-    r->req.aiocb = NULL;
 }
 
 static uint32_t scsi_init_iovec(SCSIDiskReq *r, size_t size)
@@ -197,9 +192,7 @@ static void scsi_aio_complete(void *opaque, int ret)
     scsi_req_complete(&r->req, GOOD);
 
 done:
-    if (!r->req.io_canceled) {
-        scsi_req_unref(&r->req);
-    }
+    scsi_req_unref(&r->req);
 }
 
 static bool scsi_is_cmd_fua(SCSICommand *cmd)
@@ -245,9 +238,7 @@ static void scsi_write_do_fua(SCSIDiskReq *r)
     scsi_req_complete(&r->req, GOOD);
 
 done:
-    if (!r->req.io_canceled) {
-        scsi_req_unref(&r->req);
-    }
+    scsi_req_unref(&r->req);
 }
 
 static void scsi_dma_complete_noio(void *opaque, int ret)
@@ -279,9 +270,7 @@ static void scsi_dma_complete_noio(void *opaque, int ret)
     }
 
 done:
-    if (!r->req.io_canceled) {
-        scsi_req_unref(&r->req);
-    }
+    scsi_req_unref(&r->req);
 }
 
 static void scsi_dma_complete(void *opaque, int ret)
@@ -319,9 +308,7 @@ static void scsi_read_complete(void * opaque, int ret)
     scsi_req_data(&r->req, r->qiov.size);
 
 done:
-    if (!r->req.io_canceled) {
-        scsi_req_unref(&r->req);
-    }
+    scsi_req_unref(&r->req);
 }
 
 /* Actually issue a read to the block device.  */
@@ -361,9 +348,7 @@ static void scsi_do_read(void *opaque, int ret)
     }
 
 done:
-    if (!r->req.io_canceled) {
-        scsi_req_unref(&r->req);
-    }
+    scsi_req_unref(&r->req);
 }
 
 /* Read more data from scsi device into buffer.  */
@@ -478,9 +463,7 @@ static void scsi_write_complete(void * opaque, int ret)
     }
 
 done:
-    if (!r->req.io_canceled) {
-        scsi_req_unref(&r->req);
-    }
+    scsi_req_unref(&r->req);
 }
 
 static void scsi_write_data(SCSIRequest *req)
@@ -1577,9 +1560,7 @@ static void scsi_unmap_complete(void *opaque, int ret)
     scsi_req_complete(&r->req, GOOD);
 
 done:
-    if (!r->req.io_canceled) {
-        scsi_req_unref(&r->req);
-    }
+    scsi_req_unref(&r->req);
     g_free(data);
 }
 
@@ -1672,9 +1653,7 @@ static void scsi_write_same_complete(void *opaque, int ret)
     scsi_req_complete(&r->req, GOOD);
 
 done:
-    if (!r->req.io_canceled) {
-        scsi_req_unref(&r->req);
-    }
+    scsi_req_unref(&r->req);
     qemu_vfree(data->iov.iov_base);
     g_free(data);
 }
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 0b2ff90..1c29ecb 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -132,27 +132,20 @@ static void scsi_command_complete(void *opaque, int ret)
     DPRINTF("Command complete 0x%p tag=0x%x status=%d\n",
             r, r->req.tag, status);
 
-    scsi_req_complete(&r->req, status);
     if (!r->req.io_canceled) {
-        scsi_req_unref(&r->req);
+        scsi_req_complete(&r->req, status);
     }
+    scsi_req_unref(&r->req);
 }
 
 /* Cancel a pending data transfer.  */
 static void scsi_cancel_io(SCSIRequest *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);
-
-        /* This reference was left in by scsi_*_data.  We take ownership of
-         * it independent of whether bdrv_aio_cancel completes the request
-         * or not.  */
-        scsi_req_unref(&r->req);
+    if (req->aiocb) {
+        req->io_canceled = true;
+        bdrv_aio_cancel_async(req->aiocb);
     }
-    r->req.aiocb = NULL;
 }
 
 static int execute_command(BlockDriverState *bdrv,
@@ -211,9 +204,7 @@ static void scsi_read_complete(void * opaque, int ret)
         bdrv_set_guest_block_size(s->conf.bs, s->blocksize);
 
         scsi_req_data(&r->req, len);
-        if (!r->req.io_canceled) {
-            scsi_req_unref(&r->req);
-        }
+        scsi_req_unref(&r->req);
     }
 }
 
-- 
2.0.3