Re: [PATCH v6 10/11] block: use int64_t instead of int in driver discard handlers

[Eric Blake]

On Fri, Sep 03, 2021 at 01:28:06PM +0300, 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, convert driver discard handlers bytes parameter to int64_t.
> 
> The only caller of all updated function is bdrv_co_pdiscard in
> block/io.c. It is already prepared to work with 64bit requests, but
> pass at most max(bs->bl.max_pdiscard, INT_MAX) to the driver.
> 
> Let's look at all updated functions:

> 
> Great! Now all drivers are prepared to handle 64bit discard requests,
> or else have explicit max_pdiscard limits.

Very similar analysis to the write-zeroes.

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---

> +++ b/block/nbd.c
> @@ -1457,15 +1457,17 @@ static int nbd_client_co_flush(BlockDriverState *bs)
>  }
>  
>  static int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset,
> -                                  int bytes)
> +                                  int64_t bytes)
>  {
>      BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
>      NBDRequest request = {
>          .type = NBD_CMD_TRIM,
>          .from = offset,
> -        .len = bytes,
> +        .len = bytes, /* len is uint32_t */
>      };
>  
> +    assert(bytes <= UINT32_MAX); /* rely on max_pdiscard */

Aha - you used <= here, compared to < in 7/11 on write zeroes.
Consistency says we want <= in both places.

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

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