* [PATCH v4 01/12] block/commit: pass BDRV_WANT_PRECISE to block-status
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
@ 2026-10-30 1:06 ` Denis V. Lunev
2026-10-30 1:06 ` [PATCH v4 02/12] iotests/040: cover large and fragmented commit runs Denis V. Lunev
` (11 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Denis V. Lunev @ 2026-10-30 1:06 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Denis V. Lunev, Andrey Drobyshev, Eric Blake,
Vladimir Sementsov-Ogievskiy, John Snow
From: Denis V. Lunev <den@openvz.org>
c33159dec790 replaced the want_zero bool with a bitmask of BDRV_WANT_*
flags. block/commit.c was missed and still passes "true", which is
BDRV_BLOCK_DATA, not BDRV_WANT_PRECISE. Convert it like the other
callers.
Fixes: c33159dec790 ("block: Expand block status mode from bool to flags")
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: John Snow <jsnow@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
CC: Eric Blake <eblake@redhat.com>
---
block/commit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index 2d52c39594..4e0b0f9029 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -140,8 +140,8 @@ commit_iteration(CommitBlockJob *s, int64_t offset,
/* Copy if allocated above the base */
WITH_GRAPH_RDLOCK_GUARD() {
ret = bdrv_co_common_block_status_above(blk_bs(s->top),
- s->base_overlay, true, true, offset, COMMIT_BUFFER_SIZE,
- &bytes, NULL, NULL, NULL);
+ s->base_overlay, true, BDRV_WANT_PRECISE, offset,
+ COMMIT_BUFFER_SIZE, &bytes, NULL, NULL, NULL);
}
trace_commit_one_iteration(s, offset, bytes, ret);
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 02/12] iotests/040: cover large and fragmented commit runs
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
2026-10-30 1:06 ` [PATCH v4 01/12] block/commit: pass BDRV_WANT_PRECISE to block-status Denis V. Lunev
@ 2026-10-30 1:06 ` Denis V. Lunev
2026-10-30 1:06 ` [PATCH v4 03/12] block/commit: batch block-status queries Denis V. Lunev
` (10 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Denis V. Lunev @ 2026-10-30 1:06 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Denis V. Lunev, Andrey Drobyshev,
Vladimir Sementsov-Ogievskiy, John Snow
From: Denis V. Lunev <den@openvz.org>
040 never commits runs long enough to span more than one block-status
query, so the answer that commit_iteration() is about to start caching
goes untested.
Add two cases on a base <- mid <- active chain, committing mid so the
job takes the regular commit path rather than active commit, and compare
base against a snapshot of mid taken before the commit. One case mixes
multi-megabyte data, zero and hole runs, the other fragments them down
to single clusters with every transition off the 512K boundary.
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: John Snow <jsnow@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
tests/qemu-iotests/040 | 102 ++++++++++++++++++++++++++++++++++++-
tests/qemu-iotests/040.out | 4 +-
2 files changed, 103 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 5c18e413ec..e01c9385ba 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -25,13 +25,14 @@
import time
import os
import iotests
-from iotests import qemu_img, qemu_io
+from iotests import qemu_img, qemu_img_create, qemu_io, compare_images
import struct
import errno
backing_img = os.path.join(iotests.test_dir, 'backing.img')
mid_img = os.path.join(iotests.test_dir, 'mid.img')
test_img = os.path.join(iotests.test_dir, 'test.img')
+reference_img = os.path.join(iotests.test_dir, 'reference.img')
class ImageCommitTestCase(iotests.QMPTestCase):
'''Abstract base class for image commit test cases'''
@@ -951,6 +952,105 @@ class TestCommitWithOverriddenBacking(iotests.QMPTestCase):
self.vm.qmp('block-job-complete', device='commit')
self.vm.event_wait('BLOCK_JOB_COMPLETED')
+class TestCommitLargeRuns(iotests.QMPTestCase):
+ """Commit runs long enough to cross commit_iteration()'s cached span."""
+
+ MB = 1024 * 1024
+ CLUSTER = 64 * 1024
+
+ # Runs several COMMIT_BUFFER_SIZE (512K) chunks long, of every kind.
+ SIZE = 32 * MB
+ LAYOUT = [
+ (0, 4 * MB, 'data'),
+ (4 * MB, 6 * MB, 'hole'),
+ (10 * MB, 4 * MB, 'data'),
+ (14 * MB, 6 * MB, 'zero'),
+ (20 * MB, 4 * MB, 'data'),
+ (24 * MB, 8 * MB, 'hole'),
+ ]
+
+ # The same, with every transition off the 512K boundary.
+ SIZE_FRAGMENTED = 384 * CLUSTER # 24M
+ LAYOUT_FRAGMENTED = [
+ (0, 45 * CLUSTER, 'data'),
+ (45 * CLUSTER, 55 * CLUSTER, 'hole'),
+ (100 * CLUSTER, CLUSTER, 'data'),
+ (101 * CLUSTER, 49 * CLUSTER, 'zero'),
+ (150 * CLUSTER, 80 * CLUSTER, 'data'),
+ (230 * CLUSTER, CLUSTER, 'hole'),
+ (231 * CLUSTER, 69 * CLUSTER, 'data'),
+ (300 * CLUSTER, 83 * CLUSTER, 'zero'),
+ (383 * CLUSTER, CLUSTER, 'hole'),
+ ]
+
+ def setUp(self):
+ self.vm = iotests.VM()
+ self.vm.launch()
+
+ def tearDown(self):
+ self.vm.shutdown()
+ for img in (backing_img, mid_img, test_img, reference_img):
+ if os.path.exists(img):
+ os.remove(img)
+
+ def build_images(self, layout, size):
+ # A pattern of its own in base, so a misplaced cluster shows up.
+ qemu_img_create('-f', iotests.imgfmt, backing_img, str(size))
+ qemu_io('-c', f'write -P 0x11 0 {size}', backing_img)
+
+ qemu_img_create('-f', iotests.imgfmt, '-b', backing_img, '-F',
+ iotests.imgfmt, mid_img, str(size))
+ for offset, length, kind in layout:
+ if kind == 'data':
+ qemu_io('-c', f'write -P 0x22 {offset} {length}', mid_img)
+ elif kind == 'zero':
+ qemu_io('-c', f'write -z {offset} {length}', mid_img)
+
+ # What base must equal once mid is committed into it.
+ qemu_img('convert', '-f', iotests.imgfmt, '-O', iotests.imgfmt,
+ mid_img, reference_img)
+
+ # An empty layer above mid, so top_node=mid is not the active one.
+ qemu_img_create('-f', iotests.imgfmt, '-b', mid_img, '-F',
+ iotests.imgfmt, test_img, str(size))
+
+ self.vm.cmd('blockdev-add', {
+ 'node-name': 'base',
+ 'driver': iotests.imgfmt,
+ 'file': {'driver': 'file', 'filename': backing_img},
+ })
+ self.vm.cmd('blockdev-add', {
+ 'node-name': 'mid',
+ 'driver': iotests.imgfmt,
+ 'file': {'driver': 'file', 'filename': mid_img},
+ 'backing': 'base',
+ })
+ self.vm.cmd('blockdev-add', {
+ 'node-name': 'active',
+ 'driver': iotests.imgfmt,
+ 'file': {'driver': 'file', 'filename': test_img},
+ 'backing': 'mid',
+ })
+
+ def commit_and_verify(self):
+ self.vm.cmd('block-commit', job_id='commit0', device='active',
+ top_node='mid', base_node='base')
+ self.wait_until_completed(drive='commit0')
+
+ self.vm.cmd('blockdev-del', node_name='active')
+ self.vm.cmd('blockdev-del', node_name='mid')
+ self.vm.cmd('blockdev-del', node_name='base')
+ self.assertTrue(compare_images(reference_img, backing_img))
+
+ def test_commit_large_runs(self):
+ self.build_images(self.LAYOUT, self.SIZE)
+ self.commit_and_verify()
+
+ def test_commit_fragmented_runs(self):
+ self.build_images(self.LAYOUT_FRAGMENTED, self.SIZE_FRAGMENTED)
+ self.commit_and_verify()
+
+
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2', 'qed'],
supported_protocols=['file'])
diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out
index 1bb1dc5f0e..d2e2a2d98f 100644
--- a/tests/qemu-iotests/040.out
+++ b/tests/qemu-iotests/040.out
@@ -1,5 +1,5 @@
-.................................................................
+...................................................................
----------------------------------------------------------------------
-Ran 65 tests
+Ran 67 tests
OK
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 03/12] block/commit: batch block-status queries
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
2026-10-30 1:06 ` [PATCH v4 01/12] block/commit: pass BDRV_WANT_PRECISE to block-status Denis V. Lunev
2026-10-30 1:06 ` [PATCH v4 02/12] iotests/040: cover large and fragmented commit runs Denis V. Lunev
@ 2026-10-30 1:06 ` Denis V. Lunev
2026-10-30 1:06 ` [PATCH v4 04/12] iotests/124: cover backup of zero clusters and holes Denis V. Lunev
` (9 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Denis V. Lunev @ 2026-10-30 1:06 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Denis V. Lunev, Andrey Drobyshev,
Vladimir Sementsov-Ogievskiy, John Snow
From: Denis V. Lunev <den@openvz.org>
commit_iteration() asks for block status COMMIT_BUFFER_SIZE (512K) at a
time, so a long run above base pays one query per 512K for an answer
the whole run shares.
Query the remainder of the image instead and keep the answer in a
CommitStatus owned by commit_run(). No device or export writes top
above base_overlay while the job runs: they ask for
BLK_PERM_CONSISTENT_READ along with BLK_PERM_WRITE, and the job
withholds CONSISTENT_READ there, having to share WRITE or it would
block its own writes to base through the backing chain. A job target
asks for WRITE alone and is refused by the op blocker instead. From
filtered_base downwards CONSISTENT_READ is shared again, and there only
the job writes.
Copying is still bounded by the read buffer. Zeroes need no buffer, so
COMMIT_ZERO_CHUNK bounds them, keeping a cancel from waiting on a huge
write-zeroes, and an unallocated span is crossed in one step.
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: John Snow <jsnow@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/commit.c | 58 ++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 47 insertions(+), 11 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index 4e0b0f9029..e8ddd46053 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -31,8 +31,23 @@ enum {
* contiguous regions of the image is efficient.
*/
COMMIT_BUFFER_SIZE = 512 * 1024, /* in bytes */
+
+ /*
+ * Zeroes need no buffer, so they are bounded by this instead. It stays
+ * well under BDRV_REQUEST_MAX_BYTES and keeps a cancel from waiting on
+ * a multi-gigabyte write.
+ */
+ COMMIT_ZERO_CHUNK = 256 * 1024 * 1024, /* in bytes */
};
+/* Last block-status answer, covering [offset, end). Empty when equal. */
+typedef struct CommitStatus {
+ int64_t len;
+ int64_t offset;
+ int64_t end;
+ int ret;
+} CommitStatus;
+
typedef struct CommitBlockJob {
BlockJob common;
BlockDriverState *commit_top_bs;
@@ -130,26 +145,45 @@ static void commit_clean(Job *job)
static int coroutine_fn
commit_iteration(CommitBlockJob *s, int64_t offset,
- int64_t *requested_bytes, void *buf)
+ int64_t *requested_bytes, void *buf, CommitStatus *st)
{
BlockErrorAction action;
- int64_t bytes = *requested_bytes;
+ int64_t bytes;
int ret = 0;
bool error_in_source = true;
- /* Copy if allocated above the base */
- WITH_GRAPH_RDLOCK_GUARD() {
- ret = bdrv_co_common_block_status_above(blk_bs(s->top),
- s->base_overlay, true, BDRV_WANT_PRECISE, offset,
- COMMIT_BUFFER_SIZE, &bytes, NULL, NULL, NULL);
+ assert(offset >= st->offset);
+
+ if (offset >= st->end) {
+ /* Copy if allocated above the base */
+ WITH_GRAPH_RDLOCK_GUARD() {
+ ret = bdrv_co_common_block_status_above(blk_bs(s->top),
+ s->base_overlay, true, BDRV_WANT_PRECISE, offset,
+ st->len - offset, &bytes, NULL, NULL, NULL);
+ }
+
+ if (ret < 0) {
+ trace_commit_one_iteration(s, offset, 0, ret);
+ goto fail;
+ }
+
+ st->offset = offset;
+ st->end = offset + bytes;
+ st->ret = ret;
}
- trace_commit_one_iteration(s, offset, bytes, ret);
+ ret = st->ret;
+ bytes = st->end - offset;
- if (ret < 0) {
- goto fail;
+ /* An unallocated span costs no I/O, so it is crossed in one step. */
+ if (ret & BDRV_BLOCK_ZERO) {
+ bytes = MIN(bytes, COMMIT_ZERO_CHUNK);
+ } else if (ret & BDRV_BLOCK_ALLOCATED) {
+ bytes = MIN(bytes, COMMIT_BUFFER_SIZE);
}
+ trace_commit_one_iteration(s, offset, bytes, ret);
+
if (ret & BDRV_BLOCK_ALLOCATED) {
if (ret & BDRV_BLOCK_ZERO) {
/*
@@ -215,6 +249,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
int64_t n = 0; /* bytes */
QEMU_AUTO_VFREE void *buf = NULL;
int64_t len, base_len;
+ CommitStatus st = { 0 };
len = blk_co_getlength(s->top);
if (len < 0) {
@@ -235,6 +270,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
}
buf = blk_blockalign(s->top, COMMIT_BUFFER_SIZE);
+ st.len = len;
for (offset = 0; offset < len; offset += n) {
/* Note that even when no rate limit is applied we need to yield
@@ -245,7 +281,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
break;
}
- ret = commit_iteration(s, offset, &n, buf);
+ ret = commit_iteration(s, offset, &n, buf, &st);
if (ret < 0) {
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 04/12] iotests/124: cover backup of zero clusters and holes
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
` (2 preceding siblings ...)
2026-10-30 1:06 ` [PATCH v4 03/12] block/commit: batch block-status queries Denis V. Lunev
@ 2026-10-30 1:06 ` Denis V. Lunev
2026-10-30 1:06 ` [PATCH v4 05/12] block/block-copy: extract block_copy_set_task_method() Denis V. Lunev
` (8 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Denis V. Lunev @ 2026-10-30 1:06 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Denis V. Lunev, Andrey Drobyshev,
Vladimir Sementsov-Ogievskiy, John Snow
From: Denis V. Lunev <den@openvz.org>
The backup tests compare content only, and their sources hold data
alone, so nothing pins how backup treats zero clusters and holes. Back
up a source mixing data, write-zero and holes with sync=full, sync=top
and sync=bitmap, and check the target with qemu-img map, which tells a
copied cluster apart from a write-zero and from an untouched one.
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: John Snow <jsnow@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
tests/qemu-iotests/124 | 347 ++++++++++++++++++++++++++++++++++++-
tests/qemu-iotests/124.out | 4 +-
2 files changed, 348 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index ab9ea4d8b5..7afcf61324 100755
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -22,8 +22,11 @@
#
import os
+from collections import namedtuple
+
import iotests
-from iotests import try_remove
+from iotests import (compare_images, qemu_img_create, qemu_img_map, qemu_io,
+ try_remove)
from qemu.qmp.qmp_client import ExecuteError
@@ -748,6 +751,348 @@ class TestIncrementalBackupBlkdebug(TestIncrementalBackupBase):
self.check_backups()
+# Backup of sources mixing data, zero clusters and holes.
+
+Extent = namedtuple('Extent', ['start', 'length', 'kind'])
+Case = namedtuple('Case', ['size', 'layout', 'expected'], defaults=[None])
+
+source_img = os.path.join(iotests.test_dir, 'source')
+base_img = os.path.join(iotests.test_dir, 'base')
+target_img = os.path.join(iotests.test_dir, 'target')
+
+SIZE = 64 * 1024 * 1024
+MB = 1024 * 1024
+CLUSTER = 64 * 1024
+
+# Data, write-zero and holes, every combination at a cluster boundary.
+MIXED = Case(SIZE, [
+ (0, MB, 'data'),
+ (2 * MB, MB, 'zero'),
+ (5 * MB, 2 * MB, 'data'),
+ (8 * MB, MB, 'zero'),
+ (10 * MB, MB // 2, 'data'),
+ (20 * MB, 4 * MB, 'zero'),
+ (30 * MB, MB, 'data'),
+])
+
+# Single-cluster runs, and a zero run ending exactly at EOF.
+BOUNDARY_SIZE = 16 * MB
+BOUNDARY = Case(BOUNDARY_SIZE, [
+ (0, CLUSTER, 'zero'),
+ (CLUSTER, CLUSTER, 'data'),
+ (2 * CLUSTER, CLUSTER, 'zero'),
+ (3 * CLUSTER, CLUSTER, 'data'),
+ (4 * CLUSTER, 8 * MB, 'zero'),
+ (4 * CLUSTER + 8 * MB, CLUSTER, 'data'),
+ (BOUNDARY_SIZE - CLUSTER, CLUSTER, 'zero'),
+])
+
+# A zero run past the old block_copy_chunk_size() 16M cap.
+LARGE_ZERO = Case(48 * MB, [
+ (0, MB, 'data'),
+ (4 * MB, 32 * MB, 'zero'),
+ (40 * MB, MB, 'data'),
+])
+
+# Image size not a multiple of the cluster size: a partial tail cluster.
+TAIL_SIZE = 4 * MB + 4096
+TAIL_CLUSTER = (TAIL_SIZE // CLUSTER) * CLUSTER
+TAIL_DATA = Case(TAIL_SIZE, [
+ (0, MB, 'data'),
+ (2 * MB, MB, 'zero'),
+ (4 * MB, TAIL_SIZE - 4 * MB, 'data'),
+])
+# The partial tail cluster falls out of zero_bitmap, so it copies as data.
+TAIL_ZERO = Case(TAIL_SIZE, [
+ (0, MB, 'data'),
+ (2 * MB, TAIL_SIZE - 2 * MB, 'zero'),
+], [
+ (0, MB, 'data'),
+ (2 * MB, TAIL_CLUSTER - 2 * MB, 'zero'),
+ (TAIL_CLUSTER, TAIL_SIZE - TAIL_CLUSTER, 'data'),
+])
+
+
+def coalesce(extents):
+ out = []
+ for e in extents:
+ prev = out[-1] if out else None
+ adjacent = prev is not None and prev.start + prev.length == e.start
+ if adjacent and prev.kind == e.kind:
+ out[-1] = prev._replace(length=prev.length + e.length)
+ else:
+ out.append(e)
+ return out
+
+
+def layout_to_extents(layout, size, gap='hole'):
+ extents = []
+ pos = 0
+ for offset, length, kind in layout:
+ if offset > pos:
+ extents.append(Extent(pos, offset - pos, gap))
+ extents.append(Extent(offset, length, kind))
+ pos = offset + length
+ if pos < size:
+ extents.append(Extent(pos, size - pos, gap))
+ return coalesce(extents)
+
+
+def create_image(path, size, backing=None, opts=None):
+ args = ['-f', iotests.imgfmt]
+ if opts:
+ args += ['-o', opts]
+ if backing:
+ args += ['-b', backing, '-F', iotests.imgfmt]
+ qemu_img_create(*args, path, str(size))
+
+
+class TestBackupZeroClusters(iotests.QMPTestCase):
+ def setUp(self):
+ self.vm = iotests.VM()
+ self.vm.launch()
+
+ def tearDown(self):
+ self.vm.shutdown()
+ for img in (source_img, base_img, target_img):
+ if os.path.exists(img):
+ os.remove(img)
+
+ def hmp_write(self, drive, cmd):
+ res = self.vm.hmp_qemu_io(drive, cmd)
+ self.assertNotIn('error', res['return'].lower())
+
+ def write_layout(self, layout):
+ for offset, length, kind in layout:
+ opt = '-z' if kind == 'zero' else '-P 0x5a'
+ self.hmp_write('src', f'write {opt} {offset} {length}')
+
+ def assert_map(self, case, backing=False, gap='hole'):
+ # Content is not enough, pin the data/zero/hole split as well.
+ def classify(e):
+ is_hole = e['depth'] > 0 if backing else not e['present']
+ return 'hole' if is_hole else ('zero' if e['zero'] else 'data')
+
+ actual = coalesce([Extent(e['start'], e['length'], classify(e))
+ for e in qemu_img_map(target_img)])
+ layout = case.expected if case.expected else case.layout
+
+ self.assertEqual(actual, layout_to_extents(layout, case.size, gap))
+
+ def add_source(self, case=MIXED, backing=None, opts=None):
+ create_image(source_img, case.size, backing, opts)
+
+ self.vm.cmd('blockdev-add', {
+ 'node-name': 'src',
+ 'driver': iotests.imgfmt,
+ 'file': {'driver': 'file', 'filename': source_img},
+ })
+
+ # Write through the node, so an attached bitmap sees it.
+ self.write_layout(case.layout)
+
+ def dirty_layout(self, case):
+ # A new bitmap tracks nothing yet: dirty all, then re-apply.
+ self.vm.cmd('block-dirty-bitmap-add', node='src', name='bm0')
+ self.hmp_write('src', f'write -z 0 {case.size}')
+ self.write_layout(case.layout)
+
+ def do_backup(self, sync, case, target_backing=None, prefill=None,
+ **kwargs):
+ create_image(target_img, case.size, target_backing)
+
+ if prefill is not None:
+ # Not zero, so a skipped cluster is provably untouched.
+ qemu_io('-c', f'write -P {prefill} 0 {case.size}', target_img)
+
+ self.vm.cmd('blockdev-add', {
+ 'node-name': 'target',
+ 'driver': iotests.imgfmt,
+ 'file': {'driver': 'file', 'filename': target_img},
+ })
+
+ self.vm.cmd('blockdev-backup', device='src', target='target',
+ job_id='bk0', sync=sync, **kwargs)
+ self.wait_until_completed(drive='bk0')
+
+ self.vm.cmd('blockdev-del', node_name='target')
+ self.vm.cmd('blockdev-del', node_name='src')
+
+ def backup_and_check(self, sync, case, gap='zero', backing=False,
+ **kwargs):
+ self.do_backup(sync, case, **kwargs)
+ self.assertTrue(compare_images(source_img, target_img))
+ self.assert_map(case, backing=backing, gap=gap)
+
+ def test_full(self):
+ self.add_source()
+ self.backup_and_check('full', MIXED)
+
+ def test_full_zero_overwrite(self):
+ # full skips holes, so only check that zero overwrites prefill.
+ case = Case(SIZE, [(2 * MB, MB, 'zero')])
+ self.add_source(case)
+ self.do_backup('full', case, prefill=0xcc)
+ qemu_io('-c', f'read -P 0 {2 * MB} {MB}', target_img)
+
+ def test_bitmap(self):
+ self.add_source()
+ self.dirty_layout(MIXED)
+ self.backup_and_check('bitmap', MIXED, bitmap='bm0',
+ bitmap_mode='never')
+
+ def test_top(self):
+ # Non-zero backing data, so a hole and an explicit zero differ.
+ create_image(base_img, SIZE)
+ qemu_io('-c', f'write -P 0x33 0 {SIZE}', base_img)
+
+ self.add_source(backing=base_img)
+ self.backup_and_check('top', MIXED, gap='hole', backing=True,
+ target_backing=base_img)
+
+ def test_boundary_full(self):
+ self.add_source(BOUNDARY)
+ self.backup_and_check('full', BOUNDARY)
+
+ def test_boundary_bitmap(self):
+ self.add_source(BOUNDARY)
+ self.dirty_layout(BOUNDARY)
+ self.backup_and_check('bitmap', BOUNDARY, bitmap='bm0',
+ bitmap_mode='never')
+
+ def test_large_zero_full(self):
+ self.add_source(LARGE_ZERO)
+ self.backup_and_check('full', LARGE_ZERO)
+
+ def test_large_zero_bitmap(self):
+ self.add_source(LARGE_ZERO)
+ self.dirty_layout(LARGE_ZERO)
+ self.backup_and_check('bitmap', LARGE_ZERO, bitmap='bm0',
+ bitmap_mode='never')
+
+ def test_huge_zero(self):
+ # A zero run past the cap on a single write-zeroes request, so it
+ # has to be split. qemu-io caps one write at 2G, hence three.
+ case = Case(2560 * MB, [(0, 1024 * MB, 'zero'),
+ (1024 * MB, 1024 * MB, 'zero'),
+ (2048 * MB, 512 * MB, 'zero')])
+ self.add_source(case)
+ self.backup_and_check('full', case)
+
+ def test_tail_data(self):
+ self.add_source(TAIL_DATA)
+ self.backup_and_check('full', TAIL_DATA)
+
+ def test_tail_zero(self):
+ self.add_source(TAIL_ZERO)
+ self.backup_and_check('full', TAIL_ZERO)
+
+ def test_tail_zero_bitmap(self):
+ # Same tail rounding as test_tail_zero, via the bitmap scan.
+ self.add_source(TAIL_ZERO)
+ self.dirty_layout(TAIL_ZERO)
+ self.backup_and_check('bitmap', TAIL_ZERO, bitmap='bm0',
+ bitmap_mode='never')
+
+ def test_mixed_cluster(self):
+ # 4K source clusters, so content varies inside one 64K cluster.
+ case = Case(2 * CLUSTER, [
+ (0, 8 * 1024, 'zero'),
+ (8 * 1024, 8 * 1024, 'data'),
+ (CLUSTER, CLUSTER, 'zero'),
+ ], [
+ # A mixed cluster must be data, or the data at [8k, 16k) is lost.
+ (0, CLUSTER, 'data'),
+ (CLUSTER, CLUSTER, 'zero'),
+ ])
+ self.add_source(case, opts='cluster_size=4k')
+ self.backup_and_check('full', case)
+
+ def test_top_zero_broken(self):
+ # An overlay hole before an explicit-zero run: the zero prefix
+ # must stop at the hole, or the backing data under it is lost.
+ size = 2 * CLUSTER
+ create_image(base_img, size, opts='cluster_size=4k')
+ qemu_io('-c', f'write -P 0x33 0 {size}', base_img)
+
+ # [0, 4k) stays a hole, the zero run spills into cluster 1.
+ case = Case(size, [(4096, CLUSTER, 'zero')])
+ self.add_source(case, backing=base_img, opts='cluster_size=4k')
+
+ self.do_backup('top', case, target_backing=base_img)
+ self.assertTrue(compare_images(source_img, target_img))
+
+ def test_bitmap_straddle(self):
+ # One dirty run straddling a zero/data transition, both ways.
+ case = Case(SIZE, [])
+ self.add_source(case)
+ self.vm.cmd('block-dirty-bitmap-add', node='src', name='bm0')
+
+ # One contiguous dirty run each: [7M, 9M) zero to data, and
+ # [15M, 17M) data to zero.
+ layout = [
+ (7 * MB, MB, 'zero'),
+ (8 * MB, MB, 'data'),
+ (15 * MB, MB, 'data'),
+ (16 * MB, MB, 'zero'),
+ ]
+ self.write_layout(layout)
+
+ self.do_backup('bitmap', case, bitmap='bm0', bitmap_mode='never')
+ self.assertTrue(compare_images(source_img, target_img))
+ # Nothing outside the two dirty runs may reach the target.
+ self.assert_map(case._replace(layout=layout))
+
+ def test_bitmap_fragmented(self):
+ # Isolated single-cluster dirty spots: nothing outside them
+ # may reach the target.
+ case = Case(SIZE, [])
+ self.add_source(case)
+
+ # Written before the bitmap exists, so untracked.
+ self.hmp_write('src', f'write -P 0x5a 0 {16 * MB}')
+
+ self.vm.cmd('block-dirty-bitmap-add', node='src', name='bm0')
+
+ # The only dirty bit in the data region, with a pattern of its own.
+ data_island = 4 * MB
+ self.hmp_write('src', f'write -P 0x7b {data_island} {CLUSTER}')
+
+ # The only dirty bit in the hole region.
+ zero_island = 24 * MB
+ self.hmp_write('src', f'write -z {zero_island} {CLUSTER}')
+
+ self.do_backup('bitmap', case, bitmap='bm0', bitmap_mode='never',
+ prefill=0xcc)
+
+ # Only the two isolated spots should have reached target ...
+ qemu_io('-c', f'read -P 0x7b {data_island} {CLUSTER}', target_img)
+ qemu_io('-c', f'read -P 0 {zero_island} {CLUSTER}', target_img)
+
+ # ... everything else must still hold the prefill pattern.
+ qemu_io('-c', f'read -P 0xcc 0 {CLUSTER}', target_img)
+ qemu_io('-c', f'read -P 0xcc {8 * MB} {CLUSTER}', target_img)
+
+ def test_bitmap_clean_zero_gap(self):
+ # A clean zero run between two dirty ones must stay prefilled.
+ case = Case(16 * MB, [])
+ self.add_source(case)
+ self.hmp_write('src', f'write -z 0 {16 * MB}')
+
+ self.vm.cmd('block-dirty-bitmap-add', node='src', name='bm0')
+ self.write_layout([(2 * MB, MB, 'zero'), (6 * MB, MB, 'zero')])
+
+ self.do_backup('bitmap', case, bitmap='bm0', bitmap_mode='never',
+ prefill=0xcc)
+
+ qemu_io('-c', f'read -P 0 {2 * MB} {MB}', target_img)
+ qemu_io('-c', f'read -P 0 {6 * MB} {MB}', target_img)
+
+ qemu_io('-c', f'read -P 0xcc 0 {2 * MB}', target_img)
+ qemu_io('-c', f'read -P 0xcc {3 * MB} {3 * MB}', target_img)
+ qemu_io('-c', f'read -P 0xcc {7 * MB} {9 * MB}', target_img)
+
+
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2'],
supported_protocols=['file'],
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index fa16b5ccef..5ce2f9a2ed 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-.............
+..............................
----------------------------------------------------------------------
-Ran 13 tests
+Ran 30 tests
OK
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 05/12] block/block-copy: extract block_copy_set_task_method()
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
` (3 preceding siblings ...)
2026-10-30 1:06 ` [PATCH v4 04/12] iotests/124: cover backup of zero clusters and holes Denis V. Lunev
@ 2026-10-30 1:06 ` Denis V. Lunev
2026-10-30 1:06 ` [PATCH v4 06/12] block/block-copy: track known-zero source clusters Denis V. Lunev
` (7 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Denis V. Lunev @ 2026-10-30 1:06 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Denis V. Lunev, Andrey Drobyshev,
Vladimir Sementsov-Ogievskiy, John Snow
From: Denis V. Lunev <den@openvz.org>
Move the per-task block-status query and its skip_unallocated and
BDRV_BLOCK_ZERO handling out of block_copy_dirty_clusters() into
block_copy_set_task_method(). No behavior change. The next patch
changes that decision, and having it in one place keeps that change to
just the new logic.
The block-status result no longer lands in the caller's @ret, so drop
the note saying @ret may be positive at the out: label.
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: John Snow <jsnow@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/block-copy.c | 46 ++++++++++++++++++++++++++++++----------------
1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index 1826c2e1c7..33c43cdf92 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -736,6 +736,35 @@ int64_t coroutine_fn block_copy_reset_unallocated(BlockCopyState *s,
return ret;
}
+/*
+ * Decide how @task is copied: COPY_WRITE_ZEROES if it reads as zero. May
+ * shrink @task. Returns false if @task is to be skipped (already ended,
+ * not freed).
+ */
+static bool coroutine_fn GRAPH_RDLOCK
+block_copy_set_task_method(BlockCopyState *s, BlockCopyTask *task)
+{
+ int ret;
+ int64_t status_bytes;
+
+ ret = block_copy_block_status(s, task->req.offset, task->req.bytes,
+ &status_bytes);
+ assert(ret >= 0); /* never fail */
+ if (status_bytes < task->req.bytes) {
+ block_copy_task_shrink(task, status_bytes);
+ }
+ if (qatomic_read(&s->skip_unallocated) && !(ret & BDRV_BLOCK_ALLOCATED)) {
+ block_copy_task_end(task, 0);
+ trace_block_copy_skip_range(s, task->req.offset, task->req.bytes);
+ return false;
+ }
+ if (ret & BDRV_BLOCK_ZERO) {
+ task->method = COPY_WRITE_ZEROES;
+ }
+
+ return true;
+}
+
/*
* block_copy_dirty_clusters
*
@@ -768,7 +797,6 @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
while (bytes && aio_task_pool_status(aio) == 0 &&
!qatomic_read(&call_state->cancelled)) {
BlockCopyTask *task;
- int64_t status_bytes;
task = block_copy_task_create(s, call_state, offset, bytes);
if (!task) {
@@ -782,24 +810,12 @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
found_dirty = true;
- ret = block_copy_block_status(s, task->req.offset, task->req.bytes,
- &status_bytes);
- assert(ret >= 0); /* never fail */
- if (status_bytes < task->req.bytes) {
- block_copy_task_shrink(task, status_bytes);
- }
- if (qatomic_read(&s->skip_unallocated) &&
- !(ret & BDRV_BLOCK_ALLOCATED)) {
- block_copy_task_end(task, 0);
- trace_block_copy_skip_range(s, task->req.offset, task->req.bytes);
+ if (!block_copy_set_task_method(s, task)) {
offset = task_end(task);
bytes = end - offset;
g_free(task);
continue;
}
- if (ret & BDRV_BLOCK_ZERO) {
- task->method = COPY_WRITE_ZEROES;
- }
if (!call_state->ignore_ratelimit) {
uint64_t ns = ratelimit_calculate_delay(&s->rate_limit, 0);
@@ -840,8 +856,6 @@ out:
* block_copy_task_run. If it fails, it means some task already failed
* for real reason, let's return first failure.
* Still, assert that we don't rewrite failure by success.
- *
- * Note: ret may be positive here because of block-status result.
*/
assert(ret >= 0 || aio_task_pool_status(aio) < 0);
ret = aio_task_pool_status(aio);
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 06/12] block/block-copy: track known-zero source clusters
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
` (4 preceding siblings ...)
2026-10-30 1:06 ` [PATCH v4 05/12] block/block-copy: extract block_copy_set_task_method() Denis V. Lunev
@ 2026-10-30 1:06 ` Denis V. Lunev
2026-10-30 1:06 ` [PATCH v4 07/12] block/backup: pre-fill zero_bitmap for full/bitmap Denis V. Lunev
` (6 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Denis V. Lunev @ 2026-10-30 1:06 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Denis V. Lunev, Andrey Drobyshev,
Vladimir Sementsov-Ogievskiy, John Snow
From: Denis V. Lunev <den@openvz.org>
block_copy_reset_unallocated()'s up-front scan already queries the
source, and the same query reports zero-ness. The copy loop re-queries
per task and, having no answer up front, can only size a task by the
copy buffer, so a mostly-zero image is cut into thousands of pieces
that each turn out to read as zero.
Cache the answer in zero_bitmap, published by zero_bitmap_valid once
the scan is done. block_copy_task_create() then picks the method from
the bitmap and ends the task where the answer changes, so a zero task
never reaches into data and a copy task never swallows a zero run. The
per-task query stays as the fallback until the flag is set, since CBW
intercepts guest writes while the scan is still running.
The scan and the query it replaces must resolve BDRV_BLOCK_ZERO against
the same part of the chain, so the scan moves from bdrv_co_is_allocated()
to bdrv_co_block_status_above() and the base selection is factored out
into block_copy_status_base().
zero_bitmap is a plain HBitmap, not a BdrvDirtyBitmap: an internal cache
has no business in query-named-block-nodes. One writer and readers gated
by the flag need no mutex, but the publish needs ordering, hence
store-release and load-acquire. It is allocated on demand, so sync=none
and a standalone copy-before-write filter do not pay for it.
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: John Snow <jsnow@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/backup.c | 1 +
block/block-copy.c | 108 ++++++++++++++++++++++++++++++++-----
include/block/block-copy.h | 1 +
3 files changed, 98 insertions(+), 12 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index d4713fa1cd..11d70243e2 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -280,6 +280,7 @@ static int coroutine_fn backup_run(Job *job, Error **errp)
offset += count;
}
block_copy_set_skip_unallocated(s->bcs, false);
+ block_copy_set_zero_bitmap_valid(s->bcs);
}
if (s->sync_mode == MIRROR_SYNC_MODE_NONE) {
diff --git a/block/block-copy.c b/block/block-copy.c
index 33c43cdf92..507c4bab90 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -20,6 +20,8 @@
#include "block/block_int-io.h"
#include "block/dirty-bitmap.h"
#include "block/reqlist.h"
+#include "qemu/hbitmap.h"
+#include "qemu/host-utils.h"
#include "system/block-backend.h"
#include "qemu/units.h"
#include "qemu/co-shared-resource.h"
@@ -157,6 +159,10 @@ typedef struct BlockCopyState {
bool skip_unallocated; /* atomic */
/* State fields that use a thread-safe API */
BdrvDirtyBitmap *copy_bitmap;
+ /* Clusters reading as zero; allocated on demand, frozen once valid. */
+ HBitmap *zero_bitmap;
+ /* Published only after the scan, with skip_unallocated already false. */
+ bool zero_bitmap_valid; /* atomic, store-release/load-acquire */
ProgressMeter *progress;
SharedResource *mem;
RateLimit rate_limit;
@@ -190,6 +196,7 @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state,
int64_t offset, int64_t bytes)
{
BlockCopyTask *task;
+ BlockCopyMethod method;
int64_t max_chunk;
QEMU_LOCK_GUARD(&s->lock);
@@ -201,6 +208,27 @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state,
return NULL;
}
+ method = s->method;
+
+ /*
+ * The scan already knows how this range reads: pick the method here and
+ * stop the task where the answer changes.
+ */
+ if (qatomic_load_acquire(&s->zero_bitmap_valid)) {
+ int64_t boundary;
+
+ if (hbitmap_get(s->zero_bitmap, offset)) {
+ method = COPY_WRITE_ZEROES;
+ boundary = hbitmap_next_zero(s->zero_bitmap, offset, bytes);
+ } else {
+ boundary = hbitmap_next_dirty(s->zero_bitmap, offset, bytes);
+ }
+
+ if (boundary >= 0) {
+ bytes = boundary - offset;
+ }
+ }
+
assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
bytes = QEMU_ALIGN_UP(bytes, s->cluster_size);
@@ -215,7 +243,7 @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state,
.task.func = block_copy_task_entry,
.s = s,
.call_state = call_state,
- .method = s->method,
+ .method = method,
};
reqlist_init_req(&s->reqs, &task->req, offset, bytes);
@@ -271,6 +299,9 @@ void block_copy_state_free(BlockCopyState *s)
ratelimit_destroy(&s->rate_limit);
bdrv_release_dirty_bitmap(s->copy_bitmap);
+ if (s->zero_bitmap) {
+ hbitmap_free(s->zero_bitmap);
+ }
shres_destroy(s->mem);
g_free(s);
}
@@ -619,20 +650,24 @@ static coroutine_fn int block_copy_task_entry(AioTask *task)
return ret;
}
+/* The scan and the per-task query must resolve BDRV_BLOCK_ZERO alike. */
+static GRAPH_RDLOCK BlockDriverState *block_copy_status_base(BlockCopyState *s)
+{
+ if (qatomic_read(&s->skip_unallocated)) {
+ return bdrv_backing_chain_next(s->source->bs);
+ }
+
+ return NULL;
+}
+
static coroutine_fn GRAPH_RDLOCK
int block_copy_block_status(BlockCopyState *s, int64_t offset, int64_t bytes,
int64_t *pnum)
{
int64_t num;
- BlockDriverState *base;
+ BlockDriverState *base = block_copy_status_base(s);
int ret;
- if (qatomic_read(&s->skip_unallocated)) {
- base = bdrv_backing_chain_next(s->source->bs);
- } else {
- base = NULL;
- }
-
ret = bdrv_co_block_status_above(s->source->bs, base, offset, bytes, &num,
NULL, NULL);
if (ret < 0 || num < s->cluster_size) {
@@ -652,16 +687,41 @@ int block_copy_block_status(BlockCopyState *s, int64_t offset, int64_t bytes,
return ret;
}
+/* Only the scan allocates, and it runs before zero_bitmap_valid. */
+static HBitmap *block_copy_zero_bitmap(BlockCopyState *s)
+{
+ if (!s->zero_bitmap) {
+ s->zero_bitmap = hbitmap_alloc(s->len, ctz32(s->cluster_size));
+ }
+
+ return s->zero_bitmap;
+}
+
+static void block_copy_mark_zero_prefix(BlockCopyState *s, int64_t offset,
+ int64_t zero_count)
+{
+ int64_t zero_bytes = QEMU_ALIGN_DOWN(zero_count, s->cluster_size);
+
+ if (zero_bytes > 0) {
+ hbitmap_set(block_copy_zero_bitmap(s), offset, zero_bytes);
+ }
+}
+
/*
* Check if the cluster starting at offset is allocated or not.
* return via pnum the number of contiguous clusters sharing this allocation.
+ * Also marks the zero prefix of the range in zero_bitmap.
*/
static int coroutine_fn GRAPH_RDLOCK
block_copy_is_cluster_allocated(BlockCopyState *s, int64_t offset,
int64_t *pnum)
{
BlockDriverState *bs = s->source->bs;
+ BlockDriverState *base = block_copy_status_base(s);
+ int64_t orig_offset = offset;
int64_t count, total_count = 0;
+ int64_t zero_count = 0;
+ bool zero_broken = false;
int64_t bytes = s->len - offset;
int ret;
@@ -669,25 +729,37 @@ block_copy_is_cluster_allocated(BlockCopyState *s, int64_t offset,
while (true) {
/* protected in backup_run() */
- ret = bdrv_co_is_allocated(bs, offset, bytes, &count);
+ ret = bdrv_co_block_status_above(bs, base, offset, bytes, &count,
+ NULL, NULL);
if (ret < 0) {
return ret;
}
+ if (!zero_broken) {
+ if (ret & BDRV_BLOCK_ZERO) {
+ zero_count += count;
+ } else {
+ zero_broken = true;
+ }
+ }
+
total_count += count;
- if (ret || count == 0) {
+ if ((ret & BDRV_BLOCK_ALLOCATED) || count == 0) {
/*
- * ret: partial segment(s) are considered allocated.
+ * BDRV_BLOCK_ALLOCATED: partial segment(s) are considered
+ * allocated.
* otherwise: unallocated tail is treated as an entire segment.
*/
*pnum = DIV_ROUND_UP(total_count, s->cluster_size);
- return ret;
+ block_copy_mark_zero_prefix(s, orig_offset, zero_count);
+ return !!(ret & BDRV_BLOCK_ALLOCATED);
}
/* Unallocated segment(s) with uncertain following segment(s) */
if (total_count >= s->cluster_size) {
*pnum = total_count / s->cluster_size;
+ block_copy_mark_zero_prefix(s, orig_offset, zero_count);
return 0;
}
@@ -747,6 +819,12 @@ block_copy_set_task_method(BlockCopyState *s, BlockCopyTask *task)
int ret;
int64_t status_bytes;
+ /* block_copy_task_create() already decided, from zero_bitmap. */
+ if (qatomic_load_acquire(&s->zero_bitmap_valid)) {
+ return true;
+ }
+
+ /* CBW filter could call this early. */
ret = block_copy_block_status(s, task->req.offset, task->req.bytes,
&status_bytes);
assert(ret >= 0); /* never fail */
@@ -1076,6 +1154,12 @@ void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip)
qatomic_set(&s->skip_unallocated, skip);
}
+void block_copy_set_zero_bitmap_valid(BlockCopyState *s)
+{
+ block_copy_zero_bitmap(s);
+ qatomic_store_release(&s->zero_bitmap_valid, true);
+}
+
void block_copy_set_speed(BlockCopyState *s, uint64_t speed)
{
ratelimit_set_speed(&s->rate_limit, speed, BLOCK_COPY_SLICE_TIME);
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index 0df2771181..e4e5b56753 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -101,5 +101,6 @@ void block_copy_call_cancel(BlockCopyCallState *call_state);
BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s);
int64_t block_copy_cluster_size(BlockCopyState *s);
void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip);
+void block_copy_set_zero_bitmap_valid(BlockCopyState *s);
#endif /* BLOCK_COPY_H */
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 07/12] block/backup: pre-fill zero_bitmap for full/bitmap
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
` (5 preceding siblings ...)
2026-10-30 1:06 ` [PATCH v4 06/12] block/block-copy: track known-zero source clusters Denis V. Lunev
@ 2026-10-30 1:06 ` Denis V. Lunev
2026-10-30 1:06 ` [PATCH v4 08/12] block/block-copy: rename block_copy_is_cluster_allocated() Denis V. Lunev
` (5 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Denis V. Lunev @ 2026-10-30 1:06 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Denis V. Lunev, Andrey Drobyshev,
Vladimir Sementsov-Ogievskiy, John Snow
From: Denis V. Lunev <den@openvz.org>
Only sync=top runs an up-front scan, so the previous patch did nothing
for the other modes. FULL and BITMAP know their complete copy_bitmap
before copying starts, and the same copy-before-write guarantee holds:
a cluster still dirty when the copy loop reaches it has not been written
since the scan.
Add block_copy_calculate_zero_bitmap(), driven one query at a time from
backup_scan_source() so the graph rdlock and the pause point are taken
per query, and query each confirmed span once however many dirty areas
it covers. Each step searches copy_bitmap only a bounded window ahead,
since for sync=full the rest of the image is one dirty area. The scan
only fills zero_bitmap and never clears copy_bitmap, so sync=full still
writes zeroes for clusters the source never allocated; it just stops
re-querying to find that out. Unlike the per-task queries it replaces
the scan is serial, so a sparsely dirtied job against a slow source may
spend more time here than it saves. The sync=top loop had the same
shape, so both fold into backup_scan_source().
sync=none keeps the per-task query. Its bitmap says "anything may be
copied", not "this will be copied", and it only ever copies what the
guest writes during the fleecing window, so scanning the whole image at
attach is the wrong trade.
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: John Snow <jsnow@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/backup.c | 94 ++++++++++++++++++++++++--------------
block/block-copy.c | 42 +++++++++++++++++
include/block/block-copy.h | 4 ++
3 files changed, 106 insertions(+), 34 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 11d70243e2..38b0838d69 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -247,59 +247,85 @@ static void backup_init_bcs_bitmap(BackupBlockJob *job)
job_progress_set_remaining(&job->common.job, estimate);
}
-static int coroutine_fn backup_run(Job *job, Error **errp)
+/*
+ * Walk the source before copying starts: fill zero_bitmap and, for sync=top,
+ * drop the clusters that are not allocated above the backing.
+ */
+static int coroutine_fn backup_scan_source(BackupBlockJob *s)
{
- BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
- int ret;
-
- backup_init_bcs_bitmap(s);
-
- if (s->sync_mode == MIRROR_SYNC_MODE_TOP) {
- int64_t offset = 0;
- int64_t count;
+ Job *job = &s->common.job;
+ bool top = s->sync_mode == MIRROR_SYNC_MODE_TOP;
+ int64_t offset, cached_end = 0, count;
+ int ret = 0;
- for (offset = 0; offset < s->len; ) {
- if (job_is_cancelled(job)) {
- return -ECANCELED;
- }
+ for (offset = 0; offset < s->len; offset += count) {
+ if (job_is_cancelled(job)) {
+ return -ECANCELED;
+ }
- job_pause_point(job);
+ job_pause_point(job);
- if (job_is_cancelled(job)) {
- return -ECANCELED;
- }
+ if (job_is_cancelled(job)) {
+ return -ECANCELED;
+ }
- /* rdlock protects the subsequent call to bdrv_is_allocated() */
- bdrv_graph_co_rdlock();
+ /* rdlock protects the block-status queries below */
+ bdrv_graph_co_rdlock();
+ if (top) {
ret = block_copy_reset_unallocated(s->bcs, offset, &count);
- bdrv_graph_co_rdunlock();
- if (ret < 0) {
- return ret;
- }
+ } else {
+ block_copy_calculate_zero_bitmap(s->bcs, offset, &cached_end,
+ &count);
+ }
+ bdrv_graph_co_rdunlock();
- offset += count;
+ if (ret < 0) {
+ return ret;
}
+ }
+
+ if (top) {
block_copy_set_skip_unallocated(s->bcs, false);
- block_copy_set_zero_bitmap_valid(s->bcs);
}
+ block_copy_set_zero_bitmap_valid(s->bcs);
- if (s->sync_mode == MIRROR_SYNC_MODE_NONE) {
+ return 0;
+}
+
+static int coroutine_fn backup_run(Job *job, Error **errp)
+{
+ BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
+ int ret;
+
+ backup_init_bcs_bitmap(s);
+
+ switch (s->sync_mode) {
+ case MIRROR_SYNC_MODE_TOP:
+ case MIRROR_SYNC_MODE_FULL:
+ case MIRROR_SYNC_MODE_BITMAP:
+ ret = backup_scan_source(s);
+ if (ret < 0) {
+ return ret;
+ }
+ break;
+
+ case MIRROR_SYNC_MODE_NONE:
/*
* All bits are set in bcs bitmap to allow any cluster to be copied.
- * This does not actually require them to be copied.
+ * This does not actually require them to be copied. Yield until the
+ * job is cancelled and let the before_write notify callback service
+ * CoW requests.
*/
while (!job_is_cancelled(job)) {
- /*
- * Yield until the job is cancelled. We just let our before_write
- * notify callback service CoW requests.
- */
job_yield(job);
}
- } else {
- return backup_loop(s);
+ return 0;
+
+ default:
+ g_assert_not_reached();
}
- return 0;
+ return backup_loop(s);
}
static void coroutine_fn backup_pause(Job *job)
diff --git a/block/block-copy.c b/block/block-copy.c
index 507c4bab90..4f128a0ebd 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -808,6 +808,48 @@ int64_t coroutine_fn block_copy_reset_unallocated(BlockCopyState *s,
return ret;
}
+/*
+ * One scan step, like block_copy_reset_unallocated(); @cached_end
+ * tracks progress across calls, @count how far @offset should move.
+ */
+void coroutine_fn GRAPH_RDLOCK
+block_copy_calculate_zero_bitmap(BlockCopyState *s, int64_t offset,
+ int64_t *cached_end, int64_t *count)
+{
+ int64_t max_bytes = QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
+ s->cluster_size);
+ int64_t dirty_offset, dirty_bytes, dirty_end;
+ bool found;
+
+ /* CBW mutates copy_bitmap from its own AioContext meanwhile. */
+ WITH_QEMU_LOCK_GUARD(&s->lock) {
+ found = bdrv_dirty_bitmap_next_dirty_area(s->copy_bitmap, offset,
+ s->len, max_bytes,
+ &dirty_offset, &dirty_bytes);
+ }
+
+ if (!found) {
+ *count = s->len - offset;
+ return;
+ }
+ dirty_end = dirty_offset + dirty_bytes;
+
+ if (*cached_end < dirty_end) {
+ int64_t clusters;
+ int64_t query_offset = MAX(dirty_offset, *cached_end);
+ int ret = block_copy_is_cluster_allocated(s, query_offset, &clusters);
+
+ if (ret >= 0) {
+ *cached_end = query_offset + clusters * s->cluster_size;
+ } else {
+ /* Best-effort: leave this area unmarked; it copies as data. */
+ *cached_end = dirty_end;
+ }
+ }
+
+ *count = MIN(*cached_end, dirty_end) - offset;
+}
+
/*
* Decide how @task is copied: COPY_WRITE_ZEROES if it reads as zero. May
* shrink @task. Returns false if @task is to be skipped (already ended,
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index e4e5b56753..3a61066b55 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -44,6 +44,10 @@ void block_copy_reset(BlockCopyState *s, int64_t offset, int64_t bytes);
int64_t coroutine_fn GRAPH_RDLOCK
block_copy_reset_unallocated(BlockCopyState *s, int64_t offset, int64_t *count);
+void coroutine_fn GRAPH_RDLOCK
+block_copy_calculate_zero_bitmap(BlockCopyState *s, int64_t offset,
+ int64_t *cached_end, int64_t *count);
+
int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
bool ignore_ratelimit, uint64_t timeout_ns,
BlockCopyAsyncCallbackFunc cb,
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 08/12] block/block-copy: rename block_copy_is_cluster_allocated()
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
` (6 preceding siblings ...)
2026-10-30 1:06 ` [PATCH v4 07/12] block/backup: pre-fill zero_bitmap for full/bitmap Denis V. Lunev
@ 2026-10-30 1:06 ` Denis V. Lunev
2026-10-30 1:06 ` [PATCH v4 09/12] block/block-copy: merge the two backup scan steps into block_copy_prescan() Denis V. Lunev
` (4 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Denis V. Lunev @ 2026-10-30 1:06 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Denis V. Lunev, Vladimir Sementsov-Ogievskiy,
John Snow, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
Since it started filling zero_bitmap, the function does more than its
name says: it is the scan's block-status query, reporting allocation and
recording zeroes. Rename it to block_copy_scan_status() and say so in
its comment. No behavior change.
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: John Snow <jsnow@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/block-copy.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index 4f128a0ebd..ad358f4d2f 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -708,13 +708,13 @@ static void block_copy_mark_zero_prefix(BlockCopyState *s, int64_t offset,
}
/*
- * Check if the cluster starting at offset is allocated or not.
- * return via pnum the number of contiguous clusters sharing this allocation.
- * Also marks the zero prefix of the range in zero_bitmap.
+ * One block-status query of the scan at @offset: return 1 if the cluster
+ * there is allocated, 0 if not, -errno on failure, and via @pnum the number
+ * of contiguous clusters sharing that allocation. Their zero prefix goes
+ * into zero_bitmap.
*/
static int coroutine_fn GRAPH_RDLOCK
-block_copy_is_cluster_allocated(BlockCopyState *s, int64_t offset,
- int64_t *pnum)
+block_copy_scan_status(BlockCopyState *s, int64_t offset, int64_t *pnum)
{
BlockDriverState *bs = s->source->bs;
BlockDriverState *base = block_copy_status_base(s);
@@ -793,7 +793,7 @@ int64_t coroutine_fn block_copy_reset_unallocated(BlockCopyState *s,
int ret;
int64_t clusters, bytes;
- ret = block_copy_is_cluster_allocated(s, offset, &clusters);
+ ret = block_copy_scan_status(s, offset, &clusters);
if (ret < 0) {
return ret;
}
@@ -837,7 +837,7 @@ block_copy_calculate_zero_bitmap(BlockCopyState *s, int64_t offset,
if (*cached_end < dirty_end) {
int64_t clusters;
int64_t query_offset = MAX(dirty_offset, *cached_end);
- int ret = block_copy_is_cluster_allocated(s, query_offset, &clusters);
+ int ret = block_copy_scan_status(s, query_offset, &clusters);
if (ret >= 0) {
*cached_end = query_offset + clusters * s->cluster_size;
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 09/12] block/block-copy: merge the two backup scan steps into block_copy_prescan()
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
` (7 preceding siblings ...)
2026-10-30 1:06 ` [PATCH v4 08/12] block/block-copy: rename block_copy_is_cluster_allocated() Denis V. Lunev
@ 2026-10-30 1:06 ` Denis V. Lunev
2026-11-03 3:26 ` Eric Blake
2026-10-30 1:06 ` [PATCH v4 10/12] block/block-copy: replace zero_bitmap_valid with scan_done Denis V. Lunev
` (3 subsequent siblings)
12 siblings, 1 reply; 18+ messages in thread
From: Denis V. Lunev @ 2026-10-30 1:06 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Denis V. Lunev, Vladimir Sementsov-Ogievskiy,
John Snow, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
block_copy_reset_unallocated() and block_copy_calculate_zero_bitmap()
are both one step of the same scan and differ only in what they do with
the answer. Fold them into block_copy_prescan(), which drops unallocated
clusters from copy_bitmap while skip_unallocated is set, so
backup_scan_source() loses its per-mode branch.
sync=top now walks the dirty areas the way full and bitmap do, so it no
longer queries what copy-before-write has already copied during the
scan. A failed query still fails sync=top, as nothing else can tell
which clusters to skip, and still leaves the area to be copied as data
otherwise.
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: John Snow <jsnow@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/backup.c | 7 +----
block/block-copy.c | 63 ++++++++++++++------------------------
include/block/block-copy.h | 9 ++----
3 files changed, 27 insertions(+), 52 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 38b0838d69..60c00fca67 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -271,12 +271,7 @@ static int coroutine_fn backup_scan_source(BackupBlockJob *s)
/* rdlock protects the block-status queries below */
bdrv_graph_co_rdlock();
- if (top) {
- ret = block_copy_reset_unallocated(s->bcs, offset, &count);
- } else {
- block_copy_calculate_zero_bitmap(s->bcs, offset, &cached_end,
- &count);
- }
+ ret = block_copy_prescan(s->bcs, offset, &cached_end, &count);
bdrv_graph_co_rdunlock();
if (ret < 0) {
diff --git a/block/block-copy.c b/block/block-copy.c
index ad358f4d2f..78cc855b5b 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -153,8 +153,8 @@ typedef struct BlockCopyState {
* This is indicated by skip_unallocated.
*
* In this case, block_copy() will query the source’s allocation status,
- * skip unallocated regions, clear them in the copy_bitmap, and invoke
- * block_copy_reset_unallocated() every time it does.
+ * skip unallocated regions and clear them in the copy_bitmap, as
+ * block_copy_prescan() does.
*/
bool skip_unallocated; /* atomic */
/* State fields that use a thread-safe API */
@@ -781,40 +781,17 @@ void block_copy_reset(BlockCopyState *s, int64_t offset, int64_t bytes)
}
/*
- * Reset bits in copy_bitmap starting at offset if they represent unallocated
- * data in the image. May reset subsequent contiguous bits.
- * @return 0 when the cluster at @offset was unallocated,
- * 1 otherwise, and -ret on error.
- */
-int64_t coroutine_fn block_copy_reset_unallocated(BlockCopyState *s,
- int64_t offset,
- int64_t *count)
-{
- int ret;
- int64_t clusters, bytes;
-
- ret = block_copy_scan_status(s, offset, &clusters);
- if (ret < 0) {
- return ret;
- }
-
- bytes = clusters * s->cluster_size;
-
- if (!ret) {
- block_copy_reset(s, offset, bytes);
- }
-
- *count = bytes;
- return ret;
-}
-
-/*
- * One scan step, like block_copy_reset_unallocated(); @cached_end
- * tracks progress across calls, @count how far @offset should move.
+ * One step of the scan run before copying: query the next dirty area, fill
+ * zero_bitmap and, while skip_unallocated is set, drop the clusters which
+ * are not allocated above the backing from copy_bitmap. @cached_end tracks
+ * progress across calls, @count how far @offset should move.
+ *
+ * A failed query only leaves the area unmarked, so it copies as data, but
+ * sync=top cannot tell what to skip and fails instead.
*/
-void coroutine_fn GRAPH_RDLOCK
-block_copy_calculate_zero_bitmap(BlockCopyState *s, int64_t offset,
- int64_t *cached_end, int64_t *count)
+int coroutine_fn GRAPH_RDLOCK
+block_copy_prescan(BlockCopyState *s, int64_t offset, int64_t *cached_end,
+ int64_t *count)
{
int64_t max_bytes = QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
s->cluster_size);
@@ -830,7 +807,7 @@ block_copy_calculate_zero_bitmap(BlockCopyState *s, int64_t offset,
if (!found) {
*count = s->len - offset;
- return;
+ return 0;
}
dirty_end = dirty_offset + dirty_bytes;
@@ -839,15 +816,21 @@ block_copy_calculate_zero_bitmap(BlockCopyState *s, int64_t offset,
int64_t query_offset = MAX(dirty_offset, *cached_end);
int ret = block_copy_scan_status(s, query_offset, &clusters);
- if (ret >= 0) {
- *cached_end = query_offset + clusters * s->cluster_size;
- } else {
- /* Best-effort: leave this area unmarked; it copies as data. */
+ if (ret < 0) {
+ if (qatomic_read(&s->skip_unallocated)) {
+ return ret;
+ }
*cached_end = dirty_end;
+ } else {
+ *cached_end = query_offset + clusters * s->cluster_size;
+ if (!ret && qatomic_read(&s->skip_unallocated)) {
+ block_copy_reset(s, query_offset, clusters * s->cluster_size);
+ }
}
}
*count = MIN(*cached_end, dirty_end) - offset;
+ return 0;
}
/*
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index 3a61066b55..dc51316fcf 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -41,12 +41,9 @@ void block_copy_state_free(BlockCopyState *s);
void block_copy_reset(BlockCopyState *s, int64_t offset, int64_t bytes);
-int64_t coroutine_fn GRAPH_RDLOCK
-block_copy_reset_unallocated(BlockCopyState *s, int64_t offset, int64_t *count);
-
-void coroutine_fn GRAPH_RDLOCK
-block_copy_calculate_zero_bitmap(BlockCopyState *s, int64_t offset,
- int64_t *cached_end, int64_t *count);
+int coroutine_fn GRAPH_RDLOCK
+block_copy_prescan(BlockCopyState *s, int64_t offset, int64_t *cached_end,
+ int64_t *count);
int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
bool ignore_ratelimit, uint64_t timeout_ns,
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 09/12] block/block-copy: merge the two backup scan steps into block_copy_prescan()
2026-10-30 1:06 ` [PATCH v4 09/12] block/block-copy: merge the two backup scan steps into block_copy_prescan() Denis V. Lunev
@ 2026-11-03 3:26 ` Eric Blake
0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2026-11-03 3:26 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, Vladimir Sementsov-Ogievskiy, John Snow,
Andrey Drobyshev
On Fri, Oct 30, 2026 at 02:06:09AM +0100, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
>
> block_copy_reset_unallocated() and block_copy_calculate_zero_bitmap()
> are both one step of the same scan and differ only in what they do with
> the answer. Fold them into block_copy_prescan(), which drops unallocated
> clusters from copy_bitmap while skip_unallocated is set, so
> backup_scan_source() loses its per-mode branch.
>
> sync=top now walks the dirty areas the way full and bitmap do, so it no
> longer queries what copy-before-write has already copied during the
> scan. A failed query still fails sync=top, as nothing else can tell
> which clusters to skip, and still leaves the area to be copied as data
> otherwise.
>
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: John Snow <jsnow@redhat.com>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> block/backup.c | 7 +----
> block/block-copy.c | 63 ++++++++++++++------------------------
> include/block/block-copy.h | 9 ++----
> 3 files changed, 27 insertions(+), 52 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 10/12] block/block-copy: replace zero_bitmap_valid with scan_done
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
` (8 preceding siblings ...)
2026-10-30 1:06 ` [PATCH v4 09/12] block/block-copy: merge the two backup scan steps into block_copy_prescan() Denis V. Lunev
@ 2026-10-30 1:06 ` Denis V. Lunev
2026-11-03 3:27 ` Eric Blake
2026-10-30 1:06 ` [PATCH v4 11/12] util/co-shared-resource: skip the mutex for an empty request Denis V. Lunev
` (2 subsequent siblings)
12 siblings, 1 reply; 18+ messages in thread
From: Denis V. Lunev @ 2026-10-30 1:06 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Denis V. Lunev, Vladimir Sementsov-Ogievskiy,
John Snow, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
zero_bitmap_valid says more than its name: a task created once it is
set skips its own block-status query, the sync=top skipping included,
because the scan has already settled what to copy. And the end of the
scan was announced twice, clearing skip_unallocated and then publishing
zero_bitmap_valid, with an ordering rule in a comment tying the two.
Rename the flag to scan_done, and let block_copy_set_scan_done() clear
skip_unallocated before publishing it, so backup_scan_source() makes one
call and the order is kept in one place. skip_unallocated is left to
mean that a sync=top scan is in progress. No behavior change.
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: John Snow <jsnow@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/backup.c | 6 +-----
block/block-copy.c | 19 ++++++++++++-------
include/block/block-copy.h | 2 +-
3 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 60c00fca67..1bf6cbedac 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -254,7 +254,6 @@ static void backup_init_bcs_bitmap(BackupBlockJob *job)
static int coroutine_fn backup_scan_source(BackupBlockJob *s)
{
Job *job = &s->common.job;
- bool top = s->sync_mode == MIRROR_SYNC_MODE_TOP;
int64_t offset, cached_end = 0, count;
int ret = 0;
@@ -279,10 +278,7 @@ static int coroutine_fn backup_scan_source(BackupBlockJob *s)
}
}
- if (top) {
- block_copy_set_skip_unallocated(s->bcs, false);
- }
- block_copy_set_zero_bitmap_valid(s->bcs);
+ block_copy_set_scan_done(s->bcs);
return 0;
}
diff --git a/block/block-copy.c b/block/block-copy.c
index 78cc855b5b..3b7e282e5b 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -161,8 +161,12 @@ typedef struct BlockCopyState {
BdrvDirtyBitmap *copy_bitmap;
/* Clusters reading as zero; allocated on demand, frozen once valid. */
HBitmap *zero_bitmap;
- /* Published only after the scan, with skip_unallocated already false. */
- bool zero_bitmap_valid; /* atomic, store-release/load-acquire */
+ /*
+ * Set when the scan is over: zero_bitmap is complete, and for sync=top
+ * copy_bitmap holds no unallocated clusters, so a task needs no
+ * block-status query of its own.
+ */
+ bool scan_done; /* atomic, store-release/load-acquire */
ProgressMeter *progress;
SharedResource *mem;
RateLimit rate_limit;
@@ -214,7 +218,7 @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state,
* The scan already knows how this range reads: pick the method here and
* stop the task where the answer changes.
*/
- if (qatomic_load_acquire(&s->zero_bitmap_valid)) {
+ if (qatomic_load_acquire(&s->scan_done)) {
int64_t boundary;
if (hbitmap_get(s->zero_bitmap, offset)) {
@@ -687,7 +691,7 @@ int block_copy_block_status(BlockCopyState *s, int64_t offset, int64_t bytes,
return ret;
}
-/* Only the scan allocates, and it runs before zero_bitmap_valid. */
+/* Only the scan allocates, and it runs before scan_done. */
static HBitmap *block_copy_zero_bitmap(BlockCopyState *s)
{
if (!s->zero_bitmap) {
@@ -845,7 +849,7 @@ block_copy_set_task_method(BlockCopyState *s, BlockCopyTask *task)
int64_t status_bytes;
/* block_copy_task_create() already decided, from zero_bitmap. */
- if (qatomic_load_acquire(&s->zero_bitmap_valid)) {
+ if (qatomic_load_acquire(&s->scan_done)) {
return true;
}
@@ -1179,10 +1183,11 @@ void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip)
qatomic_set(&s->skip_unallocated, skip);
}
-void block_copy_set_zero_bitmap_valid(BlockCopyState *s)
+void block_copy_set_scan_done(BlockCopyState *s)
{
block_copy_zero_bitmap(s);
- qatomic_store_release(&s->zero_bitmap_valid, true);
+ qatomic_set(&s->skip_unallocated, false);
+ qatomic_store_release(&s->scan_done, true);
}
void block_copy_set_speed(BlockCopyState *s, uint64_t speed)
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index dc51316fcf..e82c96586e 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -102,6 +102,6 @@ void block_copy_call_cancel(BlockCopyCallState *call_state);
BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s);
int64_t block_copy_cluster_size(BlockCopyState *s);
void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip);
-void block_copy_set_zero_bitmap_valid(BlockCopyState *s);
+void block_copy_set_scan_done(BlockCopyState *s);
#endif /* BLOCK_COPY_H */
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 10/12] block/block-copy: replace zero_bitmap_valid with scan_done
2026-10-30 1:06 ` [PATCH v4 10/12] block/block-copy: replace zero_bitmap_valid with scan_done Denis V. Lunev
@ 2026-11-03 3:27 ` Eric Blake
0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2026-11-03 3:27 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, Vladimir Sementsov-Ogievskiy, John Snow,
Andrey Drobyshev
On Fri, Oct 30, 2026 at 02:06:10AM +0100, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
>
> zero_bitmap_valid says more than its name: a task created once it is
> set skips its own block-status query, the sync=top skipping included,
> because the scan has already settled what to copy. And the end of the
> scan was announced twice, clearing skip_unallocated and then publishing
> zero_bitmap_valid, with an ordering rule in a comment tying the two.
>
> Rename the flag to scan_done, and let block_copy_set_scan_done() clear
> skip_unallocated before publishing it, so backup_scan_source() makes one
> call and the order is kept in one place. skip_unallocated is left to
> mean that a sync=top scan is in progress. No behavior change.
>
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: John Snow <jsnow@redhat.com>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> block/backup.c | 6 +-----
> block/block-copy.c | 19 ++++++++++++-------
> include/block/block-copy.h | 2 +-
> 3 files changed, 14 insertions(+), 13 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 11/12] util/co-shared-resource: skip the mutex for an empty request
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
` (9 preceding siblings ...)
2026-10-30 1:06 ` [PATCH v4 10/12] block/block-copy: replace zero_bitmap_valid with scan_done Denis V. Lunev
@ 2026-10-30 1:06 ` Denis V. Lunev
2026-11-03 3:32 ` Eric Blake
2026-10-30 1:06 ` [PATCH v4 12/12] block/block-copy: coalesce write-zeroes tasks Denis V. Lunev
2026-11-02 16:26 ` [PATCH v4 00/12] block: cheaper zero handling in backup and commit Eric Blake
12 siblings, 1 reply; 18+ messages in thread
From: Denis V. Lunev @ 2026-10-30 1:06 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Denis V. Lunev, Vladimir Sementsov-Ogievskiy,
John Snow, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
Getting or putting nothing cannot change the pool, yet both calls take
the mutex, and co_put_to_shres() also wakes every waiter to find the
pool unchanged. Return early when n is zero.
block-copy is about to reserve nothing for write-zeroes tasks the target
serves without a buffer, which makes such calls the common case for a
mostly-zero image.
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: John Snow <jsnow@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
util/qemu-co-shared-resource.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/util/qemu-co-shared-resource.c b/util/qemu-co-shared-resource.c
index 752eb5a1c5..f9b6af0072 100644
--- a/util/qemu-co-shared-resource.c
+++ b/util/qemu-co-shared-resource.c
@@ -69,6 +69,10 @@ static bool co_try_get_from_shres_locked(SharedResource *s, uint64_t n)
void coroutine_fn co_get_from_shres(SharedResource *s, uint64_t n)
{
assert(n <= s->total);
+ if (!n) {
+ return;
+ }
+
QEMU_LOCK_GUARD(&s->lock);
while (!co_try_get_from_shres_locked(s, n)) {
qemu_co_queue_wait(&s->queue, &s->lock);
@@ -77,6 +81,10 @@ void coroutine_fn co_get_from_shres(SharedResource *s, uint64_t n)
void coroutine_fn co_put_to_shres(SharedResource *s, uint64_t n)
{
+ if (!n) {
+ return;
+ }
+
QEMU_LOCK_GUARD(&s->lock);
assert(s->total - s->available >= n);
s->available += n;
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 11/12] util/co-shared-resource: skip the mutex for an empty request
2026-10-30 1:06 ` [PATCH v4 11/12] util/co-shared-resource: skip the mutex for an empty request Denis V. Lunev
@ 2026-11-03 3:32 ` Eric Blake
0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2026-11-03 3:32 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, Vladimir Sementsov-Ogievskiy, John Snow,
Andrey Drobyshev
On Fri, Oct 30, 2026 at 02:06:11AM +0100, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
>
> Getting or putting nothing cannot change the pool, yet both calls take
> the mutex, and co_put_to_shres() also wakes every waiter to find the
> pool unchanged. Return early when n is zero.
>
> block-copy is about to reserve nothing for write-zeroes tasks the target
> serves without a buffer, which makes such calls the common case for a
> mostly-zero image.
>
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: John Snow <jsnow@redhat.com>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> util/qemu-co-shared-resource.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 12/12] block/block-copy: coalesce write-zeroes tasks
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
` (10 preceding siblings ...)
2026-10-30 1:06 ` [PATCH v4 11/12] util/co-shared-resource: skip the mutex for an empty request Denis V. Lunev
@ 2026-10-30 1:06 ` Denis V. Lunev
2026-11-03 3:37 ` Eric Blake
2026-11-02 16:26 ` [PATCH v4 00/12] block: cheaper zero handling in backup and commit Eric Blake
12 siblings, 1 reply; 18+ messages in thread
From: Denis V. Lunev @ 2026-10-30 1:06 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Denis V. Lunev, Vladimir Sementsov-Ogievskiy,
John Snow, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
Task size was capped to block_copy_chunk_size() regardless of method,
which sizes a task by the copy buffer. A write-zeroes task carries no
buffer, so that cap only splits one long known-zero run into a crowd of
small COPY_WRITE_ZEROES tasks, each with its own request against the
target.
block_copy_task_create() already decides from zero_bitmap how far a task
may run, so add block_copy_widen_zero_area() to that decision: a
write-zeroes task runs to the next clean cluster of copy_bitmap, at most
BDRV_REQUEST_MAX_BYTES aligned down to cluster_size, and the existing
clamp cuts it back to where the zero run ends. The experimental x-perf
max-chunk does not bound it, and its documentation now says so.
Widening is safe: an overlapping caller waits on the task's BlockReq via
reqlist_wait_one() rather than observing it mid-flight.
A request that large is only cheap where the target zeroes by metadata.
BDRV_REQ_NO_FALLBACK in supported_zero_flags rules out the targets which
cannot, qcow2 v2 and iscsi among them, but it is optimistic for the
rest: file-posix advertises it at open and only learns from a failing
fallocate. So a write-zeroes task asks for it while the target is
believed to oblige, which costs nothing when it does. A refusal ends the
widening and shrinks the task to one cluster, written with fallback
allowed. The rest of the range is dirty again, and the next pass of
block_copy_common() picks it up as buffer-sized tasks running in
parallel. Part of the refused range may already be zeroed, since
-ENOTSUP only says the fast path is unavailable, and writing zeroes over
it again changes nothing.
Such a task is also the only one sure to need no buffer: without
NO_FALLBACK, bdrv_co_do_pwrite_zeroes() writes the zeroes out through a
bounce buffer. So it alone is left out of the memory pool and of the
rate limit, where one widened request would otherwise hold the job back
for as long as speed= takes to pass its whole length. The one-cluster
write left after a refusal is not charged either, though it may bounce:
that is at most one cluster per worker, once, as tasks created after the
refusal are charged in full.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: John Snow <jsnow@redhat.com>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/block-copy.c | 96 +++++++++++++++++++++++++++++++++++++++-----
qapi/block-core.json | 4 +-
2 files changed, 88 insertions(+), 12 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index 3b7e282e5b..c96b9ce4f1 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -92,19 +92,21 @@ typedef struct BlockCopyTask {
BlockCopyState *s;
BlockCopyCallState *call_state;
/*
- * @method can also be set again in the while loop of
- * block_copy_dirty_clusters(), but it is never accessed concurrently
- * because the only other function that reads it is
+ * @method and @zero_fast can also be set again in the while loop of
+ * block_copy_dirty_clusters(), but they are never accessed concurrently
+ * because the only other function that reads them is
* block_copy_task_entry() and it is invoked afterwards in the same
* iteration.
*/
BlockCopyMethod method;
+ /* A write-zeroes task asking for BDRV_REQ_NO_FALLBACK. */
+ bool zero_fast;
/*
* Generally, req is protected by lock in BlockCopyState, Still req.offset
* is only set on task creation, so may be read concurrently after creation.
- * req.bytes is changed at most once, and need only protecting the case of
- * parallel read while updating @bytes value in block_copy_task_shrink().
+ * req.bytes only shrinks, and need only protecting the case of parallel
+ * read while updating @bytes value in block_copy_task_shrink().
*/
BlockReq req;
} BlockCopyTask;
@@ -159,6 +161,8 @@ typedef struct BlockCopyState {
bool skip_unallocated; /* atomic */
/* State fields that use a thread-safe API */
BdrvDirtyBitmap *copy_bitmap;
+ /* Cleared once the target refuses BDRV_REQ_NO_FALLBACK. */
+ bool zero_widen; /* atomic */
/* Clusters reading as zero; allocated on demand, frozen once valid. */
HBitmap *zero_bitmap;
/*
@@ -191,6 +195,23 @@ static int64_t block_copy_chunk_size(BlockCopyState *s)
}
}
+/*
+ * A write-zeroes task carries no buffer, so it may cover far more than
+ * block_copy_chunk_size(). Return how far it may run; the caller clamps it
+ * to where the zero run ends.
+ */
+static int64_t block_copy_widen_zero_area(BlockCopyState *s, int64_t offset,
+ int64_t bytes)
+{
+ int64_t clean;
+
+ bytes = MIN(bytes, QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
+ s->cluster_size));
+ clean = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, offset, bytes);
+
+ return clean < 0 ? bytes : clean - offset;
+}
+
/*
* Search for the first dirty area in offset/bytes range and create task at
* the beginning of it.
@@ -201,7 +222,9 @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state,
{
BlockCopyTask *task;
BlockCopyMethod method;
+ bool zero_fast = false;
int64_t max_chunk;
+ int64_t search_end = offset + bytes;
QEMU_LOCK_GUARD(&s->lock);
max_chunk = MIN_NON_ZERO(block_copy_chunk_size(s), call_state->max_chunk);
@@ -223,6 +246,11 @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state,
if (hbitmap_get(s->zero_bitmap, offset)) {
method = COPY_WRITE_ZEROES;
+ zero_fast = qatomic_read(&s->zero_widen);
+ if (zero_fast) {
+ bytes = block_copy_widen_zero_area(s, offset,
+ search_end - offset);
+ }
boundary = hbitmap_next_zero(s->zero_bitmap, offset, bytes);
} else {
boundary = hbitmap_next_dirty(s->zero_bitmap, offset, bytes);
@@ -248,6 +276,7 @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state,
.s = s,
.call_state = call_state,
.method = method,
+ .zero_fast = zero_fast,
};
reqlist_init_req(&s->reqs, &task->req, offset, bytes);
@@ -469,6 +498,7 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
.max_transfer = QEMU_ALIGN_DOWN(
block_copy_max_transfer(source, target),
cluster_size),
+ .zero_widen = target->bs->supported_zero_flags & BDRV_REQ_NO_FALLBACK,
};
s->discard_source = discard_source;
@@ -488,6 +518,15 @@ void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm)
s->progress = pm;
}
+/*
+ * Bytes @task moves as data, for the memory pool and the rate limit. Only a
+ * write-zeroes request that may not fall back is sure to need no buffer.
+ */
+static uint64_t block_copy_task_data_bytes(BlockCopyTask *task)
+{
+ return task->zero_fast ? 0 : task->req.bytes;
+}
+
/*
* Takes ownership of @task
*
@@ -509,7 +548,7 @@ static coroutine_fn int block_copy_task_run(AioTaskPool *pool,
aio_task_pool_wait_slot(pool);
if (aio_task_pool_status(pool) < 0) {
- co_put_to_shres(task->s->mem, task->req.bytes);
+ co_put_to_shres(task->s->mem, block_copy_task_data_bytes(task));
block_copy_task_end(task, -ECANCELED);
g_free(task);
return -ECANCELED;
@@ -520,6 +559,34 @@ static coroutine_fn int block_copy_task_run(AioTaskPool *pool,
return 0;
}
+/*
+ * Widening a write-zeroes request only pays off where the target zeroes by
+ * metadata, so while the target is believed to oblige, a request asks to fail
+ * instead of falling back to writing the zeroes out. A refusal ends the
+ * widening and shrinks @t to one cluster for the caller to write the usual
+ * way; the rest is dirty again and is picked up at the buffer chunk size.
+ */
+static int coroutine_fn GRAPH_RDLOCK
+block_copy_write_zeroes_fast(BlockCopyTask *t, bool *error_is_read)
+{
+ BlockCopyState *s = t->s;
+ int64_t nbytes = MIN(t->req.offset + t->req.bytes, s->len) - t->req.offset;
+ int ret;
+
+ ret = bdrv_co_pwrite_zeroes(s->target, t->req.offset, nbytes,
+ (s->write_flags & ~BDRV_REQ_WRITE_COMPRESSED) |
+ BDRV_REQ_NO_FALLBACK);
+ if (ret == -ENOTSUP) {
+ qatomic_set(&s->zero_widen, false);
+ block_copy_task_shrink(t, s->cluster_size);
+ } else if (ret < 0) {
+ trace_block_copy_write_zeroes_fail(s, t->req.offset, ret);
+ *error_is_read = false;
+ }
+
+ return ret;
+}
+
/*
* block_copy_do_copy
*
@@ -622,8 +689,13 @@ static coroutine_fn int block_copy_task_entry(AioTask *task)
int ret = -1;
WITH_GRAPH_RDLOCK_GUARD() {
- ret = block_copy_do_copy(s, t->req.offset, t->req.bytes, &method,
- &error_is_read);
+ if (t->zero_fast) {
+ ret = block_copy_write_zeroes_fast(t, &error_is_read);
+ }
+ if (!t->zero_fast || ret == -ENOTSUP) {
+ ret = block_copy_do_copy(s, t->req.offset, t->req.bytes, &method,
+ &error_is_read);
+ }
}
WITH_QEMU_LOCK_GUARD(&s->lock) {
@@ -640,7 +712,7 @@ static coroutine_fn int block_copy_task_entry(AioTask *task)
progress_work_done(s->progress, t->req.bytes);
}
}
- co_put_to_shres(s->mem, t->req.bytes);
+ co_put_to_shres(s->mem, block_copy_task_data_bytes(t));
block_copy_task_end(t, ret);
if (s->discard_source && ret == 0) {
@@ -867,6 +939,7 @@ block_copy_set_task_method(BlockCopyState *s, BlockCopyTask *task)
}
if (ret & BDRV_BLOCK_ZERO) {
task->method = COPY_WRITE_ZEROES;
+ task->zero_fast = qatomic_read(&s->zero_widen);
}
return true;
@@ -935,11 +1008,12 @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
}
}
- ratelimit_calculate_delay(&s->rate_limit, task->req.bytes);
+ ratelimit_calculate_delay(&s->rate_limit,
+ block_copy_task_data_bytes(task));
trace_block_copy_process(s, task->req.offset);
- co_get_from_shres(s->mem, task->req.bytes);
+ co_get_from_shres(s->mem, block_copy_task_data_bytes(task));
offset = task_end(task);
bytes = end - offset;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 741c8165c0..2edd3bccaa 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1656,7 +1656,9 @@
# copying process. Doesn't influence copy-before-write
# operations. 0 means unlimited. If max-chunk is non-zero then
# it should not be less than job cluster size which is calculated
-# as maximum of target image cluster size and 64k. Default 0.
+# as maximum of target image cluster size and 64k. Write-zeroes
+# requests to a target which zeroes efficiently are not bounded by
+# it. Default 0.
#
# @min-cluster-size: Minimum size of blocks used by copy-before-write
# and background copy operations. Has to be a power of 2. No
--
2.53.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 12/12] block/block-copy: coalesce write-zeroes tasks
2026-10-30 1:06 ` [PATCH v4 12/12] block/block-copy: coalesce write-zeroes tasks Denis V. Lunev
@ 2026-11-03 3:37 ` Eric Blake
0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2026-11-03 3:37 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, Vladimir Sementsov-Ogievskiy, John Snow,
Andrey Drobyshev
On Fri, Oct 30, 2026 at 02:06:12AM +0100, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
>
> Task size was capped to block_copy_chunk_size() regardless of method,
> which sizes a task by the copy buffer. A write-zeroes task carries no
> buffer, so that cap only splits one long known-zero run into a crowd of
> small COPY_WRITE_ZEROES tasks, each with its own request against the
> target.
>
> block_copy_task_create() already decides from zero_bitmap how far a task
> may run, so add block_copy_widen_zero_area() to that decision: a
> write-zeroes task runs to the next clean cluster of copy_bitmap, at most
> BDRV_REQUEST_MAX_BYTES aligned down to cluster_size, and the existing
> clamp cuts it back to where the zero run ends. The experimental x-perf
> max-chunk does not bound it, and its documentation now says so.
> Widening is safe: an overlapping caller waits on the task's BlockReq via
> reqlist_wait_one() rather than observing it mid-flight.
>
> A request that large is only cheap where the target zeroes by metadata.
> BDRV_REQ_NO_FALLBACK in supported_zero_flags rules out the targets which
> cannot, qcow2 v2 and iscsi among them, but it is optimistic for the
> rest: file-posix advertises it at open and only learns from a failing
> fallocate. So a write-zeroes task asks for it while the target is
> believed to oblige, which costs nothing when it does. A refusal ends the
> widening and shrinks the task to one cluster, written with fallback
> allowed. The rest of the range is dirty again, and the next pass of
> block_copy_common() picks it up as buffer-sized tasks running in
> parallel. Part of the refused range may already be zeroed, since
> -ENOTSUP only says the fast path is unavailable, and writing zeroes over
> it again changes nothing.
>
> Such a task is also the only one sure to need no buffer: without
> NO_FALLBACK, bdrv_co_do_pwrite_zeroes() writes the zeroes out through a
> bounce buffer. So it alone is left out of the memory pool and of the
> rate limit, where one widened request would otherwise hold the job back
> for as long as speed= takes to pass its whole length. The one-cluster
> write left after a refusal is not charged either, though it may bounce:
> that is at most one cluster per worker, once, as tasks created after the
> refusal are charged in full.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: John Snow <jsnow@redhat.com>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> block/block-copy.c | 96 +++++++++++++++++++++++++++++++++++++++-----
> qapi/block-core.json | 4 +-
> 2 files changed, 88 insertions(+), 12 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v4 00/12] block: cheaper zero handling in backup and commit
2026-10-30 1:06 [PATCH v4 00/12] block: cheaper zero handling in backup and commit Denis V. Lunev
` (11 preceding siblings ...)
2026-10-30 1:06 ` [PATCH v4 12/12] block/block-copy: coalesce write-zeroes tasks Denis V. Lunev
@ 2026-11-02 16:26 ` Eric Blake
12 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2026-11-02 16:26 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, Vladimir Sementsov-Ogievskiy, John Snow,
Andrey Drobyshev
On Fri, Oct 30, 2026 at 02:06:00AM +0100, Denis V. Lunev wrote:
> Backup and commit re-query the source block status for every task and
> size a task by the copy buffer, so a long run of zeroes turns into a
> crowd of small write-zeroes requests. This series reuses what the
> up-front scan already learned and lets one write-zeroes task cover a
> whole run.
>
> Backup of a 16G qcow2 image holding 1G of data, the rest never
> allocated, sync=full to a raw target on ext4:
>
> tasks time
> before 16384 1.6s
> after 1088 1.0s
>
> Such an image is the normal case rather than a corner one: a guest with
> discard enabled on a disk which is mostly free leaves exactly this
> shape behind.
>
> Eric, this is the v4 you asked about on 10-28; please take it instead
> of v3.
Will do, thanks. Queueing up a pull request today.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 18+ messages in thread