From: Denis V. Lunev <[email protected]> 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. 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 <[email protected]> Signed-off-by: Denis V. Lunev <[email protected]> CC: Vladimir Sementsov-Ogievskiy <[email protected]> CC: John Snow <[email protected]> CC: Andrey Drobyshev <[email protected]> --- block/backup.c | 94 ++++++++++++++++++++++++-------------- block/block-copy.c | 41 +++++++++++++++++ include/block/block-copy.h | 4 ++ 3 files changed, 105 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 94d4f3dd69..c0a3359938 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -813,6 +813,47 @@ 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 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, INT64_MAX, + &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; + } + } + + /* 0 if the query above didn't yet reach dirty_end: try again next call. */ + *count = *cached_end >= dirty_end ? dirty_end - offset : 0; +} + /* * 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
