[Qemu-devel] [PATCH v4 00/20] block: Asynchronous request cancellation

[Qemu-devel] [PATCH v4 00/20] block: Asynchronous request cancellation

[Fam Zheng]

v4: Drop AIOCBInfo.cancel.

This series adds a new block layer API:

  void bdrv_aio_cancel_async(BlockDriverAIOCB *acb);

And use it to emulate bdrv_aio_cancel.

The function is similar to bdrv_aio_cancel in that it cancels an AIO request,
but different that it doesn't block until the request is completely cancelled
or done.

More importantly, the completion callback, BlockDriverAIOCB.cb, is guaranteed
to be called, so that the cb can take care of resource releasing and status
reporting to guest, etc.

In the following work, scsi emulation code will be shifted to use the async
cancelling.

One major benefit would be that when guest tries to cancel a request, where the
request cannot be cancelled easily, (due to throttled BlockDriverState, a lost
connection, or a large request queue), we don't need to block the whole vm with
a busy loop, which is how bdrv_aio_cancel is implemented now.

A test case that is easy to reproduce is, throttle a scsi-disk to a very low
limit, for example 50 bps, then stress the guest block device with dd or fio.

Currently, the vm will quickly hang when it loses patience and send a tmf
command to cancel the request, at which point we will busy wait in
bdrv_aio_cancel, until the request is slowly spit out from throttled_reqs.

Later, we will change scsi device code to make this asynchronous, on top of
bdrv_aio_cancel_async.

Fam

Fam Zheng (20):
  block: Add refcnt in BlockDriverAIOCB
  block: Add bdrv_aio_cancel_async
  block: Drop bdrv_em_co_aiocb_info.cancel
  block: Convert bdrv_em_aiocb_info.cancel to .cancel_async
  thread-pool: Convert thread_pool_aiocb_info.cancel to cancel_async
  linux-aio: Convert laio_aiocb_info.cancel to .cancel_async
  dma: Check iov pointer before unmap memory
  dma: Convert dma_aiocb_info.cancel to .cancel_async
  iscsi: Convert iscsi_aiocb_info.cancel to .cancel_async
  archipelago: Drop archipelago_aiocb_info.cancel
  blkdebug: Convert blkdebug_aiocb_info.cancel to .cancel_async
  blkverify: Drop blkverify_aiocb_info.cancel
  curl: Drop curl_aiocb_info.cancel
  qed: Drop qed_aiocb_info.cancel
  quorum: Convert quorum_aiocb_info.cancel to .cancel_async
  rbd: Drop rbd_aiocb_info.cancel
  sheepdog: Convert sd_aiocb_info.cancel to .cancel_async
  win32-aio: Drop win32_aiocb_info.cancel
  ide: Convert trim_aiocb_info.cancel to .cancel_async
  block: Drop AIOCBInfo.cancel

 block.c                  | 61 +++++++++++++++++++++++++++++-------------------
 block/archipelago.c      | 17 +-------------
 block/blkdebug.c         |  6 +++--
 block/blkverify.c        | 19 ---------------
 block/curl.c             |  6 -----
 block/iscsi.c            | 17 ++------------
 block/linux-aio.c        | 30 +++++++-----------------
 block/qed.c              | 21 -----------------
 block/quorum.c           |  7 ++----
 block/rbd.c              | 23 +-----------------
 block/sheepdog.c         | 46 +++++++++++++++---------------------
 block/win32-aio.c        | 14 -----------
 dma-helpers.c            | 23 ++++++------------
 hw/ide/core.c            | 12 ++++------
 include/block/aio.h      |  5 +++-
 include/block/block.h    |  1 +
 tests/test-thread-pool.c | 34 ++++++++++++++++++++-------
 thread-pool.c            | 32 +++++++++++--------------
 18 files changed, 131 insertions(+), 243 deletions(-)

-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 01/20] block: Add refcnt in BlockDriverAIOCB

[Fam Zheng]

This will be useful in synchronous cancel emulation with
bdrv_aio_cancel_async.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c             | 12 +++++++++++-
 include/block/aio.h |  2 ++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index cb670fd..78d68cb 100644
--- a/block.c
+++ b/block.c
@@ -4885,13 +4885,23 @@ void *qemu_aio_get(const AIOCBInfo *aiocb_info, BlockDriverState *bs,
     acb->bs = bs;
     acb->cb = cb;
     acb->opaque = opaque;
+    acb->refcnt = 1;
     return acb;
 }
 
+void qemu_aio_ref(void *p)
+{
+    BlockDriverAIOCB *acb = p;
+    acb->refcnt++;
+}
+
 void qemu_aio_release(void *p)
 {
     BlockDriverAIOCB *acb = p;
-    g_slice_free1(acb->aiocb_info->aiocb_size, acb);
+    assert(acb->refcnt > 0);
+    if (--acb->refcnt == 0) {
+        g_slice_free1(acb->aiocb_info->aiocb_size, acb);
+    }
 }
 
 /**************************************************************/
diff --git a/include/block/aio.h b/include/block/aio.h
index 4603c0f..2626fc7 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -35,11 +35,13 @@ struct BlockDriverAIOCB {
     BlockDriverState *bs;
     BlockDriverCompletionFunc *cb;
     void *opaque;
+    int refcnt;
 };
 
 void *qemu_aio_get(const AIOCBInfo *aiocb_info, BlockDriverState *bs,
                    BlockDriverCompletionFunc *cb, void *opaque);
 void qemu_aio_release(void *p);
+void qemu_aio_ref(void *p);
 
 typedef struct AioHandler AioHandler;
 typedef void QEMUBHFunc(void *opaque);
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 02/20] block: Add bdrv_aio_cancel_async

[Fam Zheng]

This is the async version of bdrv_aio_cancel, which doesn't block the
caller. It guarantees that the cb is called either before returning or
some time later.

bdrv_aio_cancel can base on bdrv_aio_cancel_async, later we can convert
all .io_cancel implementations to .io_cancel_async, and the aio_poll is
the common logic. In the end, .io_cancel can be dropped.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c               | 27 ++++++++++++++++++++++++++-
 include/block/aio.h   |  2 ++
 include/block/block.h |  1 +
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 78d68cb..2dfd1be 100644
--- a/block.c
+++ b/block.c
@@ -4634,7 +4634,32 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, int num_reqs)
 
 void bdrv_aio_cancel(BlockDriverAIOCB *acb)
 {
-    acb->aiocb_info->cancel(acb);
+    if (acb->aiocb_info->cancel) {
+        acb->aiocb_info->cancel(acb);
+    } else {
+        qemu_aio_ref(acb);
+        bdrv_aio_cancel_async(acb);
+        while (acb->refcnt > 1) {
+            if (acb->aiocb_info->get_aio_context) {
+                aio_poll(acb->aiocb_info->get_aio_context(acb), true);
+            } else if (acb->bs) {
+                aio_poll(bdrv_get_aio_context(acb->bs), true);
+            } else {
+                abort();
+            }
+        }
+        qemu_aio_release(acb);
+    }
+}
+
+/* Async version of aio cancel. The caller is not blocked if the acb implements
+ * cancel_async, otherwise we do nothing and let the request normally complete.
+ * In either case the completion callback must be called. */
+void bdrv_aio_cancel_async(BlockDriverAIOCB *acb)
+{
+    if (acb->aiocb_info->cancel_async) {
+        acb->aiocb_info->cancel_async(acb);
+    }
 }
 
 /**************************************************************/
diff --git a/include/block/aio.h b/include/block/aio.h
index 2626fc7..ad361e3 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -27,6 +27,8 @@ typedef void BlockDriverCompletionFunc(void *opaque, int ret);
 
 typedef struct AIOCBInfo {
     void (*cancel)(BlockDriverAIOCB *acb);
+    void (*cancel_async)(BlockDriverAIOCB *acb);
+    AioContext *(*get_aio_context)(BlockDriverAIOCB *acb);
     size_t aiocb_size;
 } AIOCBInfo;
 
diff --git a/include/block/block.h b/include/block/block.h
index 8f4ad16..35a2448 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -336,6 +336,7 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
                                    int64_t sector_num, int nb_sectors,
                                    BlockDriverCompletionFunc *cb, void *opaque);
 void bdrv_aio_cancel(BlockDriverAIOCB *acb);
+void bdrv_aio_cancel_async(BlockDriverAIOCB *acb);
 
 typedef struct BlockRequest {
     /* Fields to be filled by multiwrite caller */
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 03/20] block: Drop bdrv_em_co_aiocb_info.cancel

[Fam Zheng]

Also drop the now unused ->done pointer.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/block.c b/block.c
index 2dfd1be..4aa1bd7 100644
--- a/block.c
+++ b/block.c
@@ -4757,22 +4757,8 @@ typedef struct BlockDriverAIOCBCoroutine {
     QEMUBH* bh;
 } BlockDriverAIOCBCoroutine;
 
-static void bdrv_aio_co_cancel_em(BlockDriverAIOCB *blockacb)
-{
-    AioContext *aio_context = bdrv_get_aio_context(blockacb->bs);
-    BlockDriverAIOCBCoroutine *acb =
-        container_of(blockacb, BlockDriverAIOCBCoroutine, common);
-    bool done = false;
-
-    acb->done = &done;
-    while (!done) {
-        aio_poll(aio_context, true);
-    }
-}
-
 static const AIOCBInfo bdrv_em_co_aiocb_info = {
     .aiocb_size         = sizeof(BlockDriverAIOCBCoroutine),
-    .cancel             = bdrv_aio_co_cancel_em,
 };
 
 static void bdrv_co_em_bh(void *opaque)
@@ -4781,10 +4767,6 @@ static void bdrv_co_em_bh(void *opaque)
 
     acb->common.cb(acb->common.opaque, acb->req.error);
 
-    if (acb->done) {
-        *acb->done = true;
-    }
-
     qemu_bh_delete(acb->bh);
     qemu_aio_release(acb);
 }
@@ -4825,7 +4807,6 @@ static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
     acb->req.qiov = qiov;
     acb->req.flags = flags;
     acb->is_write = is_write;
-    acb->done = NULL;
 
     co = qemu_coroutine_create(bdrv_co_do_rw);
     qemu_coroutine_enter(co, acb);
@@ -4852,7 +4833,6 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
     BlockDriverAIOCBCoroutine *acb;
 
     acb = qemu_aio_get(&bdrv_em_co_aiocb_info, bs, cb, opaque);
-    acb->done = NULL;
 
     co = qemu_coroutine_create(bdrv_aio_flush_co_entry);
     qemu_coroutine_enter(co, acb);
@@ -4882,7 +4862,6 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
     acb = qemu_aio_get(&bdrv_em_co_aiocb_info, bs, cb, opaque);
     acb->req.sector = sector_num;
     acb->req.nb_sectors = nb_sectors;
-    acb->done = NULL;
     co = qemu_coroutine_create(bdrv_aio_discard_co_entry);
     qemu_coroutine_enter(co, acb);
 
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 04/20] block: Convert bdrv_em_aiocb_info.cancel to .cancel_async

[Fam Zheng]

All the difference is that the old .cancel doesn't call cb, but
.cancel_async does.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 4aa1bd7..b7253af 100644
--- a/block.c
+++ b/block.c
@@ -4679,6 +4679,9 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
 {
     BlockDriverAIOCBSync *acb =
         container_of(blockacb, BlockDriverAIOCBSync, common);
+
+    acb->ret = -ECANCELED;
+    acb->common.cb(acb->common.opaque, acb->ret);
     qemu_bh_delete(acb->bh);
     acb->bh = NULL;
     qemu_aio_release(acb);
@@ -4686,7 +4689,7 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
 
 static const AIOCBInfo bdrv_em_aiocb_info = {
     .aiocb_size         = sizeof(BlockDriverAIOCBSync),
-    .cancel             = bdrv_aio_cancel_em,
+    .cancel_async       = bdrv_aio_cancel_em,
 };
 
 static void bdrv_aio_bh_cb(void *opaque)
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 05/20] thread-pool: Convert thread_pool_aiocb_info.cancel to cancel_async

[Fam Zheng]

The .cancel_async has the same the first half with .cancel: try to steal
the request if not submitted yet. In this case set the elem to
THREAD_DONE status and ret to -ECANCELED, which means
thread_pool_completion_bh will call the cb with -ECANCELED.

If the request is already submitted, do nothing, as we know the normal
completion will happen in the future.

Testing code update:

Before, done_cb is only called if the request is already submitted by
thread pool. Now done_cb is always called, even before it is submitted,
because we emulate bdrv_aio_cancel with bdrv_aio_cancel_async. So also
update the test criteria accordingly.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/test-thread-pool.c | 34 ++++++++++++++++++++++++++--------
 thread-pool.c            | 32 ++++++++++++++------------------
 2 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index f40b7fc..ed2b25b 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -33,7 +33,7 @@ static int long_cb(void *opaque)
 static void done_cb(void *opaque, int ret)
 {
     WorkerTestData *data = opaque;
-    g_assert_cmpint(data->ret, ==, -EINPROGRESS);
+    g_assert(data->ret == -EINPROGRESS || data->ret == -ECANCELED);
     data->ret = ret;
     data->aiocb = NULL;
 
@@ -132,7 +132,7 @@ static void test_submit_many(void)
     }
 }
 
-static void test_cancel(void)
+static void do_test_cancel(bool sync)
 {
     WorkerTestData data[100];
     int num_canceled;
@@ -170,18 +170,25 @@ static void test_cancel(void)
     for (i = 0; i < 100; i++) {
         if (atomic_cmpxchg(&data[i].n, 0, 3) == 0) {
             data[i].ret = -ECANCELED;
-            bdrv_aio_cancel(data[i].aiocb);
-            active--;
+            if (sync) {
+                bdrv_aio_cancel(data[i].aiocb);
+            } else {
+                bdrv_aio_cancel_async(data[i].aiocb);
+            }
             num_canceled++;
         }
     }
     g_assert_cmpint(active, >, 0);
     g_assert_cmpint(num_canceled, <, 100);
 
-    /* Canceling the others will be a blocking operation.  */
     for (i = 0; i < 100; i++) {
         if (data[i].aiocb && data[i].n != 3) {
-            bdrv_aio_cancel(data[i].aiocb);
+            if (sync) {
+                /* Canceling the others will be a blocking operation.  */
+                bdrv_aio_cancel(data[i].aiocb);
+            } else {
+                bdrv_aio_cancel_async(data[i].aiocb);
+            }
         }
     }
 
@@ -193,15 +200,25 @@ static void test_cancel(void)
     for (i = 0; i < 100; i++) {
         if (data[i].n == 3) {
             g_assert_cmpint(data[i].ret, ==, -ECANCELED);
-            g_assert(data[i].aiocb != NULL);
+            g_assert(data[i].aiocb == NULL);
         } else {
             g_assert_cmpint(data[i].n, ==, 2);
-            g_assert_cmpint(data[i].ret, ==, 0);
+            g_assert(data[i].ret == 0 || data[i].ret == -ECANCELED);
             g_assert(data[i].aiocb == NULL);
         }
     }
 }
 
