qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: Sunny Zhu <sunnyzhyy@qq.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	John Snow <jsnow@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>,
	Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>,
	qemu-block@nongnu.org (open list:Block Jobs)
Subject: [PULL 07/14] mirror: Pass full sync mode rather than bool to internals
Date: Wed, 14 May 2025 21:28:50 -0500	[thread overview]
Message-ID: <20250515022904.575509-23-eblake@redhat.com> (raw)
In-Reply-To: <20250515022904.575509-16-eblake@redhat.com>

Out of the five possible values for MirrorSyncMode, INCREMENTAL and
BITMAP are already rejected up front in mirror_start, leaving NONE,
TOP, and FULL as the remaining values that the code was collapsing
into a single bool is_none_mode.  Furthermore, mirror_dirty_init() is
only reachable for modes TOP and FULL, as further guided by
s->zero_target.  However, upcoming patches want to further optimize
the pre-zeroing pass of a sync=full mirror in mirror_dirty_init(),
while avoiding that pass on a sync=top action.  Instead of throwing
away context by collapsing these two values into
s->is_none_mode=false, it is better to pass s->sync_mode throughout
the entire operation.  For active commit, the desired semantics match
sync mode TOP.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20250509204341.3553601-22-eblake@redhat.com>
Reviewed-by: Sunny Zhu <sunnyzhyy@qq.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/mirror.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 34c6c5252e1..2599b75d092 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -51,7 +51,7 @@ typedef struct MirrorBlockJob {
     BlockDriverState *to_replace;
     /* Used to block operations on the drive-mirror-replace target */
     Error *replace_blocker;
-    bool is_none_mode;
+    MirrorSyncMode sync_mode;
     BlockMirrorBackingMode backing_mode;
     /* Whether the target image requires explicit zero-initialization */
     bool zero_target;
@@ -723,9 +723,10 @@ static int mirror_exit_common(Job *job)
                              &error_abort);

     if (!abort && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
-        BlockDriverState *backing = s->is_none_mode ? src : s->base;
+        BlockDriverState *backing;
         BlockDriverState *unfiltered_target = bdrv_skip_filters(target_bs);

+        backing = s->sync_mode == MIRROR_SYNC_MODE_NONE ? src : s->base;
         if (bdrv_cow_bs(unfiltered_target) != backing) {
             bdrv_set_backing_hd(unfiltered_target, backing, &local_err);
             if (local_err) {
@@ -1020,7 +1021,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
     mirror_free_init(s);

     s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
-    if (!s->is_none_mode) {
+    if (s->sync_mode != MIRROR_SYNC_MODE_NONE) {
         ret = mirror_dirty_init(s);
         if (ret < 0 || job_is_cancelled(&s->common.job)) {
             goto immediate_exit;
@@ -1711,6 +1712,7 @@ static BlockJob *mirror_start_job(
                              int creation_flags, BlockDriverState *target,
                              const char *replaces, int64_t speed,
                              uint32_t granularity, int64_t buf_size,
+                             MirrorSyncMode sync_mode,
                              BlockMirrorBackingMode backing_mode,
                              bool zero_target,
                              BlockdevOnError on_source_error,
@@ -1719,7 +1721,7 @@ static BlockJob *mirror_start_job(
                              BlockCompletionFunc *cb,
                              void *opaque,
                              const BlockJobDriver *driver,
-                             bool is_none_mode, BlockDriverState *base,
+                             BlockDriverState *base,
                              bool auto_complete, const char *filter_node_name,
                              bool is_mirror, MirrorCopyMode copy_mode,
                              bool base_ro,
@@ -1878,7 +1880,7 @@ static BlockJob *mirror_start_job(
     s->replaces = g_strdup(replaces);
     s->on_source_error = on_source_error;
     s->on_target_error = on_target_error;
-    s->is_none_mode = is_none_mode;
+    s->sync_mode = sync_mode;
     s->backing_mode = backing_mode;
     s->zero_target = zero_target;
     qatomic_set(&s->copy_mode, copy_mode);
@@ -2015,7 +2017,6 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
                   bool unmap, const char *filter_node_name,
                   MirrorCopyMode copy_mode, Error **errp)
 {
-    bool is_none_mode;
     BlockDriverState *base;

     GLOBAL_STATE_CODE();
@@ -2028,14 +2029,13 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
     }

     bdrv_graph_rdlock_main_loop();
-    is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
     base = mode == MIRROR_SYNC_MODE_TOP ? bdrv_backing_chain_next(bs) : NULL;
     bdrv_graph_rdunlock_main_loop();

     mirror_start_job(job_id, bs, creation_flags, target, replaces,
-                     speed, granularity, buf_size, backing_mode, zero_target,
-                     on_source_error, on_target_error, unmap, NULL, NULL,
-                     &mirror_job_driver, is_none_mode, base, false,
+                     speed, granularity, buf_size, mode, backing_mode,
+                     zero_target, on_source_error, on_target_error, unmap,
+                     NULL, NULL, &mirror_job_driver, base, false,
                      filter_node_name, true, copy_mode, false, errp);
 }

@@ -2061,9 +2061,9 @@ BlockJob *commit_active_start(const char *job_id, BlockDriverState *bs,

     job = mirror_start_job(
                      job_id, bs, creation_flags, base, NULL, speed, 0, 0,
-                     MIRROR_LEAVE_BACKING_CHAIN, false,
+                     MIRROR_SYNC_MODE_TOP, MIRROR_LEAVE_BACKING_CHAIN, false,
                      on_error, on_error, true, cb, opaque,
-                     &commit_active_job_driver, false, base, auto_complete,
+                     &commit_active_job_driver, base, auto_complete,
                      filter_node_name, false, MIRROR_COPY_MODE_BACKGROUND,
                      base_read_only, errp);
     if (!job) {
-- 
2.49.0



  parent reply	other threads:[~2025-05-15  2:32 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-15  2:28 [PULL 00/14] NBD patches for 2025-05-14 Eric Blake
2025-05-15  2:28 ` [PULL 01/14] block: Expand block status mode from bool to flags Eric Blake
2025-05-15  2:28 ` [PULL 02/14] file-posix, gluster: Handle zero block status hint better Eric Blake
2025-05-15  2:28 ` [PULL 03/14] block: Let bdrv_co_is_zero_fast consolidate adjacent extents Eric Blake
2025-05-15  2:28 ` [PULL 04/14] block: Add new bdrv_co_is_all_zeroes() function Eric Blake
2025-05-15  2:28 ` [PULL 05/14] iotests: Improve iotest 194 to mirror data Eric Blake
2025-05-15  2:28 ` [PULL 06/14] mirror: Minor refactoring Eric Blake
2025-05-15  2:28 ` Eric Blake [this message]
2025-05-15  2:28 ` [PULL 08/14] mirror: Allow QMP override to declare target already zero Eric Blake
2025-05-15  2:28 ` [PULL 09/14] mirror: Drop redundant zero_target parameter Eric Blake
2025-05-15  2:28 ` [PULL 10/14] mirror: Skip pre-zeroing destination if it is already zero Eric Blake
2025-05-15  2:28 ` [PULL 11/14] mirror: Skip writing zeroes when target " Eric Blake
2025-05-15  2:28 ` [PULL 12/14] iotests/common.rc: add disk_usage function Eric Blake
2025-05-15  2:28 ` [PULL 13/14] tests: Add iotest mirror-sparse for recent patches Eric Blake
2025-05-21  9:54   ` Fiona Ebner
2025-05-21 15:32     ` Eric Blake
2025-05-22  7:30       ` Fiona Ebner
2025-05-28 11:39   ` Markus Armbruster
2025-05-28 12:40     ` Eric Blake
2025-05-28 13:27       ` Markus Armbruster
2025-05-28 15:40     ` Eric Blake
2025-05-28 16:23       ` Markus Armbruster
2025-05-28 18:22         ` Eric Blake
2025-05-15  2:28 ` [PULL 14/14] mirror: Reduce I/O when destination is detect-zeroes:unmap Eric Blake
2025-05-15 21:53 ` [PULL 00/14] NBD patches for 2025-05-14 Stefan Hajnoczi

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=20250515022904.575509-23-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=sunnyzhyy@qq.com \
    --cc=vsementsov@yandex-team.ru \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).