Re: [PATCH v3 2/8] block: Add no_fallback parameter to bdrv_co_truncate()

[Eric Blake <eblake@redhat.com>]

On 11/22/19 10:05 AM, Kevin Wolf wrote:
> This adds a no_fallback parameter to bdrv_co_truncate(), bdrv_truncate()
> and blk_truncate() in preparation for a fix that potentially needs to
> zero-write the new area. no_fallback will use BDRV_REQ_NO_FALLBACK for
> this operation and lets the truncate fail if an efficient zero write
> isn't possible.
> 
> Only qmp_block_resize() passes true for this parameter because it is a
> blocking monitor command, so we don't want to add more potentially slow
> I/O operations to it than we already have.
> 
> All other users will accept even a slow fallback to avoid failure.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---

> +++ b/include/block/block.h
> @@ -347,9 +347,10 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
>   void bdrv_refresh_filename(BlockDriverState *bs);
>   
>   int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
> -                                  PreallocMode prealloc, Error **errp);
> +                                  PreallocMode prealloc, bool no_fallback,
> +                                  Error **errp);
>   int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact,
> -                  PreallocMode prealloc, Error **errp);
> +                  PreallocMode prealloc, bool no_fallback, Error **errp);
>   

New signature, most of the changes are mechanical to pass the new 
parameter...

> +++ b/block/io.c
> @@ -3313,9 +3313,15 @@ static void bdrv_parent_cb_resize(BlockDriverState *bs)
>    * If 'exact' is true, the file must be resized to exactly the given
>    * 'offset'.  Otherwise, it is sufficient for the node to be at least
>    * 'offset' bytes in length.
> + *
> + * If 'no_fallback' is true, a possibly needed writte_zeroes operation to avoid

write

> + * making a longer backing file visible will use BDRV_REQ_NO_FALLBACK. If the
> + * zero write is necessary and this flag is set, bdrv_co_truncate() will fail
> + * if efficient zero writes cannot be provided.
>    */

> +++ b/qemu-img.c
> @@ -3836,7 +3836,7 @@ static int img_resize(int argc, char **argv)
>        * resizing, so pass @exact=true.  It is of no use to report
>        * success when the image has not actually been resized.
>        */
> -    ret = blk_truncate(blk, total_size, true, prealloc, &err);
> +    ret = blk_truncate(blk, total_size, true, prealloc, false, &err);
>       if (!ret) {
>           qprintf(quiet, "Image resized.\n");
>       } else {

Hmm - thought for a future patch (not this one): are there situations 
where it may be faster to perform bulk pre-zeroing of the tail of a file 
by performing two truncates (smaller and then larger) because we know 
that just-added bytes from a truncate will read as zero?  This may be 
true for some file systems (but is not true for block devices, nor for 
things like NBD that lack resize).  Anyway, unrelated to this patch.

With the typo fixed,

Reviewed-by: Eric Blake <eblake@redhat.com>

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