+static void test_cancel(void)
+{
+    do_test_cancel(true);
+}
+
+static void test_cancel_async(void)
+{
+    do_test_cancel(false);
+}
+
 int main(int argc, char **argv)
 {
     int ret;
@@ -217,6 +234,7 @@ int main(int argc, char **argv)
     g_test_add_func("/thread-pool/submit-co", test_submit_co);
     g_test_add_func("/thread-pool/submit-many", test_submit_many);
     g_test_add_func("/thread-pool/cancel", test_cancel);
+    g_test_add_func("/thread-pool/cancel-async", test_cancel_async);
 
     ret = g_test_run();
 
diff --git a/thread-pool.c b/thread-pool.c
index 23888dc..6afd343 100644
--- a/thread-pool.c
+++ b/thread-pool.c
@@ -32,7 +32,6 @@ enum ThreadState {
     THREAD_QUEUED,
     THREAD_ACTIVE,
     THREAD_DONE,
-    THREAD_CANCELED,
 };
 
 struct ThreadPoolElement {
@@ -59,7 +58,6 @@ struct ThreadPool {
     AioContext *ctx;
     QEMUBH *completion_bh;
     QemuMutex lock;
-    QemuCond check_cancel;
     QemuCond worker_stopped;
     QemuSemaphore sem;
     int max_threads;
@@ -74,7 +72,6 @@ struct ThreadPool {
     int idle_threads;
     int new_threads;     /* backlog of threads we need to create */
     int pending_threads; /* threads created but not running yet */
-    int pending_cancellations; /* whether we need a cond_broadcast */
     bool stopping;
 };
 
@@ -114,9 +111,6 @@ static void *worker_thread(void *opaque)
         req->state = THREAD_DONE;
 
         qemu_mutex_lock(&pool->lock);
-        if (pool->pending_cancellations) {
-            qemu_cond_broadcast(&pool->check_cancel);
-        }
 
         qemu_bh_schedule(pool->completion_bh);
     }
@@ -174,7 +168,7 @@ static void thread_pool_completion_bh(void *opaque)
 
 restart:
     QLIST_FOREACH_SAFE(elem, &pool->head, all, next) {
-        if (elem->state != THREAD_CANCELED && elem->state != THREAD_DONE) {
+        if (elem->state != THREAD_DONE) {
             continue;
         }
         if (elem->state == THREAD_DONE) {
@@ -218,22 +212,26 @@ static void thread_pool_cancel(BlockDriverAIOCB *acb)
          */
         qemu_sem_timedwait(&pool->sem, 0) == 0) {
         QTAILQ_REMOVE(&pool->request_list, elem, reqs);
-        elem->state = THREAD_CANCELED;
         qemu_bh_schedule(pool->completion_bh);
-    } else {
-        pool->pending_cancellations++;
-        while (elem->state != THREAD_CANCELED && elem->state != THREAD_DONE) {
-            qemu_cond_wait(&pool->check_cancel, &pool->lock);
-        }
-        pool->pending_cancellations--;
+
+        elem->state = THREAD_DONE;
+        elem->ret = -ECANCELED;
     }
+
     qemu_mutex_unlock(&pool->lock);
-    thread_pool_completion_bh(pool);
+}
+
+static AioContext *thread_pool_get_aio_context(BlockDriverAIOCB *acb)
+{
+    ThreadPoolElement *elem = (ThreadPoolElement *)acb;
+    ThreadPool *pool = elem->pool;
+    return pool->ctx;
 }
 
 static const AIOCBInfo thread_pool_aiocb_info = {
     .aiocb_size         = sizeof(ThreadPoolElement),
-    .cancel             = thread_pool_cancel,
+    .cancel_async       = thread_pool_cancel,
+    .get_aio_context    = thread_pool_get_aio_context,
 };
 
 BlockDriverAIOCB *thread_pool_submit_aio(ThreadPool *pool,
@@ -300,7 +298,6 @@ static void thread_pool_init_one(ThreadPool *pool, AioContext *ctx)
     pool->ctx = ctx;
     pool->completion_bh = aio_bh_new(ctx, thread_pool_completion_bh, pool);
     qemu_mutex_init(&pool->lock);
-    qemu_cond_init(&pool->check_cancel);
     qemu_cond_init(&pool->worker_stopped);
     qemu_sem_init(&pool->sem, 0);
     pool->max_threads = 64;
@@ -343,7 +340,6 @@ void thread_pool_free(ThreadPool *pool)
 
     qemu_bh_delete(pool->completion_bh);
     qemu_sem_destroy(&pool->sem);
-    qemu_cond_destroy(&pool->check_cancel);
     qemu_cond_destroy(&pool->worker_stopped);
     qemu_mutex_destroy(&pool->lock);
     g_free(pool);
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 06/20] linux-aio: Convert laio_aiocb_info.cancel to .cancel_async

[Fam Zheng]

Just call io_cancel (2), if it fails, it means the request is not
canceled, so the event loop will eventually call
qemu_laio_process_completion.

In qemu_laio_process_completion, change to call the cb unconditionally.
It is required by bdrv_aio_cancel_async.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/linux-aio.c | 30 ++++++++----------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/block/linux-aio.c b/block/linux-aio.c
index 9aca758..b67f56c 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -85,9 +85,8 @@ static void qemu_laio_process_completion(struct qemu_laio_state *s,
                 ret = -EINVAL;
             }
         }
-
-        laiocb->common.cb(laiocb->common.opaque, ret);
     }
+    laiocb->common.cb(laiocb->common.opaque, ret);
 
     qemu_aio_release(laiocb);
 }
@@ -153,35 +152,22 @@ static void laio_cancel(BlockDriverAIOCB *blockacb)
     struct io_event event;
     int ret;
 
-    if (laiocb->ret != -EINPROGRESS)
+    if (laiocb->ret != -EINPROGRESS) {
         return;
-
-    /*
-     * Note that as of Linux 2.6.31 neither the block device code nor any
-     * filesystem implements cancellation of AIO request.
-     * Thus the polling loop below is the normal code path.
-     */
+    }
     ret = io_cancel(laiocb->ctx->ctx, &laiocb->iocb, &event);
-    if (ret == 0) {
-        laiocb->ret = -ECANCELED;
+    laiocb->ret = -ECANCELED;
+    if (ret != 0) {
+        /* iocb is not cancelled, cb will be called by the event loop later */
         return;
     }
 
-    /*
-     * We have to wait for the iocb to finish.
-     *
-     * The only way to get the iocb status update is by polling the io context.
-     * We might be able to do this slightly more optimal by removing the
-     * O_NONBLOCK flag.
-     */
-    while (laiocb->ret == -EINPROGRESS) {
-        qemu_laio_completion_cb(&laiocb->ctx->e);
-    }
+    laiocb->common.cb(laiocb->common.opaque, laiocb->ret);
 }
 
 static const AIOCBInfo laio_aiocb_info = {
     .aiocb_size         = sizeof(struct qemu_laiocb),
-    .cancel             = laio_cancel,
+    .cancel_async       = laio_cancel,
 };
 
 static void ioq_init(LaioQueue *io_q)
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 07/20] dma: Check iov pointer before unmap memory

[Fam Zheng]

Not all the iov elements are always valid.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 dma-helpers.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/dma-helpers.c b/dma-helpers.c
index 499b52b..3655d88 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -105,6 +105,9 @@ static void dma_bdrv_unmap(DMAAIOCB *dbs)
     int i;
 
     for (i = 0; i < dbs->iov.niov; ++i) {
+        if (!(dbs->iov.iov[i].iov_base && dbs->iov.iov[i].iov_len)) {
+            break;
+        }
         dma_memory_unmap(dbs->sg->as, dbs->iov.iov[i].iov_base,
                          dbs->iov.iov[i].iov_len, dbs->dir,
                          dbs->iov.iov[i].iov_len);
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 08/20] dma: Convert dma_aiocb_info.cancel to .cancel_async

[Fam Zheng]

Just forward the request to bdrv_aio_cancel_async.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 dma-helpers.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/dma-helpers.c b/dma-helpers.c
index 3655d88..8163940 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -73,7 +73,6 @@ typedef struct {
     QEMUSGList *sg;
     uint64_t sector_num;
     DMADirection dir;
-    bool in_cancel;
     int sg_cur_index;
     dma_addr_t sg_cur_byte;
     QEMUIOVector iov;
@@ -128,12 +127,7 @@ static void dma_complete(DMAAIOCB *dbs, int ret)
         qemu_bh_delete(dbs->bh);
         dbs->bh = NULL;
     }
-    if (!dbs->in_cancel) {
-        /* Requests may complete while dma_aio_cancel is in progress.  In
-         * this case, the AIOCB should not be released because it is still
-         * referenced by dma_aio_cancel.  */
-        qemu_aio_release(dbs);
-    }
+    qemu_aio_release(dbs);
 }
 
 static void dma_bdrv_cb(void *opaque, int ret)
@@ -189,19 +183,14 @@ static void dma_aio_cancel(BlockDriverAIOCB *acb)
     trace_dma_aio_cancel(dbs);
 
     if (dbs->acb) {
-        BlockDriverAIOCB *acb = dbs->acb;
-        dbs->acb = NULL;
-        dbs->in_cancel = true;
-        bdrv_aio_cancel(acb);
-        dbs->in_cancel = false;
+        bdrv_aio_cancel_async(dbs->acb);
     }
-    dbs->common.cb = NULL;
-    dma_complete(dbs, 0);
 }
 
+
 static const AIOCBInfo dma_aiocb_info = {
     .aiocb_size         = sizeof(DMAAIOCB),
-    .cancel             = dma_aio_cancel,
+    .cancel_async       = dma_aio_cancel,
 };
 
 BlockDriverAIOCB *dma_bdrv_io(
@@ -220,7 +209,6 @@ BlockDriverAIOCB *dma_bdrv_io(
     dbs->sg_cur_index = 0;
     dbs->sg_cur_byte = 0;
     dbs->dir = dir;
-    dbs->in_cancel = false;
     dbs->io_func = io_func;
     dbs->bh = NULL;
     qemu_iovec_init(&dbs->iov, sg->nsg);
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 09/20] iscsi: Convert iscsi_aiocb_info.cancel to .cancel_async

[Fam Zheng]

Also drop the unused field "canceled".

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/iscsi.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 3e19202..a0aca5f 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -88,7 +88,6 @@ typedef struct IscsiAIOCB {
     struct scsi_task *task;
     uint8_t *buf;
     int status;
-    int canceled;
     int64_t sector_num;
     int nb_sectors;
 #ifdef __linux__
@@ -120,9 +119,7 @@ iscsi_bh_cb(void *p)
     g_free(acb->buf);
     acb->buf = NULL;
 
-    if (acb->canceled == 0) {
-        acb->common.cb(acb->common.opaque, acb->status);
-    }
+    acb->common.cb(acb->common.opaque, acb->status);
 
     if (acb->task != NULL) {
         scsi_free_scsi_task(acb->task);
@@ -240,20 +237,15 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
         return;
     }
 
-    acb->canceled = 1;
-
     /* send a task mgmt call to the target to cancel the task on the target */
     iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
                                      iscsi_abort_task_cb, acb);
 
-    while (acb->status == -EINPROGRESS) {
-        aio_poll(iscsilun->aio_context, true);
-    }
 }
 
 static const AIOCBInfo iscsi_aiocb_info = {
     .aiocb_size         = sizeof(IscsiAIOCB),
-    .cancel             = iscsi_aio_cancel,
+    .cancel_async       = iscsi_aio_cancel,
 };
 
 
