[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/3] block/io: align requests to subcluster_size
From: |
Andrey Drobyshev |
Subject: |
Re: [PATCH 2/3] block/io: align requests to subcluster_size |
Date: |
Tue, 11 Jul 2023 20:46:38 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.1 |
On 7/10/23 22:47, Eric Blake wrote:
> On Mon, Jun 26, 2023 at 07:08:33PM +0300, Andrey Drobyshev via wrote:
>> When target image is using subclusters, and we align the request during
>> copy-on-read, it makes sense to align to subcluster_size rather than
>> cluster_size. Otherwise we end up with unnecessary allocations.
>>
>> This commit renames bdrv_round_to_clusters() to bdrv_round_to_subclusters()
>> and utilizes subcluster_size field of BlockDriverInfo to make necessary
>> alignments. It affects copy-on-read as well as mirror job (which is
>> using bdrv_round_to_clusters()).
>>
>> This change also fixes the following bug with failing assert (covered by
>> the test in the subsequent commit):
>>
>> qemu-img create -f qcow2 base.qcow2 64K
>> qemu-img create -f qcow2 -o
>> extended_l2=on,backing_file=base.qcow2,backing_fmt=qcow2 img.qcow2 64K
>> qemu-io -c "write -P 0xaa 0 2K" img.qcow2
>> qemu-io -C -c "read -P 0x00 2K 62K" img.qcow2
>>
>> qemu-io: ../block/io.c:1236: bdrv_co_do_copy_on_readv: Assertion `skip_bytes
>> < pnum' failed.
>>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>> ---
>> block/io.c | 50 ++++++++++++++++++++--------------------
>> block/mirror.c | 8 +++----
>> include/block/block-io.h | 2 +-
>> 3 files changed, 30 insertions(+), 30 deletions(-)
>>
>> +++ b/include/block/block-io.h
>> @@ -189,7 +189,7 @@ bdrv_get_info(BlockDriverState *bs, BlockDriverInfo
>> *bdi);
>> ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs,
>> Error **errp);
>> BlockStatsSpecific *bdrv_get_specific_stats(BlockDriverState *bs);
>> -void bdrv_round_to_clusters(BlockDriverState *bs,
>> +void bdrv_round_to_subclusters(BlockDriverState *bs,
>> int64_t offset, int64_t bytes,
>> int64_t *cluster_offset,
>> int64_t *cluster_bytes);
>
> Indentation on subsequent lines should be fixed.
Thanks for pointing that out, got it fixed in v2:
https://lists.nongnu.org/archive/html/qemu-block/2023-07/msg00182.html
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>