[PATCH v3 06/10] block: Make 'bytes' param of bdrv_co_{pread, pwrite, preadv, pwritev}() an int64_t

Alberto Faria posted 10 patches 3 years, 8 months ago
Maintainers: Ari Sundholm <ari@tuxera.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>, "Denis V. Lunev" <den@openvz.org>, Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>, Eric Blake <eblake@redhat.com>, John Snow <jsnow@redhat.com>, Stefan Weil <sw@weilnetz.de>, Jeff Cody <codyprime@gmail.com>, "Daniel P. Berrangé" <berrange@redhat.com>
There is a newer version of this series
[PATCH v3 06/10] block: Make 'bytes' param of bdrv_co_{pread, pwrite, preadv, pwritev}() an int64_t
Posted by Alberto Faria 3 years, 8 months ago
For consistency with other I/O functions, and in preparation to
implement bdrv_{pread,pwrite}() using generated_co_wrapper.

unsigned int fits in int64_t, so all callers remain correct.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 block/coroutines.h           | 4 ++--
 include/block/block_int-io.h | 8 ++++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/block/coroutines.h b/block/coroutines.h
index 830ecaa733..3f41238b33 100644
--- a/block/coroutines.h
+++ b/block/coroutines.h
@@ -91,11 +91,11 @@ int coroutine_fn blk_co_do_flush(BlockBackend *blk);
  */
 
 int generated_co_wrapper
-bdrv_preadv(BdrvChild *child, int64_t offset, unsigned int bytes,
+bdrv_preadv(BdrvChild *child, int64_t offset, int64_t bytes,
             QEMUIOVector *qiov, BdrvRequestFlags flags);
 
 int generated_co_wrapper
-bdrv_pwritev(BdrvChild *child, int64_t offset, unsigned int bytes,
+bdrv_pwritev(BdrvChild *child, int64_t offset, int64_t bytes,
              QEMUIOVector *qiov, BdrvRequestFlags flags);
 
 int generated_co_wrapper
diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index d4d3bed783..e31f2b4311 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -56,20 +56,24 @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
     QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
 
 static inline int coroutine_fn bdrv_co_pread(BdrvChild *child,
-    int64_t offset, unsigned int bytes, void *buf, BdrvRequestFlags flags)
+    int64_t offset, int64_t bytes, void *buf, BdrvRequestFlags flags)
 {
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
     IO_CODE();
 
+    assert(bytes <= SIZE_MAX);
+
     return bdrv_co_preadv(child, offset, bytes, &qiov, flags);
 }
 
 static inline int coroutine_fn bdrv_co_pwrite(BdrvChild *child,
-    int64_t offset, unsigned int bytes, const void *buf, BdrvRequestFlags flags)
+    int64_t offset, int64_t bytes, const void *buf, BdrvRequestFlags flags)
 {
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
     IO_CODE();
 
+    assert(bytes <= SIZE_MAX);
+
     return bdrv_co_pwritev(child, offset, bytes, &qiov, flags);
 }
 
-- 
2.35.3
Re: [PATCH v3 06/10] block: Make 'bytes' param of bdrv_co_{pread,pwrite,preadv,pwritev}() an int64_t
Posted by Stefan Hajnoczi 3 years, 8 months ago
On Thu, May 19, 2022 at 03:48:36PM +0100, Alberto Faria wrote:
> For consistency with other I/O functions, and in preparation to
> implement bdrv_{pread,pwrite}() using generated_co_wrapper.
> 
> unsigned int fits in int64_t, so all callers remain correct.
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>  block/coroutines.h           | 4 ++--
>  include/block/block_int-io.h | 8 ++++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/block/coroutines.h b/block/coroutines.h
> index 830ecaa733..3f41238b33 100644
> --- a/block/coroutines.h
> +++ b/block/coroutines.h
> @@ -91,11 +91,11 @@ int coroutine_fn blk_co_do_flush(BlockBackend *blk);
>   */
>  
>  int generated_co_wrapper
> -bdrv_preadv(BdrvChild *child, int64_t offset, unsigned int bytes,
> +bdrv_preadv(BdrvChild *child, int64_t offset, int64_t bytes,
>              QEMUIOVector *qiov, BdrvRequestFlags flags);
>  
>  int generated_co_wrapper
> -bdrv_pwritev(BdrvChild *child, int64_t offset, unsigned int bytes,
> +bdrv_pwritev(BdrvChild *child, int64_t offset, int64_t bytes,
>               QEMUIOVector *qiov, BdrvRequestFlags flags);
>  
>  int generated_co_wrapper
> diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
> index d4d3bed783..e31f2b4311 100644
> --- a/include/block/block_int-io.h
> +++ b/include/block/block_int-io.h
> @@ -56,20 +56,24 @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
>      QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
>  
>  static inline int coroutine_fn bdrv_co_pread(BdrvChild *child,
> -    int64_t offset, unsigned int bytes, void *buf, BdrvRequestFlags flags)
> +    int64_t offset, int64_t bytes, void *buf, BdrvRequestFlags flags)
>  {
>      QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
>      IO_CODE();
>  
> +    assert(bytes <= SIZE_MAX);

Maybe let the existing bdrv_check_request32() call in bdrv_co_preadv()
check this? It returns -EIO if bytes is too large.
Re: [PATCH v3 06/10] block: Make 'bytes' param of bdrv_co_{pread, pwrite, preadv, pwritev}() an int64_t
Posted by Alberto Faria 3 years, 8 months ago
On Thu, May 26, 2022 at 10:00 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> Maybe let the existing bdrv_check_request32() call in bdrv_co_preadv()
> check this? It returns -EIO if bytes is too large.

I'd be okay with that. Does this warrant changing blk_co_pread() and
blk_co_pwrite() as well?

Eric, what do you think?
Re: [PATCH v3 06/10] block: Make 'bytes' param of bdrv_co_{pread,pwrite,preadv,pwritev}() an int64_t
Posted by Eric Blake 3 years, 8 months ago
On Thu, May 26, 2022 at 12:05:55PM +0100, Alberto Faria wrote:
> On Thu, May 26, 2022 at 10:00 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > Maybe let the existing bdrv_check_request32() call in bdrv_co_preadv()

in bdrv_co_preadv_part()

> > check this? It returns -EIO if bytes is too large.
> 
> I'd be okay with that. Does this warrant changing blk_co_pread() and
> blk_co_pwrite() as well?
> 
> Eric, what do you think?
>

Yes, reusing the existing function covers more cases with common error
messages.  All that matters is that we check for overflow before
trying to populate the qiov.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Re: [PATCH v3 06/10] block: Make 'bytes' param of bdrv_co_{pread,pwrite,preadv,pwritev}() an int64_t
Posted by Eric Blake 3 years, 8 months ago
On Thu, May 19, 2022 at 03:48:36PM +0100, Alberto Faria wrote:
> For consistency with other I/O functions, and in preparation to
> implement bdrv_{pread,pwrite}() using generated_co_wrapper.
> 
> unsigned int fits in int64_t, so all callers remain correct.
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>  block/coroutines.h           | 4 ++--
>  include/block/block_int-io.h | 8 ++++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
>

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

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