@@ -638,10 +630,6 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
     g_free(acb->buf);
     acb->buf = NULL;
 
-    if (acb->canceled != 0) {
-        return;
-    }
-
     acb->status = 0;
     if (status < 0) {
         error_report("Failed to ioctl(SG_IO) to iSCSI lun. %s",
@@ -683,7 +671,6 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
     acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque);
 
     acb->iscsilun = iscsilun;
-    acb->canceled    = 0;
     acb->bh          = NULL;
     acb->status      = -EINPROGRESS;
     acb->buf         = NULL;
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 10/20] archipelago: Drop archipelago_aiocb_info.cancel

[Fam Zheng]

The cancelled flag is no longer useful. Later the request will complete
as before, and cb will be called.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/archipelago.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/block/archipelago.c b/block/archipelago.c
index 34f72dc..c0f288a 100644
--- a/block/archipelago.c
+++ b/block/archipelago.c
@@ -91,7 +91,6 @@ typedef struct ArchipelagoAIOCB {
     struct BDRVArchipelagoState *s;
     QEMUIOVector *qiov;
     ARCHIPCmd cmd;
-    bool cancelled;
     int status;
     int64_t size;
     int64_t ret;
@@ -317,9 +316,7 @@ static void qemu_archipelago_complete_aio(void *opaque)
     aio_cb->common.cb(aio_cb->common.opaque, aio_cb->ret);
     aio_cb->status = 0;
 
-    if (!aio_cb->cancelled) {
-        qemu_aio_release(aio_cb);
-    }
+    qemu_aio_release(aio_cb);
     g_free(reqdata);
 }
 
@@ -723,19 +720,8 @@ static int qemu_archipelago_create(const char *filename,
     return ret;
 }
 
-static void qemu_archipelago_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    ArchipelagoAIOCB *aio_cb = (ArchipelagoAIOCB *) blockacb;
-    aio_cb->cancelled = true;
-    while (aio_cb->status == -EINPROGRESS) {
-        aio_poll(bdrv_get_aio_context(aio_cb->common.bs), true);
-    }
-    qemu_aio_release(aio_cb);
-}
-
 static const AIOCBInfo archipelago_aiocb_info = {
     .aiocb_size = sizeof(ArchipelagoAIOCB),
-    .cancel = qemu_archipelago_aio_cancel,
 };
 
 static int archipelago_submit_request(BDRVArchipelagoState *s,
@@ -918,7 +904,6 @@ static BlockDriverAIOCB *qemu_archipelago_aio_rw(BlockDriverState *bs,
 
     aio_cb->ret = 0;
     aio_cb->s = s;
-    aio_cb->cancelled = false;
     aio_cb->status = -EINPROGRESS;
 
     off = sector_num * BDRV_SECTOR_SIZE;
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 11/20] blkdebug: Convert blkdebug_aiocb_info.cancel to .cancel_async

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/blkdebug.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 69b330e..9f23530 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -55,8 +55,8 @@ typedef struct BlkdebugSuspendedReq {
 static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb);
 
 static const AIOCBInfo blkdebug_aiocb_info = {
-    .aiocb_size = sizeof(BlkdebugAIOCB),
-    .cancel     = blkdebug_aio_cancel,
+    .aiocb_size    = sizeof(BlkdebugAIOCB),
+    .cancel_async  = blkdebug_aio_cancel,
 };
 
 enum {
@@ -453,6 +453,8 @@ static void error_callback_bh(void *opaque)
 static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb)
 {
     BlkdebugAIOCB *acb = container_of(blockacb, BlkdebugAIOCB, common);
+    acb->ret = -ECANCELED;
+    acb->common.cb(acb->common.opaque, acb->ret);
     if (acb->bh) {
         qemu_bh_delete(acb->bh);
         acb->bh = NULL;
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 12/20] blkverify: Drop blkverify_aiocb_info.cancel

[Fam Zheng]

Also the finished pointer is not used any more.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/blkverify.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/block/blkverify.c b/block/blkverify.c
index 163064c..460393f 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -29,7 +29,6 @@ struct BlkverifyAIOCB {
 
     int ret;                    /* first completed request's result */
     unsigned int done;          /* completion counter */
-    bool *finished;             /* completion signal for cancel */
 
     QEMUIOVector *qiov;         /* user I/O vector */
     QEMUIOVector raw_qiov;      /* cloned I/O vector for raw file */
@@ -38,22 +37,8 @@ struct BlkverifyAIOCB {
     void (*verify)(BlkverifyAIOCB *acb);
 };
 
-static void blkverify_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    BlkverifyAIOCB *acb = (BlkverifyAIOCB *)blockacb;
-    AioContext *aio_context = bdrv_get_aio_context(blockacb->bs);
-    bool finished = false;
-
-    /* Wait until request completes, invokes its callback, and frees itself */
-    acb->finished = &finished;
-    while (!finished) {
-        aio_poll(aio_context, true);
-    }
-}
-
 static const AIOCBInfo blkverify_aiocb_info = {
     .aiocb_size         = sizeof(BlkverifyAIOCB),
-    .cancel             = blkverify_aio_cancel,
 };
 
 static void GCC_FMT_ATTR(2, 3) blkverify_err(BlkverifyAIOCB *acb,
@@ -194,7 +179,6 @@ static BlkverifyAIOCB *blkverify_aio_get(BlockDriverState *bs, bool is_write,
     acb->qiov = qiov;
     acb->buf = NULL;
     acb->verify = NULL;
-    acb->finished = NULL;
     return acb;
 }
 
@@ -208,9 +192,6 @@ static void blkverify_aio_bh(void *opaque)
         qemu_vfree(acb->buf);
     }
     acb->common.cb(acb->common.opaque, acb->ret);
-    if (acb->finished) {
-        *acb->finished = true;
-    }
     qemu_aio_release(acb);
 }
 
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 13/20] curl: Drop curl_aiocb_info.cancel

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 0258339..ed64b18 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -613,14 +613,8 @@ out_noclean:
     return -EINVAL;
 }
 
-static void curl_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    // Do we have to implement canceling? Seems to work without...
-}
-
 static const AIOCBInfo curl_aiocb_info = {
     .aiocb_size         = sizeof(CURLAIOCB),
-    .cancel             = curl_aio_cancel,
 };
 
 
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 14/20] qed: Drop qed_aiocb_info.cancel

[Fam Zheng]

Also drop the now unused ->finished field.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/qed.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index ba395af..07cdb47 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -18,22 +18,8 @@
 #include "qapi/qmp/qerror.h"
 #include "migration/migration.h"
 
-static void qed_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    QEDAIOCB *acb = (QEDAIOCB *)blockacb;
-    AioContext *aio_context = bdrv_get_aio_context(blockacb->bs);
-    bool finished = false;
-
-    /* Wait for the request to finish */
-    acb->finished = &finished;
-    while (!finished) {
-        aio_poll(aio_context, true);
-    }
-}
-
 static const AIOCBInfo qed_aiocb_info = {
     .aiocb_size         = sizeof(QEDAIOCB),
-    .cancel             = qed_aio_cancel,
 };
 
 static int bdrv_qed_probe(const uint8_t *buf, int buf_size,
@@ -918,18 +904,12 @@ static void qed_aio_complete_bh(void *opaque)
     BlockDriverCompletionFunc *cb = acb->common.cb;
     void *user_opaque = acb->common.opaque;
     int ret = acb->bh_ret;
-    bool *finished = acb->finished;
 
     qemu_bh_delete(acb->bh);
     qemu_aio_release(acb);
 
     /* Invoke callback */
     cb(user_opaque, ret);
-
-    /* Signal cancel completion */
-    if (finished) {
-        *finished = true;
-    }
 }
 
 static void qed_aio_complete(QEDAIOCB *acb, int ret)
