Re: [PATCH v6 03/11] block: use int64_t instead of uint64_t in driver read handlers

[Eric Blake <eblake@redhat.com>]

On Fri, Sep 03, 2021 at 01:27:59PM +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 read handlers parameters which are already 64bit to
> signed type.
> 
> While being here, convert also flags parameter to be BdrvRequestFlags.
> 
> Now let's consider all callers. Simple
> 
>   git grep '\->bdrv_\(aio\|co\)_preadv\(_part\)\?'
> 
> shows that's there three callers of driver function:
> 
>  bdrv_driver_preadv() in block/io.c, passes int64_t, checked by
>    bdrv_check_qiov_request() to be non-negative.
> 
>  qcow2_load_vmstate() does bdrv_check_qiov_request().
> 
>  do_perform_cow_read() has uint64_t argument. And a lot of things in
>  qcow2 driver are uint64_t, so converting it is big job. But we must
>  not work with requests that don't satisfy bdrv_check_qiov_request(),
>  so let's just assert it here.
> 
> Still, the functions may be called directly, not only by drv->...
> Let's check:
> 
> git grep '\.bdrv_\(aio\|co\)_preadv\(_part\)\?\s*=' | \
> awk '{print $4}' | sed 's/,//' | sed 's/&//' | sort | uniq | \
> while read func; do git grep "$func(" | \
> grep -v "$func(BlockDriverState"; done
> 
> The only one such caller:
> 
>     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, &data, 1);
>     ...
>     ret = bdrv_replace_test_co_preadv(bs, 0, 1, &qiov, 0);
> 
> in tesTS/unit/test-bdrv-drain.c, and it's OK obviously.

Odd capitalization.

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

> +++ b/block/qcow2-cluster.c
> @@ -505,7 +505,19 @@ static int coroutine_fn do_perform_cow_read(BlockDriverState *bs,
>          return -ENOMEDIUM;
>      }
>  
> -    /* Call .bdrv_co_readv() directly instead of using the public block-layer
> +    /*
> +     * We never deal with requests that doesn't satisfy
> +     * bdrv_check_qiov_request(), and aligning requests to clusters never break

never breaks

> +     * this condition. So, do some assertions before calling
> +     * bs->drv->bdrv_co_preadv_part() which has int64_t arguments.
> +     */
> +    assert(src_cluster_offset <= INT64_MAX);
> +    assert(src_cluster_offset + offset_in_cluster <= INT64_MAX);
> +    assert(qiov->size <= INT64_MAX);
> +    bdrv_check_qiov_request(src_cluster_offset + offset_in_cluster, qiov->size,
> +                            qiov, 0, &error_abort);

> +++ b/slirp
> @@ -1 +1 @@
> -Subproject commit a88d9ace234a24ce1c17189642ef9104799425e0
> +Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece
> diff --git a/ui/keycodemapdb b/ui/keycodemapdb
> index d21009b1c9..6119e6e19a 160000
> --- a/ui/keycodemapdb
> +++ b/ui/keycodemapdb
> @@ -1 +1 @@
> -Subproject commit d21009b1c9f94b740ea66be8e48a1d8ad8124023
> +Subproject commit 6119e6e19a050df847418de7babe5166779955e4

Oops.  Fix that (or I can do it while staging, if the rest of the
series is okay), and you have:

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

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