Re: [PATCH v4 14/16] block/io: support int64_t bytes in bdrv_co_p{read,write}v_part()

[Eric Blake]

On 12/11/20 12:39 PM, Vladimir Sementsov-Ogievskiy wrote:
> We are generally moving to int64_t for both offset and bytes parameters
> on all io paths.
> 
> Main motivation is realization of 64-bit write_zeroes operation for
> fast zeroing large disk chunks, up to the whole disk.
> 
> We chose signed type, to be consistent with off_t (which is signed) and
> with possibility for signed return type (where negative value means
> error).
> 
> So, prepare bdrv_co_preadv_part() and bdrv_co_pwritev_part() and their
> remaining dependencies now.
> 
> bdrv_pad_request() is updated simultaneously, as pointer to bytes passed
> to it both from bdrv_co_pwritev_part() and bdrv_co_preadv_part().
> 
> So, all callers of bdrv_pad_request() are updated to pass 64bit bytes.
> bdrv_pad_request() is already good for 64bit requests, add
> corresponding assertion.
> 
> Look at bdrv_co_preadv_part() and bdrv_co_pwritev_part().
> Type is widening, so callers are safe. Let's look inside the functions.
> 
> In bdrv_co_preadv_part() and bdrv_aligned_pwritev() we only pass bytes
> to other already int64_t interfaces (and some obviously safe
> calculations), it's OK.
> 
> In bdrv_co_do_zero_pwritev() aligned_bytes may become large now, still
> it's passed to bdrv_aligned_pwritev which supports int64_t bytes.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/block/block_int.h |  4 ++--
>  block/io.c                | 14 ++++++++------
>  block/trace-events        |  4 ++--
>  3 files changed, 12 insertions(+), 10 deletions(-)
> 

> @@ -1745,7 +1747,7 @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
>      BdrvRequestPadding pad;
>      int ret;
>  
> -    trace_bdrv_co_preadv(bs, offset, bytes, flags);
> +    trace_bdrv_co_preadv_part(bs, offset, bytes, flags);
>  

> @@ -2173,7 +2175,7 @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
>      int ret;
>      bool padded = false;
>  
> -    trace_bdrv_co_pwritev(child->bs, offset, bytes, flags);
> +    trace_bdrv_co_pwritev_part(child->bs, offset, bytes, flags);

The change in trace names makes sense, but isn't specifically called out
in the commit message.

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

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