diff mbox series

[1/3] job: add job_complete_ex with do_graph_change argument

Message ID 20210727164754.62895-2-vsementsov@virtuozzo.com
State New
Headers show
Series mirror: rework soft-cancelling READY mirror | expand

Commit Message

Vladimir Sementsov-Ogievskiy July 27, 2021, 4:47 p.m. UTC
It's an alternative for soft-cancelling mirror job after READY: mirror
should finish all in-flight requests, but don't change block graph in
any way.

To be used in the next commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/qemu/job.h               |  5 ++++-
 block/mirror.c                   | 20 ++++++++++++++------
 job.c                            |  9 +++++++--
 tests/unit/test-bdrv-drain.c     |  2 +-
 tests/unit/test-block-iothread.c |  2 +-
 tests/unit/test-blockjob.c       |  4 ++--
 6 files changed, 29 insertions(+), 13 deletions(-)
diff mbox series

Patch

diff --git a/include/qemu/job.h b/include/qemu/job.h
index 41162ed494..3dfb79cee6 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -211,7 +211,7 @@  struct JobDriver {
      * Optional callback for job types whose completion must be triggered
      * manually.
      */
-    void (*complete)(Job *job, Error **errp);
+    void (*complete)(Job *job, bool do_graph_change, Error **errp);
 
     /**
      * If the callback is not NULL, prepare will be invoked when all the jobs
@@ -492,6 +492,9 @@  void job_transition_to_ready(Job *job);
 /** Asynchronously complete the specified @job. */
 void job_complete(Job *job, Error **errp);
 
+/** Asynchronously complete the specified @job. */
+void job_complete_ex(Job *job, bool do_graph_change, Error **errp);
+
 /**
  * Asynchronously cancel the specified @job. If @force is true, the job should
  * be cancelled immediately without waiting for a consistent state.
diff --git a/block/mirror.c b/block/mirror.c
index 98fc66eabf..ad9736eb5e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -44,6 +44,11 @@  typedef struct MirrorBlockJob {
     BlockDriverState *base;
     BlockDriverState *base_overlay;
 
+    /*
+     * Do final graph changes. True at start, may be changed by
+     * mirror_complete().
+     */
+    bool do_graph_change;
     /* The name of the graph node to replace */
     char *replaces;
     /* The BDS to replace */
@@ -648,7 +653,7 @@  static int mirror_exit_common(Job *job)
     BlockDriverState *target_bs;
     BlockDriverState *mirror_top_bs;
     Error *local_err = NULL;
-    bool abort = job->ret < 0;
+    bool do_graph_change = s->do_graph_change && job->ret >= 0;
     int ret = 0;
 
     if (s->prepared) {
@@ -689,7 +694,7 @@  static int mirror_exit_common(Job *job)
     bs_opaque->stop = true;
     bdrv_child_refresh_perms(mirror_top_bs, mirror_top_bs->backing,
                              &error_abort);
-    if (!abort && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
+    if (do_graph_change && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
         BlockDriverState *backing = s->is_none_mode ? src : s->base;
         BlockDriverState *unfiltered_target = bdrv_skip_filters(target_bs);
 
@@ -701,7 +706,7 @@  static int mirror_exit_common(Job *job)
                 ret = -EPERM;
             }
         }
-    } else if (!abort && s->backing_mode == MIRROR_OPEN_BACKING_CHAIN) {
+    } else if (do_graph_change && s->backing_mode == MIRROR_OPEN_BACKING_CHAIN) {
         assert(!bdrv_backing_chain_next(target_bs));
         ret = bdrv_open_backing_file(bdrv_skip_filters(target_bs), NULL,
                                      "backing", &local_err);
@@ -716,7 +721,7 @@  static int mirror_exit_common(Job *job)
         aio_context_acquire(replace_aio_context);
     }
 
-    if (s->should_complete && !abort) {
+    if (s->should_complete && do_graph_change) {
         BlockDriverState *to_replace = s->to_replace ?: src;
         bool ro = bdrv_is_read_only(to_replace);
 
@@ -1124,7 +1129,7 @@  immediate_exit:
     return ret;
 }
 
-static void mirror_complete(Job *job, Error **errp)
+static void mirror_complete(Job *job, bool do_graph_change, Error **errp)
 {
     MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);
 
@@ -1134,8 +1139,10 @@  static void mirror_complete(Job *job, Error **errp)
         return;
     }
 
+    s->do_graph_change = do_graph_change;
+
     /* block all operations on to_replace bs */
-    if (s->replaces) {
+    if (s->do_graph_change && s->replaces) {
         AioContext *replace_aio_context;
 
         s->to_replace = bdrv_find_node(s->replaces);
@@ -1737,6 +1744,7 @@  static BlockJob *mirror_start_job(
     blk_set_allow_aio_context_change(s->target, true);
     blk_set_disable_request_queuing(s->target, true);
 
+    s->do_graph_change = true;
     s->replaces = g_strdup(replaces);
     s->on_source_error = on_source_error;
     s->on_target_error = on_target_error;
diff --git a/job.c b/job.c
index e7a5d28854..52127dd6bd 100644
--- a/job.c
+++ b/job.c
@@ -987,7 +987,7 @@  int job_complete_sync(Job *job, Error **errp)
     return job_finish_sync(job, job_complete, errp);
 }
 
-void job_complete(Job *job, Error **errp)
+void job_complete_ex(Job *job, bool do_graph_change, Error **errp)
 {
     /* Should not be reachable via external interface for internal jobs */
     assert(job->id);
@@ -1000,7 +1000,12 @@  void job_complete(Job *job, Error **errp)
         return;
     }
 
-    job->driver->complete(job, errp);
+    job->driver->complete(job, true, errp);
+}
+
+void job_complete(Job *job, Error **errp)
+{
+    job_complete_ex(job, false, errp);
 }
 
 int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error **errp)
diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index ce071b5fc5..b754eca27b 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -823,7 +823,7 @@  static int coroutine_fn test_job_run(Job *job, Error **errp)
     return s->run_ret;
 }
 
