All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, armbru@redhat.com, mreitz@redhat.com,
	kwolf@redhat.com, vsementsov@virtuozzo.com, jsnow@redhat.com
Subject: [PATCH 1/3] job: add job_complete_ex with do_graph_change argument
Date: Tue, 27 Jul 2021 19:47:52 +0300	[thread overview]
Message-ID: <20210727164754.62895-2-vsementsov@virtuozzo.com> (raw)
In-Reply-To: <20210727164754.62895-1-vsementsov@virtuozzo.com>

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 --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;
-- 
2.29.2



  reply	other threads:[~2021-07-27 16:49 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27 16:47 [PATCH RFC 0/3] mirror: rework soft-cancelling READY mirror Vladimir Sementsov-Ogievskiy
2021-07-27 16:47 ` Vladimir Sementsov-Ogievskiy [this message]
2021-07-27 16:47 ` [PATCH 2/3] job: use complete(do_graph_change=false) to handle soft cancel Vladimir Sementsov-Ogievskiy
2021-07-27 16:47 ` [PATCH 3/3] job: drop force argument of *job*cancel Vladimir Sementsov-Ogievskiy
2021-07-27 16:56 ` [PATCH RFC 0/3] mirror: rework soft-cancelling READY mirror Vladimir Sementsov-Ogievskiy
2021-07-28  7:00 ` Max Reitz
2021-07-29 10:02   ` Vladimir Sementsov-Ogievskiy
2021-07-29 10:38     ` Max Reitz
2021-07-29 11:35       ` Vladimir Sementsov-Ogievskiy
2021-07-29 13:47         ` Max Reitz
2021-07-29 16:29           ` Vladimir Sementsov-Ogievskiy
2021-07-30 15:11             ` Max Reitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210727164754.62895-2-vsementsov@virtuozzo.com \
    --to=vsementsov@virtuozzo.com \
    --cc=armbru@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.