Re: [PATCH v2 3/8] qemu-img: rebase: use backing files' BlockBackend for buffer alignment

[Hanna Czenczek <hreitz@redhat.com>]

On 15.09.23 18:20, Andrey Drobyshev wrote:
> Since commit bb1c05973cf ("qemu-img: Use qemu_blockalign"), buffers for
> the data read from the old and new backing files are aligned using
> BlockDriverState (or BlockBackend later on) referring to the target image.
> However, this isn't quite right, because buf_new is only being used for
> reading from the new backing, while buf_old is being used for both reading
> from the old backing and writing to the target.  Let's take that into account
> and use more appropriate values as alignments.
>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
>   qemu-img.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 50660ba920..d12e4a4753 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -3750,8 +3750,13 @@ static int img_rebase(int argc, char **argv)
>           int64_t n;
>           float local_progress = 0;
>   
> -        buf_old = blk_blockalign(blk, IO_BUF_SIZE);
> -        buf_new = blk_blockalign(blk, IO_BUF_SIZE);
> +        if (blk_old_backing && bdrv_opt_mem_align(blk_bs(blk)) >
> +            bdrv_opt_mem_align(blk_bs(blk_old_backing))) {
> +            buf_old = blk_blockalign(blk, IO_BUF_SIZE);
> +        } else {
> +            buf_old = blk_blockalign(blk_old_backing, IO_BUF_SIZE);
> +        }

As I read this, if blk_old_backing is NULL, we will go to the 
blk_blockalign(blk_old_backing, IO_BUF_SIZE) path.  I think if it is 
NULL, we should align on blk instead.

Hanna

> +        buf_new = blk_blockalign(blk_new_backing, IO_BUF_SIZE);
>   
>           size = blk_getlength(blk);
>           if (size < 0) {