07.10.2019 18:48, Max Reitz wrote:
> On 03.10.19 19:15, Vladimir Sementsov-Ogievskiy wrote:
>> No reason to limit buffered copy to one cluster. Let's allow up to 1
>> MiB.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
>> ---
>> include/block/block-copy.h | 2 +-
>> block/block-copy.c | 44 +++++++++++++++++++++++++++-----------
>> 2 files changed, 32 insertions(+), 14 deletions(-)
>
>
> Er, oops, looks like I was a bit quick there...
>
>> @@ -100,17 +101,28 @@ BlockCopyState *block_copy_state_new(BdrvChild
>> *source, BdrvChild *target,
>> .mem = qemu_co_shared_amount_new(BLOCK_COPY_MAX_MEM),
>> };
>>
>> - s->copy_range_size = QEMU_ALIGN_DOWN(max_transfer, cluster_size),
>> - /*
>> - * Set use_copy_range, consider the following:
>> - * 1. Compression is not supported for copy_range.
>> - * 2. copy_range does not respect max_transfer (it's a TODO), so we
>> factor
>> - * that in here. If max_transfer is smaller than the
>> job->cluster_size,
>> - * we do not use copy_range (in that case it's zero after aligning
>> down
>> - * above).
>> - */
>> - s->use_copy_range =
>> - !(write_flags & BDRV_REQ_WRITE_COMPRESSED) && s->copy_range_size >
>> 0;
>> + if (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;
>> + } else if (write_flags & BDRV_REQ_WRITE_COMPRESSED) {
>> + /* Compression is not supported for copy_range */
>> + s->use_copy_range = false;
>> + s->copy_size = MAX(cluster_size, BLOCK_COPY_MAX_BUFFER);
>> + } else {
>> + /*
>> + * copy_range does not respect max_transfer (it's a TODO), so we
>> factor
>> + * that in here.
>> + */
>> + s->use_copy_range = true;
>> + s->copy_size = MIN(MAX(cluster_size, BLOCK_COPY_MAX_COPY_RANGE),
>
> This is already part of max_transfer, isn’t it?
>
> (That doesn’t make it wrong, but I think max_transfer will always be
> less than or equal to MAX(cluster_size, BLOCK_COPY_MAX_COPY_RANGE) anyway.)
>
> Max
Hmm, oops, yes. It'd better be just QEMU_ALIGN_DOWN(max_transfer, cluster_size)
>
>> + QEMU_ALIGN_DOWN(max_transfer, cluster_size));
>
--
Best regards,
Vladimir
