Re: [Qemu-block] [Qemu-devel] [PATCH 2/3] qemu-img: Use zero writes after source backing EOF

[Eric Blake]

On 07/13/2018 06:14 AM, Max Reitz wrote:
> Past the end of the source backing file, we memset() buf_old to zero, so
> it is clearly easy to use blk_pwrite_zeroes() instead of blk_pwrite()
> then.
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
>   qemu-img.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index dd684d8bf0..2552e7dad6 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -3403,6 +3403,8 @@ static int img_rebase(int argc, char **argv)
>           }
>   
>           for (offset = 0; offset < size; offset += n) {
> +            bool buf_old_is_zero = false;
> +
>               /* How many bytes can we handle with the next read? */
>               n = MIN(IO_BUF_SIZE, size - offset);
>   
> @@ -3423,6 +3425,7 @@ static int img_rebase(int argc, char **argv)
>                */
>               if (offset >= old_backing_size) {
>                   memset(buf_old, 0, n);
> +                buf_old_is_zero = true;

Do we still need to spend time on the memset(), or...

>               } else {
>                   if (offset + n > old_backing_size) {
>                       n = old_backing_size - offset;
> @@ -3458,8 +3461,12 @@ static int img_rebase(int argc, char **argv)
>                   if (compare_buffers(buf_old + written, buf_new + written,
>                                       n - written, &pnum))
>                   {
> -                    ret = blk_pwrite(blk, offset + written,
> -                                     buf_old + written, pnum, 0);
> +                    if (buf_old_is_zero) {
> +                        ret = blk_pwrite_zeroes(blk, offset + written, pnum, 
> 0);

...are we able to guarantee that old_buf will not be used when 
buf_old_is_zero?

> +                    } else {
> +                        ret = blk_pwrite(blk, offset + written,
> +                                         buf_old + written, pnum, 0);
> +                    }
>                       if (ret < 0) {
>                           error_report("Error while writing to COW image: %s",
>                               strerror(-ret));

The series seems reasonable, but is post-3.0 material, so I haven't 
reviewed it any closer than this question.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org