@@ -1396,7 +1376,6 @@ static BlockDriverAIOCB *qed_aio_setup(BlockDriverState *bs,
                         opaque, flags);
 
     acb->flags = flags;
-    acb->finished = NULL;
     acb->qiov = qiov;
     acb->qiov_offset = 0;
     acb->cur_pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE;
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 15/20] quorum: Convert quorum_aiocb_info.cancel to .cancel_async

[Fam Zheng]

Before, we cancel all the child requests with bdrv_aio_cancel, then free
the acb..

Now we just kick off asynchronous cancellation of child requests and
return, we know quorum_aio_cb will be called later, so in the end
quorum_aio_finalize will take care of calling the caller's cb.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/quorum.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index 093382e..0951d52 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -138,16 +138,13 @@ static void quorum_aio_cancel(BlockDriverAIOCB *blockacb)
 
     /* cancel all callbacks */
     for (i = 0; i < s->num_children; i++) {
-        bdrv_aio_cancel(acb->qcrs[i].aiocb);
+        bdrv_aio_cancel_async(acb->qcrs[i].aiocb);
     }
-
-    g_free(acb->qcrs);
-    qemu_aio_release(acb);
 }
 
 static AIOCBInfo quorum_aiocb_info = {
     .aiocb_size         = sizeof(QuorumAIOCB),
-    .cancel             = quorum_aio_cancel,
+    .cancel_async       = quorum_aio_cancel,
 };
 
 static void quorum_aio_finalize(QuorumAIOCB *acb)
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 16/20] rbd: Drop rbd_aiocb_info.cancel

[Fam Zheng]

And also drop the now unused "cancelled" field.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/rbd.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index ea969e7..349d465 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -77,7 +77,6 @@ typedef struct RBDAIOCB {
     int64_t sector_num;
     int error;
     struct BDRVRBDState *s;
-    int cancelled;
     int status;
 } RBDAIOCB;
 
@@ -407,9 +406,7 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb)
     acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret));
     acb->status = 0;
 
-    if (!acb->cancelled) {
-        qemu_aio_release(acb);
-    }
+    qemu_aio_release(acb);
 }
 
 /* TODO Convert to fine grained options */
@@ -538,25 +535,8 @@ static void qemu_rbd_close(BlockDriverState *bs)
     rados_shutdown(s->cluster);
 }
 
-/*
- * Cancel aio. Since we don't reference acb in a non qemu threads,
- * it is safe to access it here.
- */
-static void qemu_rbd_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    RBDAIOCB *acb = (RBDAIOCB *) blockacb;
-    acb->cancelled = 1;
-
-    while (acb->status == -EINPROGRESS) {
-        aio_poll(bdrv_get_aio_context(acb->common.bs), true);
-    }
-
-    qemu_aio_release(acb);
-}
-
 static const AIOCBInfo rbd_aiocb_info = {
     .aiocb_size = sizeof(RBDAIOCB),
-    .cancel = qemu_rbd_aio_cancel,
 };
 
 static void rbd_finish_bh(void *opaque)
@@ -639,7 +619,6 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,
     acb->ret = 0;
     acb->error = 0;
     acb->s = s;
-    acb->cancelled = 0;
     acb->bh = NULL;
     acb->status = -EINPROGRESS;
 
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 17/20] sheepdog: Convert sd_aiocb_info.cancel to .cancel_async

[Fam Zheng]

Also drop the now unused SheepdogAIOCB.finished field. Note that this
aio is internal to sheepdog driver and has NULL cb and opaque, and
should be unused at all.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/sheepdog.c | 46 +++++++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index f91afc3..a04ac4e 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -315,7 +315,6 @@ struct SheepdogAIOCB {
     void (*aio_done_func)(SheepdogAIOCB *);
 
     bool cancelable;
-    bool *finished;
     int nr_pending;
 };
 
@@ -446,9 +445,6 @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
 static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
 {
     qemu_coroutine_enter(acb->coroutine, NULL);
-    if (acb->finished) {
-        *acb->finished = true;
-    }
     qemu_aio_release(acb);
 }
 
@@ -482,36 +478,33 @@ static void sd_aio_cancel(BlockDriverAIOCB *blockacb)
     SheepdogAIOCB *acb = (SheepdogAIOCB *)blockacb;
     BDRVSheepdogState *s = acb->common.bs->opaque;
     AIOReq *aioreq, *next;
-    bool finished = false;
-
-    acb->finished = &finished;
-    while (!finished) {
-        if (sd_acb_cancelable(acb)) {
-            /* Remove outstanding requests from pending and failed queues.  */
-            QLIST_FOREACH_SAFE(aioreq, &s->pending_aio_head, aio_siblings,
-                               next) {
-                if (aioreq->aiocb == acb) {
-                    free_aio_req(s, aioreq);
-                }
+
+    if (sd_acb_cancelable(acb)) {
+        /* Remove outstanding requests from pending and failed queues.  */
+        QLIST_FOREACH_SAFE(aioreq, &s->pending_aio_head, aio_siblings,
+                           next) {
+            if (aioreq->aiocb == acb) {
+                free_aio_req(s, aioreq);
             }
-            QLIST_FOREACH_SAFE(aioreq, &s->failed_aio_head, aio_siblings,
-                               next) {
-                if (aioreq->aiocb == acb) {
-                    free_aio_req(s, aioreq);
-                }
+        }
+        QLIST_FOREACH_SAFE(aioreq, &s->failed_aio_head, aio_siblings,
+                           next) {
+            if (aioreq->aiocb == acb) {
+                free_aio_req(s, aioreq);
             }
+        }
 
-            assert(acb->nr_pending == 0);
-            sd_finish_aiocb(acb);
-            return;
+        assert(acb->nr_pending == 0);
+        if (acb->common.cb) {
+            acb->common.cb(acb->common.opaque, -ECANCELED);
         }
-        aio_poll(s->aio_context, true);
+        sd_finish_aiocb(acb);
     }
 }
 
 static const AIOCBInfo sd_aiocb_info = {
-    .aiocb_size = sizeof(SheepdogAIOCB),
-    .cancel = sd_aio_cancel,
+    .aiocb_size     = sizeof(SheepdogAIOCB),
+    .cancel_async   = sd_aio_cancel,
 };
 
 static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
@@ -528,7 +521,6 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
 
     acb->aio_done_func = NULL;
     acb->cancelable = true;
-    acb->finished = NULL;
     acb->coroutine = qemu_coroutine_self();
     acb->ret = 0;
     acb->nr_pending = 0;
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 18/20] win32-aio: Drop win32_aiocb_info.cancel

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/win32-aio.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/block/win32-aio.c b/block/win32-aio.c
index 5030e32..eed86f7 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -106,22 +106,8 @@ static void win32_aio_completion_cb(EventNotifier *e)
     }
 }
 
-static void win32_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    QEMUWin32AIOCB *waiocb = (QEMUWin32AIOCB *)blockacb;
-
-    /*
-     * CancelIoEx is only supported in Vista and newer.  For now, just
-     * wait for completion.
-     */
-    while (!HasOverlappedIoCompleted(&waiocb->ov)) {
-        aio_poll(bdrv_get_aio_context(blockacb->bs), true);
-    }
-}
-
 static const AIOCBInfo win32_aiocb_info = {
     .aiocb_size         = sizeof(QEMUWin32AIOCB),
-    .cancel             = win32_aio_cancel,
 };
 
 BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 19/20] ide: Convert trim_aiocb_info.cancel to .cancel_async

[Fam Zheng]

We know that either bh is scheduled or ide_issue_trim_cb will be called
again, so we just set i, j and ret to the right values. In both cases,
ide_trim_bh_cb will be called.

Also forward the cancellation to the iocb->aiocb which we get from
bdrv_aio_discard.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/ide/core.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index b48127f..edc86c3 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -334,23 +334,21 @@ static void trim_aio_cancel(BlockDriverAIOCB *acb)
 {
     TrimAIOCB *iocb = container_of(acb, TrimAIOCB, common);
 
-    /* Exit the loop in case bdrv_aio_cancel calls ide_issue_trim_cb again.  */
+    /* Exit the loop so ide_issue_trim_cb will not continue  */
     iocb->j = iocb->qiov->niov - 1;
     iocb->i = (iocb->qiov->iov[iocb->j].iov_len / 8) - 1;
 
-    /* Tell ide_issue_trim_cb not to trigger the completion, too.  */
-    qemu_bh_delete(iocb->bh);
-    iocb->bh = NULL;
+    iocb->ret = -ECANCELED;
 
     if (iocb->aiocb) {
-        bdrv_aio_cancel(iocb->aiocb);
+        bdrv_aio_cancel_async(iocb->aiocb);
+        iocb->aiocb = NULL;
     }
-    qemu_aio_release(iocb);
 }
 
 static const AIOCBInfo trim_aiocb_info = {
     .aiocb_size         = sizeof(TrimAIOCB),
-    .cancel             = trim_aio_cancel,
+    .cancel_async       = trim_aio_cancel,
 };
 
 static void ide_trim_bh_cb(void *opaque)
-- 
2.1.0.27.g96db324

[Qemu-devel] [PATCH v4 20/20] block: Drop AIOCBInfo.cancel

[Fam Zheng]

Now that all the implementations are converted to asynchronous version
and we can emulate synchronous cancellation with it. Let's drop the
unused member.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c             | 24 ++++++++++--------------
 include/block/aio.h |  1 -
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index b7253af..af7f271 100644
--- a/block.c
+++ b/block.c
@@ -4634,22 +4634,18 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, int num_reqs)
 
 void bdrv_aio_cancel(BlockDriverAIOCB *acb)
 {
-    if (acb->aiocb_info->cancel) {
-        acb->aiocb_info->cancel(acb);
-    } else {
-        qemu_aio_ref(acb);
-        bdrv_aio_cancel_async(acb);
-        while (acb->refcnt > 1) {
-            if (acb->aiocb_info->get_aio_context) {
-                aio_poll(acb->aiocb_info->get_aio_context(acb), true);
-            } else if (acb->bs) {
-                aio_poll(bdrv_get_aio_context(acb->bs), true);
-            } else {
-                abort();
-            }
+    qemu_aio_ref(acb);
+    bdrv_aio_cancel_async(acb);
+    while (acb->refcnt > 1) {
+        if (acb->aiocb_info->get_aio_context) {
+            aio_poll(acb->aiocb_info->get_aio_context(acb), true);
+        } else if (acb->bs) {
+            aio_poll(bdrv_get_aio_context(acb->bs), true);
+        } else {
+            abort();
         }
-        qemu_aio_release(acb);
     }
+    qemu_aio_release(acb);
 }
 
 /* Async version of aio cancel. The caller is not blocked if the acb implements
diff --git a/include/block/aio.h b/include/block/aio.h
index ad361e3..f2d0582 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -26,7 +26,6 @@ typedef struct BlockDriverAIOCB BlockDriverAIOCB;
 typedef void BlockDriverCompletionFunc(void *opaque, int ret);
 
 typedef struct AIOCBInfo {
-    void (*cancel)(BlockDriverAIOCB *acb);
     void (*cancel_async)(BlockDriverAIOCB *acb);
     AioContext *(*get_aio_context)(BlockDriverAIOCB *acb);
     size_t aiocb_size;
-- 
2.1.0.27.g96db324

Re: [Qemu-devel] [PATCH v4 04/20] block: Convert bdrv_em_aiocb_info.cancel to .cancel_async

[Paolo Bonzini]

Il 03/09/2014 13:23, Fam Zheng ha scritto:
> All the difference is that the old .cancel doesn't call cb, but
> .cancel_async does.

This requires auditing all callbacks though, doesn't it?  Or at least
adding an

    if (ret == -ECANCELED) {
        return;
    }

at the beginning.

Paolo

> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 4aa1bd7..b7253af 100644
> --- a/block.c
> +++ b/block.c
> @@ -4679,6 +4679,9 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
>  {
>      BlockDriverAIOCBSync *acb =
>          container_of(blockacb, BlockDriverAIOCBSync, common);
> +
> +    acb->ret = -ECANCELED;
> +    acb->common.cb(acb->common.opaque, acb->ret);
>      qemu_bh_delete(acb->bh);
>      acb->bh = NULL;
>      qemu_aio_release(acb);
> @@ -4686,7 +4689,7 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
>  
>  static const AIOCBInfo bdrv_em_aiocb_info = {
>      .aiocb_size         = sizeof(BlockDriverAIOCBSync),
> -    .cancel             = bdrv_aio_cancel_em,
> +    .cancel_async       = bdrv_aio_cancel_em,
>  };
>  
>  static void bdrv_aio_bh_cb(void *opaque)
> 

Re: [Qemu-devel] [PATCH v4 07/20] dma: Check iov pointer before unmap memory

[Paolo Bonzini]

Il 03/09/2014 13:23, Fam Zheng ha scritto:
> Not all the iov elements are always valid.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  dma-helpers.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/dma-helpers.c b/dma-helpers.c
> index 499b52b..3655d88 100644
> --- a/dma-helpers.c
> +++ b/dma-helpers.c
> @@ -105,6 +105,9 @@ static void dma_bdrv_unmap(DMAAIOCB *dbs)
>      int i;
>  
>      for (i = 0; i < dbs->iov.niov; ++i) {
> +        if (!(dbs->iov.iov[i].iov_base && dbs->iov.iov[i].iov_len)) {
> +            break;
> +        }
>          dma_memory_unmap(dbs->sg->as, dbs->iov.iov[i].iov_base,
>                           dbs->iov.iov[i].iov_len, dbs->dir,
>                           dbs->iov.iov[i].iov_len);
> 

Why is this needed by this patch series?  Also, the only addition to iov
is here in dma_bdrv_cb:

        mem = dma_memory_map(dbs->sg->as, cur_addr, &cur_len, dbs->dir);
        if (!mem)
            break;
        qemu_iovec_add(&dbs->iov, mem, cur_len);

so iov_base cannot be NULL.

If cur_len is zero after dma_memory_map returns, mem should be NULL too.

If cur_len is zero before dma_memory_map is invoked, address_space_map
(and thus dma_memory_map) will return NULL too.  However, in this case
exiting the loop is wrong.  Perhaps it's better to add an if() in
dma_bdrv_cb that checks for cur_len == 0.

Paolo

Re: [Qemu-devel] [PATCH v4 11/20] blkdebug: Convert blkdebug_aiocb_info.cancel to .cancel_async

[Paolo Bonzini]

Il 03/09/2014 13:23, Fam Zheng ha scritto:
>  static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb)
>  {
>      BlkdebugAIOCB *acb = container_of(blockacb, BlkdebugAIOCB, common);
> +    acb->ret = -ECANCELED;
> +    acb->common.cb(acb->common.opaque, acb->ret);
>      if (acb->bh) {
>          qemu_bh_delete(acb->bh);
>          acb->bh = NULL;

I think you shouldn't invoke the callback here, and instead you can just
let the bottom half do the work.

Actually, a no-op implementation of blkdebug_aio_cancel would work.

Paolo

Re: [Qemu-devel] [PATCH v4 04/20] block: Convert bdrv_em_aiocb_info.cancel to .cancel_async

[Fam Zheng]

On Wed, 09/03 13:28, Paolo Bonzini wrote:
> Il 03/09/2014 13:23, Fam Zheng ha scritto:
> > All the difference is that the old .cancel doesn't call cb, but
> > .cancel_async does.
> 
> This requires auditing all callbacks though, doesn't it?  Or at least
> adding an
> 
>     if (ret == -ECANCELED) {
>         return;
>     }
> 
> at the beginning.

You are right, this is an unexpected error code for ide at least.

Fam

> 
> Paolo
> 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  block.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/block.c b/block.c
> > index 4aa1bd7..b7253af 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -4679,6 +4679,9 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
> >  {
> >      BlockDriverAIOCBSync *acb =
> >          container_of(blockacb, BlockDriverAIOCBSync, common);
> > +
> > +    acb->ret = -ECANCELED;
> > +    acb->common.cb(acb->common.opaque, acb->ret);
> >      qemu_bh_delete(acb->bh);
> >      acb->bh = NULL;
> >      qemu_aio_release(acb);
> > @@ -4686,7 +4689,7 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
> >  
> >  static const AIOCBInfo bdrv_em_aiocb_info = {
> >      .aiocb_size         = sizeof(BlockDriverAIOCBSync),
> > -    .cancel             = bdrv_aio_cancel_em,
> > +    .cancel_async       = bdrv_aio_cancel_em,
> >  };
> >  
> >  static void bdrv_aio_bh_cb(void *opaque)
> > 
> 

Re: [Qemu-devel] [PATCH v4 04/20] block: Convert bdrv_em_aiocb_info.cancel to .cancel_async

[Paolo Bonzini]

Il 04/09/2014 03:20, Fam Zheng ha scritto:
>> > 
>> > This requires auditing all callbacks though, doesn't it?  Or at least
>> > adding an
>> > 
>> >     if (ret == -ECANCELED) {
>> >         return;
>> >     }
>> > 
>> > at the beginning.
> You are right, this is an unexpected error code for ide at least.

Yeahh, I think it's IDE only.

It is fine for SCSI which is already testing io_canceled.  It should not
matter (or it could even help) for NVME.

Paolo

Re: [Qemu-devel] [PATCH v4 01/20] block: Add refcnt in BlockDriverAIOCB

[Benoît Canet]

The Wednesday 03 Sep 2014 à 19:23:36 (+0800), Fam Zheng wrote :
> This will be useful in synchronous cancel emulation with
> bdrv_aio_cancel_async.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block.c             | 12 +++++++++++-
>  include/block/aio.h |  2 ++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index cb670fd..78d68cb 100644
> --- a/block.c
> +++ b/block.c
> @@ -4885,13 +4885,23 @@ void *qemu_aio_get(const AIOCBInfo *aiocb_info, BlockDriverState *bs,
>      acb->bs = bs;
>      acb->cb = cb;
>      acb->opaque = opaque;
> +    acb->refcnt = 1;
>      return acb;
>  }
>  
> +void qemu_aio_ref(void *p)
> +{
> +    BlockDriverAIOCB *acb = p;
> +    acb->refcnt++;
> +}


> +
>  void qemu_aio_release(void *p)

I would expect _release to change to _unref for symetry.

>  {
>      BlockDriverAIOCB *acb = p;
> -    g_slice_free1(acb->aiocb_info->aiocb_size, acb);
> +    assert(acb->refcnt > 0);
> +    if (--acb->refcnt == 0) {
> +        g_slice_free1(acb->aiocb_info->aiocb_size, acb);
> +    }
>  }
>  
>  /**************************************************************/
> diff --git a/include/block/aio.h b/include/block/aio.h
> index 4603c0f..2626fc7 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -35,11 +35,13 @@ struct BlockDriverAIOCB {
>      BlockDriverState *bs;
>      BlockDriverCompletionFunc *cb;
>      void *opaque;
> +    int refcnt;
>  };
>  
>  void *qemu_aio_get(const AIOCBInfo *aiocb_info, BlockDriverState *bs,
>                     BlockDriverCompletionFunc *cb, void *opaque);
>  void qemu_aio_release(void *p);
> +void qemu_aio_ref(void *p);
>  
>  typedef struct AioHandler AioHandler;
>  typedef void QEMUBHFunc(void *opaque);
> -- 
> 2.1.0.27.g96db324
> 

Re: [Qemu-devel] [PATCH v4 04/20] block: Convert bdrv_em_aiocb_info.cancel to .cancel_async

[Benoît Canet]

The Wednesday 03 Sep 2014 à 19:23:39 (+0800), Fam Zheng wrote :
> All the difference is that the old .cancel doesn't call cb, but
> .cancel_async does.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 4aa1bd7..b7253af 100644
> --- a/block.c
> +++ b/block.c
> @@ -4679,6 +4679,9 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
>  {
>      BlockDriverAIOCBSync *acb =
>          container_of(blockacb, BlockDriverAIOCBSync, common);
> +
> +    acb->ret = -ECANCELED;
> +    acb->common.cb(acb->common.opaque, acb->ret);
>      qemu_bh_delete(acb->bh);
>      acb->bh = NULL;
>      qemu_aio_release(acb);
> @@ -4686,7 +4689,7 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
>  
>  static const AIOCBInfo bdrv_em_aiocb_info = {
>      .aiocb_size         = sizeof(BlockDriverAIOCBSync),
> -    .cancel             = bdrv_aio_cancel_em,
> +    .cancel_async       = bdrv_aio_cancel_em,
>  };

Note from an AIO noob. Could we explain somewhere in the block layer what the _em suffix means ?

>  
>  static void bdrv_aio_bh_cb(void *opaque)
> -- 
> 2.1.0.27.g96db324
> 
> 

Re: [Qemu-devel] [PATCH v4 05/20] thread-pool: Convert thread_pool_aiocb_info.cancel to cancel_async

[Benoît Canet]

The Wednesday 03 Sep 2014 à 19:23:40 (+0800), Fam Zheng wrote :

> The .cancel_async has the same the first half with .cancel: try to steal

"The .cancel_async share the same first half with .cancel" ?

> the request if not submitted yet. In this case set the elem to
> THREAD_DONE status and ret to -ECANCELED, which means
> thread_pool_completion_bh will call the cb with -ECANCELED.
> 
> If the request is already submitted, do nothing, as we know the normal
> completion will happen in the future.
> 
> Testing code update:
> 
> Before, done_cb is only called if the request is already submitted by
> thread pool. Now done_cb is always called, even before it is submitted,
> because we emulate bdrv_aio_cancel with bdrv_aio_cancel_async. So also
> update the test criteria accordingly.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  tests/test-thread-pool.c | 34 ++++++++++++++++++++++++++--------
>  thread-pool.c            | 32 ++++++++++++++------------------
>  2 files changed, 40 insertions(+), 26 deletions(-)
> 
> diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
> index f40b7fc..ed2b25b 100644
> --- a/tests/test-thread-pool.c
> +++ b/tests/test-thread-pool.c
> @@ -33,7 +33,7 @@ static int long_cb(void *opaque)
>  static void done_cb(void *opaque, int ret)
>  {
>      WorkerTestData *data = opaque;
> -    g_assert_cmpint(data->ret, ==, -EINPROGRESS);
> +    g_assert(data->ret == -EINPROGRESS || data->ret == -ECANCELED);
>      data->ret = ret;
>      data->aiocb = NULL;
>  
> @@ -132,7 +132,7 @@ static void test_submit_many(void)
>      }
>  }
>  
> -static void test_cancel(void)
> +static void do_test_cancel(bool sync)
>  {
>      WorkerTestData data[100];
>      int num_canceled;
> @@ -170,18 +170,25 @@ static void test_cancel(void)
>      for (i = 0; i < 100; i++) {
>          if (atomic_cmpxchg(&data[i].n, 0, 3) == 0) {
>              data[i].ret = -ECANCELED;
> -            bdrv_aio_cancel(data[i].aiocb);
> -            active--;
> +            if (sync) {
> +                bdrv_aio_cancel(data[i].aiocb);
> +            } else {
> +                bdrv_aio_cancel_async(data[i].aiocb);
> +            }
>              num_canceled++;
>          }
>      }
>      g_assert_cmpint(active, >, 0);
>      g_assert_cmpint(num_canceled, <, 100);
>  
> -    /* Canceling the others will be a blocking operation.  */
>      for (i = 0; i < 100; i++) {
>          if (data[i].aiocb && data[i].n != 3) {
> -            bdrv_aio_cancel(data[i].aiocb);
> +            if (sync) {
> +                /* Canceling the others will be a blocking operation.  */
> +                bdrv_aio_cancel(data[i].aiocb);
> +            } else {
> +                bdrv_aio_cancel_async(data[i].aiocb);
> +            }
>          }
>      }
>  
> @@ -193,15 +200,25 @@ static void test_cancel(void)
>      for (i = 0; i < 100; i++) {
>          if (data[i].n == 3) {
>              g_assert_cmpint(data[i].ret, ==, -ECANCELED);
> -            g_assert(data[i].aiocb != NULL);
> +            g_assert(data[i].aiocb == NULL);
>          } else {
>              g_assert_cmpint(data[i].n, ==, 2);
> -            g_assert_cmpint(data[i].ret, ==, 0);
> +            g_assert(data[i].ret == 0 || data[i].ret == -ECANCELED);
>              g_assert(data[i].aiocb == NULL);
>          }
>      }
>  }
>  
> +static void test_cancel(void)
> +{
> +    do_test_cancel(true);
> +}
> +
> +static void test_cancel_async(void)
> +{
> +    do_test_cancel(false);
> +}
> +
>  int main(int argc, char **argv)
>  {
>      int ret;
> @@ -217,6 +234,7 @@ int main(int argc, char **argv)
>      g_test_add_func("/thread-pool/submit-co", test_submit_co);
>      g_test_add_func("/thread-pool/submit-many", test_submit_many);
>      g_test_add_func("/thread-pool/cancel", test_cancel);
> +    g_test_add_func("/thread-pool/cancel-async", test_cancel_async);
>  
>      ret = g_test_run();
>  
> diff --git a/thread-pool.c b/thread-pool.c
> index 23888dc..6afd343 100644
> --- a/thread-pool.c
> +++ b/thread-pool.c
> @@ -32,7 +32,6 @@ enum ThreadState {
>      THREAD_QUEUED,
>      THREAD_ACTIVE,
>      THREAD_DONE,
> -    THREAD_CANCELED,
>  };
>  
>  struct ThreadPoolElement {
> @@ -59,7 +58,6 @@ struct ThreadPool {
>      AioContext *ctx;
>      QEMUBH *completion_bh;
>      QemuMutex lock;
> -    QemuCond check_cancel;
>      QemuCond worker_stopped;
>      QemuSemaphore sem;
>      int max_threads;
> @@ -74,7 +72,6 @@ struct ThreadPool {
>      int idle_threads;
>      int new_threads;     /* backlog of threads we need to create */
>      int pending_threads; /* threads created but not running yet */
> -    int pending_cancellations; /* whether we need a cond_broadcast */
>      bool stopping;
>  };
>  
> @@ -114,9 +111,6 @@ static void *worker_thread(void *opaque)
>          req->state = THREAD_DONE;
>  
>          qemu_mutex_lock(&pool->lock);
> -        if (pool->pending_cancellations) {
> -            qemu_cond_broadcast(&pool->check_cancel);
> -        }
>  
>          qemu_bh_schedule(pool->completion_bh);
>      }
> @@ -174,7 +168,7 @@ static void thread_pool_completion_bh(void *opaque)
>  
>  restart:
>      QLIST_FOREACH_SAFE(elem, &pool->head, all, next) {
> -        if (elem->state != THREAD_CANCELED && elem->state != THREAD_DONE) {
> +        if (elem->state != THREAD_DONE) {
>              continue;
>          }
>          if (elem->state == THREAD_DONE) {
> @@ -218,22 +212,26 @@ static void thread_pool_cancel(BlockDriverAIOCB *acb)
>           */
>          qemu_sem_timedwait(&pool->sem, 0) == 0) {
>          QTAILQ_REMOVE(&pool->request_list, elem, reqs);
> -        elem->state = THREAD_CANCELED;
>          qemu_bh_schedule(pool->completion_bh);
> -    } else {
> -        pool->pending_cancellations++;
> -        while (elem->state != THREAD_CANCELED && elem->state != THREAD_DONE) {
> -            qemu_cond_wait(&pool->check_cancel, &pool->lock);
> -        }
> -        pool->pending_cancellations--;
> +
> +        elem->state = THREAD_DONE;
> +        elem->ret = -ECANCELED;
>      }
> +
>      qemu_mutex_unlock(&pool->lock);
> -    thread_pool_completion_bh(pool);
> +}
> +
> +static AioContext *thread_pool_get_aio_context(BlockDriverAIOCB *acb)
> +{
> +    ThreadPoolElement *elem = (ThreadPoolElement *)acb;
> +    ThreadPool *pool = elem->pool;
> +    return pool->ctx;
>  }
>  
>  static const AIOCBInfo thread_pool_aiocb_info = {
>      .aiocb_size         = sizeof(ThreadPoolElement),
> -    .cancel             = thread_pool_cancel,
> +    .cancel_async       = thread_pool_cancel,
> +    .get_aio_context    = thread_pool_get_aio_context,
>  };
>  
>  BlockDriverAIOCB *thread_pool_submit_aio(ThreadPool *pool,
> @@ -300,7 +298,6 @@ static void thread_pool_init_one(ThreadPool *pool, AioContext *ctx)
>      pool->ctx = ctx;
>      pool->completion_bh = aio_bh_new(ctx, thread_pool_completion_bh, pool);
>      qemu_mutex_init(&pool->lock);
> -    qemu_cond_init(&pool->check_cancel);
>      qemu_cond_init(&pool->worker_stopped);
>      qemu_sem_init(&pool->sem, 0);
>      pool->max_threads = 64;
> @@ -343,7 +340,6 @@ void thread_pool_free(ThreadPool *pool)
>  
>      qemu_bh_delete(pool->completion_bh);
>      qemu_sem_destroy(&pool->sem);
> -    qemu_cond_destroy(&pool->check_cancel);
>      qemu_cond_destroy(&pool->worker_stopped);
>      qemu_mutex_destroy(&pool->lock);
>      g_free(pool);
> -- 
> 2.1.0.27.g96db324
> 
> 

Re: [Qemu-devel] [PATCH v4 15/20] quorum: Convert quorum_aiocb_info.cancel to .cancel_async

[Benoît Canet]

The Wednesday 03 Sep 2014 à 19:23:50 (+0800), Fam Zheng wrote :
> Before, we cancel all the child requests with bdrv_aio_cancel, then free
> the acb..
> 
> Now we just kick off asynchronous cancellation of child requests and
> return, we know quorum_aio_cb will be called later, so in the end
> quorum_aio_finalize will take care of calling the caller's cb.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/quorum.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/block/quorum.c b/block/quorum.c
> index 093382e..0951d52 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -138,16 +138,13 @@ static void quorum_aio_cancel(BlockDriverAIOCB *blockacb)
>  
>      /* cancel all callbacks */
>      for (i = 0; i < s->num_children; i++) {
> -        bdrv_aio_cancel(acb->qcrs[i].aiocb);
> +        bdrv_aio_cancel_async(acb->qcrs[i].aiocb);
>      }
> -
> -    g_free(acb->qcrs);
> -    qemu_aio_release(acb);
>  }
>  
>  static AIOCBInfo quorum_aiocb_info = {
>      .aiocb_size         = sizeof(QuorumAIOCB),
> -    .cancel             = quorum_aio_cancel,
> +    .cancel_async       = quorum_aio_cancel,
>  };
>  
>  static void quorum_aio_finalize(QuorumAIOCB *acb)
> -- 
> 2.1.0.27.g96db324
> 

Look good but this seems to conflict with one of Liu ongoing patches
as the the quorum_cancel method is currently broken and Liu repair it
in his latest patchset.

See: http://lists.gnu.org/archive/html/qemu-devel/2014-09/msg00022.html

I think one of your series must rebase on the other.

Liu: can you coordinate with Fam ?

Re: [Qemu-devel] [PATCH v4 04/20] block: Convert bdrv_em_aiocb_info.cancel to .cancel_async

[Fam Zheng]

On Thu, 09/04 17:21, Benoît Canet wrote:
> The Wednesday 03 Sep 2014 à 19:23:39 (+0800), Fam Zheng wrote :
> > All the difference is that the old .cancel doesn't call cb, but
> > .cancel_async does.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  block.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/block.c b/block.c
> > index 4aa1bd7..b7253af 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -4679,6 +4679,9 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
> >  {
> >      BlockDriverAIOCBSync *acb =
> >          container_of(blockacb, BlockDriverAIOCBSync, common);
> > +
> > +    acb->ret = -ECANCELED;
> > +    acb->common.cb(acb->common.opaque, acb->ret);
> >      qemu_bh_delete(acb->bh);
> >      acb->bh = NULL;
> >      qemu_aio_release(acb);
> > @@ -4686,7 +4689,7 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
> >  
> >  static const AIOCBInfo bdrv_em_aiocb_info = {
> >      .aiocb_size         = sizeof(BlockDriverAIOCBSync),
> > -    .cancel             = bdrv_aio_cancel_em,
> > +    .cancel_async       = bdrv_aio_cancel_em,
> >  };
> 
> Note from an AIO noob. Could we explain somewhere in the block layer what the _em suffix means ?

I didn't write these functions but I think it means "emulation".

Fam

Re: [Qemu-devel] [PATCH v4 04/20] block: Convert bdrv_em_aiocb_info.cancel to .cancel_async

[Benoît Canet]

The Friday 05 Sep 2014 à 18:55:51 (+0800), Fam Zheng wrote :
> On Thu, 09/04 17:21, Benoît Canet wrote:
> > The Wednesday 03 Sep 2014 à 19:23:39 (+0800), Fam Zheng wrote :
> > > All the difference is that the old .cancel doesn't call cb, but
> > > .cancel_async does.
> > > 
> > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > > ---
> > >  block.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/block.c b/block.c
> > > index 4aa1bd7..b7253af 100644
> > > --- a/block.c
> > > +++ b/block.c
> > > @@ -4679,6 +4679,9 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
> > >  {
> > >      BlockDriverAIOCBSync *acb =
> > >          container_of(blockacb, BlockDriverAIOCBSync, common);
> > > +
> > > +    acb->ret = -ECANCELED;
> > > +    acb->common.cb(acb->common.opaque, acb->ret);
> > >      qemu_bh_delete(acb->bh);
> > >      acb->bh = NULL;
> > >      qemu_aio_release(acb);
> > > @@ -4686,7 +4689,7 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
> > >  
> > >  static const AIOCBInfo bdrv_em_aiocb_info = {
> > >      .aiocb_size         = sizeof(BlockDriverAIOCBSync),
> > > -    .cancel             = bdrv_aio_cancel_em,
> > > +    .cancel_async       = bdrv_aio_cancel_em,
> > >  };
> > 
> > Note from an AIO noob. Could we explain somewhere in the block layer what the _em suffix means ?
> 
> I didn't write these functions but I think it means "emulation".

Thanks

Best regards

Benoît

> 
> Fam