-static void test_job_complete(Job *job, Error **errp)
+static void test_job_complete(Job *job, bool do_graph_change, Error **errp)
 {
     TestBlockJob *s = container_of(job, TestBlockJob, common.job);
     s->should_complete = true;
diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
index c39e70b2f5..d07ba69aee 100644
--- a/tests/unit/test-block-iothread.c
+++ b/tests/unit/test-block-iothread.c
@@ -385,7 +385,7 @@  static int coroutine_fn test_job_run(Job *job, Error **errp)
     return 0;
 }
 
-static void test_job_complete(Job *job, Error **errp)
+static void test_job_complete(Job *job, bool do_graph_change, Error **errp)
 {
     TestBlockJob *s = container_of(job, TestBlockJob, common.job);
     s->should_complete = true;
diff --git a/tests/unit/test-blockjob.c b/tests/unit/test-blockjob.c
index dcacfa6c7c..b2653a3733 100644
--- a/tests/unit/test-blockjob.c
+++ b/tests/unit/test-blockjob.c
@@ -165,7 +165,7 @@  typedef struct CancelJob {
     bool should_complete;
 } CancelJob;
 
-static void cancel_job_complete(Job *job, Error **errp)
+static void cancel_job_complete(Job *job, bool do_graph_change, Error **errp)
 {
     CancelJob *s = container_of(job, CancelJob, common.job);
     s->should_complete = true;
@@ -382,7 +382,7 @@  typedef struct YieldingJob {
     bool should_complete;
 } YieldingJob;
 
-static void yielding_job_complete(Job *job, Error **errp)
+static void yielding_job_complete(Job *job, bool do_graph_change, Error **errp)
 {
     YieldingJob *s = container_of(job, YieldingJob, common.job);
     s->should_complete = true;