From: Paolo Bonzini <pbonz...@redhat.com> Put the logic to determine the copy size in a separate function, so that there is a simple state machine for the possible methods of copying data from one BlockDriverState to the other.
While at it, store the common computation of block_copy_max_transfer into a new field of BlockCopyState, and make sure that we always obey max_transfer; that's more efficient even for the COPY_RANGE_READ_WRITE case. Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> --- block/block-copy.c | 59 ++++++++++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index 37c8e8504b..10ce51a244 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -28,6 +28,13 @@ #define BLOCK_COPY_MAX_WORKERS 64 #define BLOCK_COPY_SLICE_TIME 100000000ULL /* ns */ +typedef enum { + COPY_READ_WRITE_CLUSTER, + COPY_READ_WRITE, + COPY_RANGE_SMALL, + COPY_RANGE_FULL +} BlockCopyMethod; + static coroutine_fn int block_copy_task_entry(AioTask *task); typedef struct BlockCopyCallState { @@ -85,8 +92,8 @@ typedef struct BlockCopyState { BdrvDirtyBitmap *copy_bitmap; int64_t in_flight_bytes; int64_t cluster_size; - bool use_copy_range; - int64_t copy_size; + BlockCopyMethod method; + int64_t max_transfer; uint64_t len; QLIST_HEAD(, BlockCopyTask) tasks; /* All tasks from all block-copy calls */ QLIST_HEAD(, BlockCopyCallState) calls; @@ -148,6 +155,23 @@ static bool coroutine_fn block_copy_wait_one(BlockCopyState *s, int64_t offset, return true; } +static inline int64_t block_copy_chunk_size(BlockCopyState *s) +{ + switch (s->method) { + case COPY_READ_WRITE_CLUSTER: + return s->cluster_size; + case COPY_READ_WRITE: + case COPY_RANGE_SMALL: + return MIN(MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER), + s->max_transfer); + case COPY_RANGE_FULL: + return MIN(MAX(s->cluster_size, BLOCK_COPY_MAX_COPY_RANGE), + s->max_transfer); + default: + abort(); + } +} + /* * Search for the first dirty area in offset/bytes range and create task at * the beginning of it. @@ -157,8 +181,9 @@ static BlockCopyTask *block_copy_task_create(BlockCopyState *s, int64_t offset, int64_t bytes) { BlockCopyTask *task; - int64_t max_chunk = MIN_NON_ZERO(s->copy_size, call_state->max_chunk); + int64_t max_chunk = block_copy_chunk_size(s); + max_chunk = MIN_NON_ZERO(max_chunk, call_state->max_chunk); if (!bdrv_dirty_bitmap_next_dirty_area(s->copy_bitmap, offset, offset + bytes, max_chunk, &offset, &bytes)) @@ -265,28 +290,27 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, .len = bdrv_dirty_bitmap_size(copy_bitmap), .write_flags = write_flags, .mem = shres_create(BLOCK_COPY_MAX_MEM), + .max_transfer = QEMU_ALIGN_DOWN(block_copy_max_transfer(source, target) + , cluster_size), }; - if (block_copy_max_transfer(source, target) < cluster_size) { + if (s->max_transfer < cluster_size) { /* * copy_range does not respect max_transfer. We don't want to bother * with requests smaller than block-copy cluster size, so fallback to * buffered copying (read and write respect max_transfer on their * behalf). */ - s->use_copy_range = false; - s->copy_size = cluster_size; + s->method = COPY_READ_WRITE_CLUSTER; } else if (write_flags & BDRV_REQ_WRITE_COMPRESSED) { /* Compression supports only cluster-size writes and no copy-range. */ - s->use_copy_range = false; - s->copy_size = cluster_size; + s->method = COPY_READ_WRITE_CLUSTER; } else { /* * We enable copy-range, but keep small copy_size, until first * successful copy_range (look at block_copy_do_copy). */ - s->use_copy_range = use_copy_range; - s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER); + s->method = use_copy_range ? COPY_RANGE_SMALL : COPY_READ_WRITE; } ratelimit_init(&s->rate_limit); @@ -369,30 +393,25 @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s, return ret; } - if (s->use_copy_range) { + if (s->method >= COPY_RANGE_SMALL) { ret = bdrv_co_copy_range(s->source, offset, s->target, offset, nbytes, 0, s->write_flags); if (ret < 0) { trace_block_copy_copy_range_fail(s, offset, ret); - s->use_copy_range = false; - s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER); + s->method = COPY_READ_WRITE; /* Fallback to read+write with allocated buffer */ } else { - if (s->use_copy_range) { + if (s->method == COPY_RANGE_SMALL) { /* * Successful copy-range. Now increase copy_size. copy_range * does not respect max_transfer (it's a TODO), so we factor * that in here. * - * Note: we double-check s->use_copy_range for the case when + * Note: we double-check s->method for the case when * parallel block-copy request unsets it during previous * bdrv_co_copy_range call. */ - s->copy_size = - MIN(MAX(s->cluster_size, BLOCK_COPY_MAX_COPY_RANGE), - QEMU_ALIGN_DOWN(block_copy_max_transfer(s->source, - s->target), - s->cluster_size)); + s->method = COPY_RANGE_FULL; } goto out; } -- 2.30.2