[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH for-4.0?] qemu-img: Enable BDRV_REQ_MAY_UNMAP in
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH for-4.0?] qemu-img: Enable BDRV_REQ_MAY_UNMAP in convert |
Date: |
Mon, 25 Mar 2019 09:30:03 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 |
On 3/23/19 5:59 PM, Nir Soffer wrote:
> With Kevin's "block: Fix slow pre-zeroing in qemu-img convert"[1]
> we skip the pre zero step called like this:
>
> blk_make_zero(s->target, BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK)
>
> And we write zeroes later using:
>
> blk_co_pwrite_zeroes(s->target,
> sector_num << BDRV_SECTOR_BITS,
> n << BDRV_SECTOR_BITS, 0);
>
> Since we use flags=0, this is translated to NBD_CMD_WRITE_ZEROES with
> NBD_CMD_FLAG_NO_HOLE flag, which cause the NBD server to allocated space
> instead of punching a hole.
>
>
> $ qemu-img info dst.img
> virtual size: 50M (52428800 bytes)
> disk size: 5.0M
Up to here makes sense for the commit message...
>
> Tested on top of Kevin patches:
> http://lists.nongnu.org/archive/html/qemu-block/2019-03/msg00755.html
>
> I'm not sure this change is correct for all cases, posting for
> discussion.
...this part probably belongs...
>
> [1] http://lists.nongnu.org/archive/html/qemu-block/2019-03/msg00761.html
>
> Signed-off-by: Nir Soffer <address@hidden>
> ---
...here, after the ---, as it is useful for reviewers but not needed for
the permanent git log.
> qemu-img.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 8ee63daeae..ca9deb3758 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1752,11 +1752,12 @@ static int coroutine_fn
> convert_co_write(ImgConvertState *s, int64_t sector_num,
> assert(!s->target_has_backing);
> break;
> }
> ret = blk_co_pwrite_zeroes(s->target,
> sector_num << BDRV_SECTOR_BITS,
> - n << BDRV_SECTOR_BITS, 0);
> + n << BDRV_SECTOR_BITS,
> + BDRV_REQ_MAY_UNMAP);
The idea makes sense to me. Maybe Kevin will come up with a
counterargument why we don't want convert to allow holes by default, but
unless we have a strong argument against it, it looks like a performance
improvement worth having in 4.0.
Reviewed-by: Eric Blake <address@hidden>
> if (ret < 0) {
> return ret;
> }
> break;
> }
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature