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 10/14] mirror: Skip pre-zeroing destination if it is already zero
Date: Wed, 14 May 2025 21:28:53 -0500 [thread overview]
Message-ID: <20250515022904.575509-26-eblake@redhat.com> (raw)
In-Reply-To: <20250515022904.575509-16-eblake@redhat.com>
When doing a sync=full mirroring, we can skip pre-zeroing the
destination if it already reads as zeroes and we are not also trying
to punch holes due to detect-zeroes. With this patch, there are fewer
scenarios that have to pass in an explicit target-is-zero, while still
resulting in a sparse destination remaining sparse.
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
source that does not report itself as fully allocated, coupled with a
destination BDS that can quickly report that it already reads as zero.
(For a source that reports as fully allocated, such as a file, the
rest of mirror_dirty_init() still sets the entire dirty bitmap to
true, so even though we avoided the pre-zeroing, we are not yet
avoiding all redundant I/O).
Iotest 194 detects the difference made by this patch: for a file
source (where block status reports the entire image as allocated, and
therefore we end up writing zeroes everywhere in the destination
anyways), the job length remains the same. But for a qcow2 source and
a destination that reads as all zeroes, the dirty bitmap changes to
just tracking the allocated portions of the source, which results in
faster completion and smaller job statistics. For the test to pass
with both ./check -file and -qcow2, a new python filter is needed to
mask out the now-varying job amounts (this matches the shell filters
_filter_block_job_{offset,len} in common.filter). A later test will
also be added which further validates expected sparseness, so it does
not matter that 194 is no longer explicitly looking at how many bytes
were copied.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20250509204341.3553601-25-eblake@redhat.com>
Reviewed-by: Sunny Zhu <sunnyzhyy@qq.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block/mirror.c | 24 ++++++++++++++++--------
tests/qemu-iotests/194 | 6 ++++--
tests/qemu-iotests/194.out | 4 ++--
tests/qemu-iotests/iotests.py | 12 +++++++++++-
4 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index d04db85883d..bca99ec206b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -848,23 +848,31 @@ static int coroutine_fn GRAPH_UNLOCKED mirror_dirty_init(MirrorBlockJob *s)
target_bs->detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP &&
bdrv_can_write_zeroes_with_unmap(target_bs);
+ /* Determine if the image is already zero, regardless of sync mode. */
bdrv_graph_co_rdlock();
bs = s->mirror_top_bs->backing->bs;
+ if (s->target_is_zero) {
+ ret = 1;
+ } else {
+ ret = bdrv_co_is_all_zeroes(target_bs);
+ }
bdrv_graph_co_rdunlock();
- if (s->sync_mode == MIRROR_SYNC_MODE_TOP) {
+ /* Determine if a pre-zeroing pass is necessary. */
+ if (ret < 0) {
+ return ret;
+ } else if (s->sync_mode == MIRROR_SYNC_MODE_TOP) {
/* In TOP mode, there is no benefit to a pre-zeroing pass. */
- } else if (!s->target_is_zero || punch_holes) {
+ } else if (ret == 0 || punch_holes) {
/*
* Here, we are in FULL mode; our goal is to avoid writing
* zeroes if the destination already reads as zero, except
* when we are trying to punch holes. This is possible if
- * zeroing happened externally (s->target_is_zero) or if we
- * have a fast way to pre-zero the image (the dirty bitmap
- * will be populated later by the non-zero portions, the same
- * as for TOP mode). If pre-zeroing is not fast, or we need
- * to punch holes, then our only recourse is to write the
- * entire image.
+ * zeroing happened externally (ret > 0) or if we have a fast
+ * way to pre-zero the image (the dirty bitmap will be
+ * populated later by the non-zero portions, the same as for
+ * TOP mode). If pre-zeroing is not fast, or we need to punch
+ * holes, then our only recourse is to write the entire image.
*/
if (!bdrv_can_write_zeroes_with_unmap(target_bs)) {
bdrv_set_dirty_bitmap(s->dirty_bitmap, 0, s->bdev_length);
diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
index d0b9c084f5f..e114c0b2695 100755
--- a/tests/qemu-iotests/194
+++ b/tests/qemu-iotests/194
@@ -62,7 +62,8 @@ with iotests.FilePath('source.img') as source_img_path, \
iotests.log('Waiting for `drive-mirror` to complete...')
iotests.log(source_vm.event_wait('BLOCK_JOB_READY'),
- filters=[iotests.filter_qmp_event])
+ filters=[iotests.filter_qmp_event,
+ iotests.filter_block_job])
iotests.log('Starting migration...')
capabilities = [{'capability': 'events', 'state': True},
@@ -88,7 +89,8 @@ with iotests.FilePath('source.img') as source_img_path, \
while True:
event2 = source_vm.event_wait('BLOCK_JOB_COMPLETED')
- iotests.log(event2, filters=[iotests.filter_qmp_event])
+ iotests.log(event2, filters=[iotests.filter_qmp_event,
+ iotests.filter_block_job])
if event2['event'] == 'BLOCK_JOB_COMPLETED':
iotests.log('Stopping the NBD server on destination...')
iotests.log(dest_vm.qmp('nbd-server-stop'))
diff --git a/tests/qemu-iotests/194.out b/tests/qemu-iotests/194.out
index 6940e809cde..d02655a5147 100644
--- a/tests/qemu-iotests/194.out
+++ b/tests/qemu-iotests/194.out
@@ -7,7 +7,7 @@ Launching NBD server on destination...
Starting `drive-mirror` on source...
{"return": {}}
Waiting for `drive-mirror` to complete...
-{"data": {"device": "mirror-job0", "len": 1073741824, "offset": 1073741824, "speed": 0, "type": "mirror"}, "event": "BLOCK_JOB_READY", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"data": {"device": "mirror-job0", "len": "LEN", "offset": "OFFSET", "speed": 0, "type": "mirror"}, "event": "BLOCK_JOB_READY", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
Starting migration...
{"return": {}}
{"execute": "migrate-start-postcopy", "arguments": {}}
@@ -18,7 +18,7 @@ Starting migration...
{"data": {"status": "completed"}, "event": "MIGRATION", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
Gracefully ending the `drive-mirror` job on source...
{"return": {}}
-{"data": {"device": "mirror-job0", "len": 1073741824, "offset": 1073741824, "speed": 0, "type": "mirror"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"data": {"device": "mirror-job0", "len": "LEN", "offset": "OFFSET", "speed": 0, "type": "mirror"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
Stopping the NBD server on destination...
{"return": {}}
Wait for migration completion on target...
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 7292c8b342a..05274772ce4 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -601,13 +601,23 @@ def filter_chown(msg):
return chown_re.sub("chown UID:GID", msg)
def filter_qmp_event(event):
- '''Filter a QMP event dict'''
+ '''Filter the timestamp of a QMP event dict'''
event = dict(event)
if 'timestamp' in event:
event['timestamp']['seconds'] = 'SECS'
event['timestamp']['microseconds'] = 'USECS'
return event
+def filter_block_job(event):
+ '''Filter the offset and length of a QMP block job event dict'''
+ event = dict(event)
+ if 'data' in event:
+ if 'offset' in event['data']:
+ event['data']['offset'] = 'OFFSET'
+ if 'len' in event['data']:
+ event['data']['len'] = 'LEN'
+ return event
+
def filter_qmp(qmsg, filter_fn):
'''Given a string filter, filter a QMP object's values.
filter_fn takes a (key, value) pair.'''
--
2.49.0
next prev 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 ` [PULL 07/14] mirror: Pass full sync mode rather than bool to internals Eric Blake
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 ` Eric Blake [this message]
2025-05-15 2:28 ` [PULL 11/14] mirror: Skip writing zeroes when target is already zero 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-26-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).