[Qemu-devel] [PATCH] mirror: follow AioContext change gracefully

[Qemu-devel] [PATCH] mirror: follow AioContext change gracefully

[Stefan Hajnoczi]

When dataplane is enabled or disabled the drive switches to a new
AioContext.  The mirror block job must also move to the new AioContext
so that drive accesses are always made within its AioContext.

This work-in-progress patch partially achieves that by draining
s->target of in-flight write requests to reach a quiescent point.  The
job is resumed in the new AioContext after moving s->target into the new
AioContext.

Unsolved cases include block_job_sleep_ns(), bdrv_is_allocated_above(),
and bdrv_get_block_status_above().  Currently they continue executing in
the old AioContext.  Perhaps something similar to
block_job_is_cancelled() checking is necessary so the coroutine can move
itself to the new AioContext.

Cc: Fam Zheng <famz@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
Jason: Please try this patch and see if it fixes the abort.  It has
fixed migration after drive_mirror for me.
---
 block/mirror.c | 41 +++++++++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 80fd3c7..62d948d 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -63,6 +63,7 @@ typedef struct MirrorBlockJob {
     int ret;
     bool unmap;
     bool waiting_for_io;
+    bool detached; /* temporarily detached from AioContext, don't do I/O */
     int target_cluster_sectors;
     int max_iov;
 } MirrorBlockJob;
@@ -119,7 +120,7 @@ static void mirror_iteration_done(MirrorOp *op, int ret)
     qemu_iovec_destroy(&op->qiov);
     g_free(op);
 
-    if (s->waiting_for_io) {
+    if (s->waiting_for_io && !s->detached) {
         qemu_coroutine_enter(s->common.co, NULL);
     }
 }
@@ -375,7 +376,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
         assert(!(sector_num % sectors_per_chunk));
         ret = bdrv_get_block_status_above(source, NULL, sector_num,
                                           nb_chunks * sectors_per_chunk,
-                                          &io_sectors, &file);
+                                          &io_sectors, &file); /* TODO blocking */
         if (ret < 0) {
             io_sectors = nb_chunks * sectors_per_chunk;
         }
@@ -442,6 +443,29 @@ static void mirror_drain(MirrorBlockJob *s)
     }
 }
 
+static void mirror_attached_aio_context(AioContext *new_context, void *opaque)
+{
+    MirrorBlockJob *s = opaque;
+
+    blk_set_aio_context(s->target, new_context);
+
+    /* Resume execution */
+    s->detached = false;
+    if (s->waiting_for_io) {
+        qemu_coroutine_enter(s->common.co, NULL);
+    }
+}
+
+static void mirror_detach_aio_context(void *opaque)
+{
+    MirrorBlockJob *s = opaque;
+
+    s->detached = true;
+
+    /* Complete pending write requests */
+    blk_drain(s->target);
+}
+
 typedef struct {
     int ret;
 } MirrorExitData;
@@ -491,6 +515,8 @@ static void mirror_exit(BlockJob *job, void *opaque)
     if (replace_aio_context) {
         aio_context_release(replace_aio_context);
     }
+    blk_remove_aio_context_notifier(s->common.blk, mirror_attached_aio_context,
+                                    mirror_detach_aio_context, s);
     g_free(s->replaces);
     bdrv_op_unblock_all(target_bs, s->common.blocker);
     blk_unref(s->target);
@@ -580,14 +606,14 @@ static void coroutine_fn mirror_run(void *opaque)
 
             if (now - last_pause_ns > SLICE_TIME) {
                 last_pause_ns = now;
-                block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, 0);
+                block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, 0); /* TODO blocking */
             }
 
             if (block_job_is_cancelled(&s->common)) {
                 goto immediate_exit;
             }
 
-            ret = bdrv_is_allocated_above(bs, base, sector_num, nb_sectors, &n);
+            ret = bdrv_is_allocated_above(bs, base, sector_num, nb_sectors, &n); /* TODO blocking */
 
             if (ret < 0) {
                 goto immediate_exit;
@@ -680,13 +706,13 @@ static void coroutine_fn mirror_run(void *opaque)
         ret = 0;
         trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
         if (!s->synced) {
-            block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, delay_ns);
+            block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, delay_ns); /* TODO blocking */
             if (block_job_is_cancelled(&s->common)) {
                 break;
             }
         } else if (!should_complete) {
             delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0);
-            block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, delay_ns);
+            block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, delay_ns); /* TODO blocking */
         } else if (cnt == 0) {
             /* The two disks are in sync.  Exit and report successful
              * completion.
@@ -851,6 +877,9 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
 
     bdrv_op_block_all(target, s->common.blocker);
 
+    blk_add_aio_context_notifier(s->common.blk, mirror_attached_aio_context,
+                                 mirror_detach_aio_context, s);
+
     s->common.co = qemu_coroutine_create(mirror_run);
     trace_mirror_start(bs, s, s->common.co, opaque);
     qemu_coroutine_enter(s->common.co, s);
-- 
2.5.5

Re: [Qemu-devel] [PATCH] mirror: follow AioContext change gracefully

[Stefan Hajnoczi]

On Thu, Jun 9, 2016 at 5:25 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> This work-in-progress patch partially achieves that by draining
> s->target of in-flight write requests to reach a quiescent point.

I forgot the [RFC] tag on this patch.  Just wanted to confirm that I'm
not proposing this patch for merge.

Stefan