diff mbox

[2/3] scsi-disk: identify AIO callbacks more clearly

Message ID 1438855297-28377-3-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Aug. 6, 2015, 10:01 a.m. UTC 
Functions that are not callbacks should assert that aiocb is NULL and
have a non-opaque argument (usually a pointer to SCSIDiskReq).

AIO callbacks should assert that aiocb is not NULL and take care of
calling block_acct done.  They also have an opaque argument.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-disk.c | 91 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 61 insertions(+), 30 deletions(-)

Comments

Fam Zheng Aug. 6, 2015, 12:35 p.m. UTC | #1 
On Thu, 08/06 12:01, Paolo Bonzini wrote:
>  /* Actually issue a read to the block device.  */
> -static void scsi_do_read(void *opaque, int ret)
> +static void scsi_do_read(SCSIDiskReq *r, int ret)
>  {
> -    SCSIDiskReq *r = opaque;
>      SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
>      uint32_t n;
>  
> -    if (r->req.aiocb != NULL) {
> -        r->req.aiocb = NULL;
> -        block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
> -    }
> +    assert (r->req.aiocb == NULL);
> +
>      if (r->req.io_canceled) {
>          scsi_req_cancel_complete(&r->req);
>          goto done;
> @@ -349,6 +347,18 @@ done:
>      scsi_req_unref(&r->req);
>  }
>  
> +static void scsi_do_read_cb(void *opaque, int ret)
> +{
> +    SCSIDiskReq *r = (SCSIDiskReq *)opaque;
> +    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
> +
> +    assert (r->req.aiocb != NULL);

Majority of asserts across the code base don't get whitespace in the middle,
but 4 out of 8 in this patch do :)
diff mbox

Patch

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 64f0694..8fb0f43 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -217,6 +217,8 @@  static void scsi_write_do_fua(SCSIDiskReq *r)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
 
+    assert(r->req.aiocb == NULL);
+
     if (r->req.io_canceled) {
         scsi_req_cancel_complete(&r->req);
         goto done;
@@ -235,15 +237,10 @@  done:
     scsi_req_unref(&r->req);
 }
 
-static void scsi_dma_complete_noio(void *opaque, int ret)
+static void scsi_dma_complete_noio(SCSIDiskReq *r, int ret)
 {
-    SCSIDiskReq *r = (SCSIDiskReq *)opaque;
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+    assert(r->req.aiocb == NULL);
 
-    if (r->req.aiocb != NULL) {
-        r->req.aiocb = NULL;
-        block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
-    }
     if (r->req.io_canceled) {
         scsi_req_cancel_complete(&r->req);
         goto done;
@@ -271,9 +268,13 @@  done:
 static void scsi_dma_complete(void *opaque, int ret)
 {
     SCSIDiskReq *r = (SCSIDiskReq *)opaque;
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
 
     assert(r->req.aiocb != NULL);
-    scsi_dma_complete_noio(opaque, ret);
+    r->req.aiocb = NULL;
+
+    block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
+    scsi_dma_complete_noio(r, ret);
 }
 
 static void scsi_read_complete(void * opaque, int ret)
@@ -308,16 +309,13 @@  done:
 }
 
 /* Actually issue a read to the block device.  */
-static void scsi_do_read(void *opaque, int ret)
+static void scsi_do_read(SCSIDiskReq *r, int ret)
 {
-    SCSIDiskReq *r = opaque;
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     uint32_t n;
 
-    if (r->req.aiocb != NULL) {
-        r->req.aiocb = NULL;
-        block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
-    }
+    assert (r->req.aiocb == NULL);
+
     if (r->req.io_canceled) {
         scsi_req_cancel_complete(&r->req);
         goto done;
@@ -349,6 +347,18 @@  done:
     scsi_req_unref(&r->req);
 }
 
+static void scsi_do_read_cb(void *opaque, int ret)
+{
+    SCSIDiskReq *r = (SCSIDiskReq *)opaque;
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+
+    assert (r->req.aiocb != NULL);
+    r->req.aiocb = NULL;
+
+    block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
+    scsi_do_read(opaque, ret);
+}
+
 /* Read more data from scsi device into buffer.  */
 static void scsi_read_data(SCSIRequest *req)
 {
@@ -384,7 +394,7 @@  static void scsi_read_data(SCSIRequest *req)
     if (first && scsi_is_cmd_fua(&r->req.cmd)) {
         block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct, 0,
                          BLOCK_ACCT_FLUSH);
-        r->req.aiocb = blk_aio_flush(s->qdev.conf.blk, scsi_do_read, r);
+        r->req.aiocb = blk_aio_flush(s->qdev.conf.blk, scsi_do_read_cb, r);
     } else {
         scsi_do_read(r, 0);
     }
@@ -430,16 +440,12 @@  static int scsi_handle_rw_error(SCSIDiskReq *r, int error)
     return action != BLOCK_ERROR_ACTION_IGNORE;
 }
 
-static void scsi_write_complete(void * opaque, int ret)
+static void scsi_write_complete_noio(SCSIDiskReq *r, int ret)
 {
-    SCSIDiskReq *r = (SCSIDiskReq *)opaque;
-    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     uint32_t n;
 
-    if (r->req.aiocb != NULL) {
-        r->req.aiocb = NULL;
-        block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
-    }
+    assert (r->req.aiocb == NULL);
+
     if (r->req.io_canceled) {
         scsi_req_cancel_complete(&r->req);
         goto done;
@@ -467,6 +473,18 @@  done:
     scsi_req_unref(&r->req);
 }
 
+static void scsi_write_complete(void * opaque, int ret)
+{
+    SCSIDiskReq *r = (SCSIDiskReq *)opaque;
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+
+    assert (r->req.aiocb != NULL);
+    r->req.aiocb = NULL;
+
+    block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
+    scsi_write_complete_noio(r, ret);
+}
+
 static void scsi_write_data(SCSIRequest *req)
 {
     SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
@@ -480,18 +498,18 @@  static void scsi_write_data(SCSIRequest *req)
     scsi_req_ref(&r->req);
     if (r->req.cmd.mode != SCSI_XFER_TO_DEV) {
         DPRINTF("Data transfer direction invalid\n");
-        scsi_write_complete(r, -EINVAL);
+        scsi_write_complete_noio(r, -EINVAL);
         return;
     }
 
     if (!r->req.sg && !r->qiov.size) {
         /* Called for the first time.  Ask the driver to send us more data.  */
         r->started = true;
-        scsi_write_complete(r, 0);
+        scsi_write_complete_noio(r, 0);
         return;
     }
     if (s->tray_open) {
-        scsi_write_complete(r, -ENOMEDIUM);
+        scsi_write_complete_noio(r, -ENOMEDIUM);
         return;
     }
 
@@ -500,7 +518,7 @@  static void scsi_write_data(SCSIRequest *req)
         if (r->req.sg) {
             scsi_dma_complete_noio(r, 0);
         } else {
-            scsi_write_complete(r, 0);
+            scsi_write_complete_noio(r, 0);
         }
         return;
     }
@@ -1557,15 +1575,17 @@  typedef struct UnmapCBData {
     int count;
 } UnmapCBData;
 
-static void scsi_unmap_complete(void *opaque, int ret)
+static void scsi_unmap_complete(void *opaque, int ret);
+
+static void scsi_unmap_complete_noio(UnmapCBData *data, int ret)
 {
-    UnmapCBData *data = opaque;
     SCSIDiskReq *r = data->r;
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
     uint64_t sector_num;
     uint32_t nb_sectors;
 
-    r->req.aiocb = NULL;
+    assert(r->req.aiocb == NULL);
+
     if (r->req.io_canceled) {
         scsi_req_cancel_complete(&r->req);
         goto done;
@@ -1601,6 +1621,17 @@  done:
     g_free(data);
 }
 
+static void scsi_unmap_complete(void *opaque, int ret)
+{
+    UnmapCBData *data = opaque;
+    SCSIDiskReq *r = data->r;
+
+    assert(r->req.aiocb != NULL);
+    r->req.aiocb = NULL;
+
+    scsi_unmap_complete_noio(data, ret);
+}
+
 static void scsi_disk_emulate_unmap(SCSIDiskReq *r, uint8_t *inbuf)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
@@ -1638,7 +1669,7 @@  static void scsi_disk_emulate_unmap(SCSIDiskReq *r, uint8_t *inbuf)
 
     /* The matching unref is in scsi_unmap_complete, before data is freed.  */
     scsi_req_ref(&r->req);
-    scsi_unmap_complete(data, 0);
+    scsi_unmap_complete_noio(data, 0);
     return;
 
 invalid_param_len: