qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: qemu-block@nongnu.org, vsementsov@yandex-team.ru,
	stefanha@redhat.com, John Snow <jsnow@redhat.com>,
	Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
Subject: [PATCH v3 07/11] mirror: Skip pre-zeroing destination if it is already zero
Date: Thu, 24 Apr 2025 19:52:07 -0500	[thread overview]
Message-ID: <20250425005439.2252467-20-eblake@redhat.com> (raw)
In-Reply-To: <20250425005439.2252467-13-eblake@redhat.com>

When doing a sync=full mirroring, QMP drive-mirror requests full
zeroing if it did not just create the destination, and blockdev-mirror
requests full zeroing unconditionally.  This is because during a full
sync, we must ensure that the portions of the disk that are not
otherwise touched by the source still read as zero upon completion.

However, in mirror_dirty_init(), we were blindly assuming that if the
destination allows punching holes, we should pre-zero the entire
image; and if it does not allow punching holes, then treat the entire
source as dirty rather than mirroring just the allocated portions of
the source.  Without the ability to punch holes, this results in the
destination file being fully allocated; and even when punching holes
is supported, it causes duplicate I/O to the portions of the
destination corresponding to chunks of the source that are allocated
but read as zero.

Smarter is to avoid the pre-zeroing pass over the destination if it
can be proved the destination already reads as zero.  Note that a
later patch will then further improve things to skip writing to the
destination for parts of the image where the source is zero; but even
with just this patch, it is possible to see a difference for any BDS
that can quickly report that it already reads as zero.

Note, however, that if the destination was opened with
"detect-zeroes": "unmap", then the user wants us to punch holes where
possible for any zeroes in the source; in that case, we are better off
doing unmap up front in bulk.

Signed-off-by: Eric Blake <eblake@redhat.com>

---

v3: add exemption for "detect-zeroes":"unmap" on destination
---
 block/mirror.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 34c6c5252e1..4059bf96854 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -849,13 +849,30 @@ static int coroutine_fn GRAPH_UNLOCKED mirror_dirty_init(MirrorBlockJob *s)
     bdrv_graph_co_rdunlock();

     if (s->zero_target) {
+        offset = 0;
+        bdrv_graph_co_rdlock();
+        ret = bdrv_co_is_all_zeroes(target_bs);
+        bdrv_graph_co_rdunlock();
+        if (ret < 0) {
+            return ret;
+        }
+        /*
+         * If the destination already reads as zero, and we are not
+         * requested to punch holes into existing zeroes, then we can
+         * skip pre-zeroing the destination.
+         */
+        if (ret > 0 &&
+            (target_bs->detect_zeroes != BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP ||
+             !bdrv_can_write_zeroes_with_unmap(target_bs))) {
+            offset = s->bdev_length;
+        }
         if (!bdrv_can_write_zeroes_with_unmap(target_bs)) {
             bdrv_set_dirty_bitmap(s->dirty_bitmap, 0, s->bdev_length);
             return 0;
         }

         s->initial_zeroing_ongoing = true;
-        for (offset = 0; offset < s->bdev_length; ) {
+        while (offset < s->bdev_length) {
             int bytes = MIN(s->bdev_length - offset,
                             QEMU_ALIGN_DOWN(INT_MAX, s->granularity));

-- 
2.49.0



  parent reply	other threads:[~2025-04-25  0:55 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-25  0:52 [PATCH v3 00/11] Make blockdev-mirror dest sparse in more cases Eric Blake
2025-04-25  0:52 ` [PATCH v3 01/11] block: Expand block status mode from bool to flags Eric Blake
2025-04-25  0:52 ` [PATCH v3 02/11] file-posix, gluster: Handle zero block status hint better Eric Blake
2025-04-25  0:52 ` [PATCH v3 03/11] block: Let bdrv_co_is_zero_fast consolidate adjacent extents Eric Blake
2025-05-01 17:20   ` Stefan Hajnoczi
2025-04-25  0:52 ` [PATCH v3 04/11] block: Add new bdrv_co_is_all_zeroes() function Eric Blake
2025-05-01 17:24   ` Stefan Hajnoczi
2025-04-25  0:52 ` [PATCH v3 05/11] iotests: Improve iotest 194 to mirror data Eric Blake
2025-04-25  0:52 ` [PATCH v3 06/11] mirror: Minor refactoring Eric Blake
2025-04-25  0:52 ` Eric Blake [this message]
2025-04-30 16:09   ` [PATCH v3 07/11] mirror: Skip pre-zeroing destination if it is already zero Sunny Zhu
2025-05-01 17:33     ` Eric Blake
2025-05-02  3:26       ` Sunny Zhu
2025-04-25  0:52 ` [PATCH v3 08/11] mirror: Skip writing zeroes when target " Eric Blake
2025-04-30 16:38   ` Re " Sunny Zhu
2025-05-01 17:58     ` Eric Blake
2025-05-02  5:43       ` Sunny Zhu
2025-04-25  0:52 ` [PATCH v3 09/11] iotests/common.rc: add disk_usage function Eric Blake
2025-04-25  0:52 ` [PATCH v3 10/11] tests: Add iotest mirror-sparse for recent patches Eric Blake
2025-05-01 17:34   ` Stefan Hajnoczi
2025-05-01 18:00     ` Eric Blake
2025-05-09 15:33   ` Eric Blake
2025-08-01  7:54   ` Michael Tokarev
2025-04-25  0:52 ` [PATCH v3 11/11] mirror: Allow QMP override to declare target already zero Eric Blake

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=20250425005439.2252467-20-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=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).