Re: [Qemu-devel] [PATCH 3/8] block/io: handle alignment and max_transfer for copy_range

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1507 bytes --]

On 07.08.19 10:07, Vladimir Sementsov-Ogievskiy wrote:
> copy_range ignores these limitations, let's improve it. block/backup
> code handles max_transfer for copy_range by itself, now it's not needed
> more, drop it.

Shouldn’t this be two separate patches?

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/backup.c | 11 ++---------
>  block/io.c     | 41 +++++++++++++++++++++++++++++++++--------
>  2 files changed, 35 insertions(+), 17 deletions(-)

[...]

> diff --git a/block/io.c b/block/io.c
> index 06305c6ea6..5abbd0fff2 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -3005,6 +3005,12 @@ static int coroutine_fn bdrv_co_copy_range_internal(
>  {
>      BdrvTrackedRequest req;
>      int ret;
> +    uint32_t align = MAX(src->bs->bl.request_alignment,
> +                         dst->bs->bl.request_alignment);
> +    uint32_t max_transfer =
> +            QEMU_ALIGN_DOWN(MIN_NON_ZERO(MIN_NON_ZERO(src->bs->bl.max_transfer,
> +                                                      dst->bs->bl.max_transfer),
> +                                         INT_MAX), align);

I suppose the outer QEMU_ALIGN_DOWN() may result in @max_transfer of 0
(in theory, if one’s max_transfer is smaller than the other’s alignment).

Not likely, but should still be handled.

The rest looks good to me.

Max

>      /* TODO We can support BDRV_REQ_NO_FALLBACK here */
>      assert(!(read_flags & BDRV_REQ_NO_FALLBACK));


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]