[PATCH 00/12] block: 64bit blk io

[PATCH 00/12] block: 64bit blk io

[Vladimir Sementsov-Ogievskiy]

Hi all!

That's a new part of 64bit block-layer story, now to update
blk_* io functions.


Vladimir Sementsov-Ogievskiy (12):
  block-backend: blk_check_byte_request(): int64_t bytes
  block-backend: make blk_co_preadv() 64bit
  block-backend: convert blk_co_pwritev_part to int64_t bytes
  block-backend: convert blk_co_pdiscard to int64_t bytes
  block-backend: rename _do_ helper functions to _co_do_
  block-coroutine-wrapper.py: support BlockBackend first argument
  block-backend: drop blk_prw, use block-coroutine-wrapper
  block-backend: convert blk_foo wrappers to use int64_t bytes parameter
  block-backend: convert blk_co_copy_range to int64_t bytes
  block-backend: convert blk_aio_ functions to int64_t bytes paramter
  block-backend: blk_pread, blk_pwrite: rename count parameter to bytes
  block-backend: drop INT_MAX restriction from blk_check_byte_request()

 block/coroutines.h                 |  33 ++++
 include/sysemu/block-backend.h     |  23 +--
 block/block-backend.c              | 247 +++++++++++++----------------
 block/trace-events                 |   4 +-
 scripts/block-coroutine-wrapper.py |  12 +-
 5 files changed, 165 insertions(+), 154 deletions(-)

-- 
2.31.1

[PATCH 01/12] block-backend: blk_check_byte_request(): int64_t bytes

[Vladimir Sementsov-Ogievskiy]

Rename size and make it int64_t to correspond to modern block layer,
which always uses int64_t for offset and bytes (not in blk layer yet,
which is a task for following commits).

All callers pass int or unsigned int.

So, for bytes in [0, INT_MAX] nothing is changed, for negative bytes we
now fail on "bytes < 0" check instead of "bytes > INT_MAX" check.

Note, that blk_check_byte_request() still doesn't allow requests
exceeding INT_MAX.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/block-backend.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 6140d133e2..d121ca3868 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1161,11 +1161,11 @@ void blk_set_disable_request_queuing(BlockBackend *blk, bool disable)
 }
 
 static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
-                                  size_t size)
+                                  int64_t bytes)
 {
     int64_t len;
 
-    if (size > INT_MAX) {
+    if (bytes < 0 || bytes > INT_MAX) {
         return -EIO;
     }
 
@@ -1183,7 +1183,7 @@ static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
             return len;
         }
 
-        if (offset > len || len - offset < size) {
+        if (offset > len || len - offset < bytes) {
             return -EIO;
         }
     }
-- 
2.31.1

Re: [PATCH 01/12] block-backend: blk_check_byte_request(): int64_t bytes

[Eric Blake]

On Wed, Oct 06, 2021 at 03:17:07PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> Rename size and make it int64_t to correspond to modern block layer,
> which always uses int64_t for offset and bytes (not in blk layer yet,
> which is a task for following commits).
> 
> All callers pass int or unsigned int.
> 
> So, for bytes in [0, INT_MAX] nothing is changed, for negative bytes we
> now fail on "bytes < 0" check instead of "bytes > INT_MAX" check.
> 
> Note, that blk_check_byte_request() still doesn't allow requests
> exceeding INT_MAX.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/block-backend.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 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

[PATCH 02/12] block-backend: make blk_co_preadv() 64bit

[Vladimir Sementsov-Ogievskiy]

For both updated functions type of bytes becomes wider, so all callers
should be OK with it.

blk_co_preadv() only pass its arguments to blk_do_preadv().

blk_do_preadv() pass bytes to:

 - trace_blk_co_preadv, which is updated too
 - blk_check_byte_request, throttle_group_co_io_limits_intercept,
   bdrv_co_preadv, which are already int64_t.

Note that requests exceeding INT_MAX are still restricted by
blk_check_byte_request().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/sysemu/block-backend.h | 2 +-
 block/block-backend.c          | 4 ++--
 block/trace-events             | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 29d4fdbf63..32a88878bb 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -126,7 +126,7 @@ BlockBackend *blk_by_dev(void *dev);
 BlockBackend *blk_by_qdev_id(const char *id, Error **errp);
 void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, void *opaque);
 int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
-                               unsigned int bytes, QEMUIOVector *qiov,
+                               int64_t bytes, QEMUIOVector *qiov,
                                BdrvRequestFlags flags);
 int coroutine_fn blk_co_pwritev_part(BlockBackend *blk, int64_t offset,
                                      unsigned int bytes,
diff --git a/block/block-backend.c b/block/block-backend.c
index d121ca3868..be5a7fb5fb 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1205,7 +1205,7 @@ static void coroutine_fn blk_wait_while_drained(BlockBackend *blk)
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
 static int coroutine_fn
-blk_do_preadv(BlockBackend *blk, int64_t offset, unsigned int bytes,
+blk_do_preadv(BlockBackend *blk, int64_t offset, int64_t bytes,
               QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     int ret;
@@ -1236,7 +1236,7 @@ blk_do_preadv(BlockBackend *blk, int64_t offset, unsigned int bytes,
 }
 
 int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
-                               unsigned int bytes, QEMUIOVector *qiov,
+                               int64_t bytes, QEMUIOVector *qiov,
                                BdrvRequestFlags flags)
 {
     int ret;
diff --git a/block/trace-events b/block/trace-events
index f2d0a9b62a..ff397ffff4 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -5,7 +5,7 @@ bdrv_open_common(void *bs, const char *filename, int flags, const char *format_n
 bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d"
 
 # block-backend.c
-blk_co_preadv(void *blk, void *bs, int64_t offset, unsigned int bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %u flags 0x%x"
+blk_co_preadv(void *blk, void *bs, int64_t offset, int64_t bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %" PRId64 " flags 0x%x"
 blk_co_pwritev(void *blk, void *bs, int64_t offset, unsigned int bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %u flags 0x%x"
 blk_root_attach(void *child, void *blk, void *bs) "child %p blk %p bs %p"
 blk_root_detach(void *child, void *blk, void *bs) "child %p blk %p bs %p"
-- 
2.31.1

Re: [PATCH 02/12] block-backend: make blk_co_preadv() 64bit

[Eric Blake]

On Wed, Oct 06, 2021 at 03:17:08PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> For both updated functions type of bytes becomes wider, so all callers
> should be OK with it.
> 
> blk_co_preadv() only pass its arguments to blk_do_preadv().
> 
> blk_do_preadv() pass bytes to:
> 
>  - trace_blk_co_preadv, which is updated too
>  - blk_check_byte_request, throttle_group_co_io_limits_intercept,
>    bdrv_co_preadv, which are already int64_t.
> 
> Note that requests exceeding INT_MAX are still restricted by
> blk_check_byte_request().
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/sysemu/block-backend.h | 2 +-
>  block/block-backend.c          | 4 ++--
>  block/trace-events             | 2 +-
>  3 files changed, 4 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

[PATCH 03/12] block-backend: convert blk_co_pwritev_part to int64_t bytes

[Vladimir Sementsov-Ogievskiy]

We convert blk_do_pwritev_part() and some wrappers:
blk_co_pwritev_part(), blk_co_pwritev(), blk_co_pwrite_zeroes().

All functions are converted so that parameter type becomes wider, so
all callers should be OK with it.

Look at blk_do_pwritev_part() body:
bytes is passed to:

 - trace_blk_co_pwritev (we update it here)
 - blk_check_byte_request, throttle_group_co_io_limits_intercept,
   bdrv_co_pwritev_part - all already has int64_t argument.

Note that requests exceeding INT_MAX are still restricted by
blk_check_byte_request().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/sysemu/block-backend.h | 6 +++---
 block/block-backend.c          | 8 ++++----
 block/trace-events             | 2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 32a88878bb..844bb039c5 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -129,11 +129,11 @@ int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
                                int64_t bytes, QEMUIOVector *qiov,
                                BdrvRequestFlags flags);
 int coroutine_fn blk_co_pwritev_part(BlockBackend *blk, int64_t offset,
-                                     unsigned int bytes,
+                                     int64_t bytes,
                                      QEMUIOVector *qiov, size_t qiov_offset,
                                      BdrvRequestFlags flags);
 int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
-                               unsigned int bytes, QEMUIOVector *qiov,
+                               int64_t bytes, QEMUIOVector *qiov,
                                BdrvRequestFlags flags);
 
 static inline int coroutine_fn blk_co_pread(BlockBackend *blk, int64_t offset,
@@ -242,7 +242,7 @@ int blk_get_open_flags_from_root_state(BlockBackend *blk);
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
                   BlockCompletionFunc *cb, void *opaque);
 int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
-                                      int bytes, BdrvRequestFlags flags);
+                                      int64_t bytes, BdrvRequestFlags flags);
 int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
                           int bytes);
 int blk_truncate(BlockBackend *blk, int64_t offset, bool exact,
diff --git a/block/block-backend.c b/block/block-backend.c
index be5a7fb5fb..b09ec5a7c7 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1250,7 +1250,7 @@ int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
 static int coroutine_fn
-blk_do_pwritev_part(BlockBackend *blk, int64_t offset, unsigned int bytes,
+blk_do_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes,
                     QEMUIOVector *qiov, size_t qiov_offset,
                     BdrvRequestFlags flags)
 {
@@ -1286,7 +1286,7 @@ blk_do_pwritev_part(BlockBackend *blk, int64_t offset, unsigned int bytes,
 }
 
 int coroutine_fn blk_co_pwritev_part(BlockBackend *blk, int64_t offset,
-                                     unsigned int bytes,
+                                     int64_t bytes,
                                      QEMUIOVector *qiov, size_t qiov_offset,
                                      BdrvRequestFlags flags)
 {
@@ -1300,7 +1300,7 @@ int coroutine_fn blk_co_pwritev_part(BlockBackend *blk, int64_t offset,
 }
 
 int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
-                                unsigned int bytes, QEMUIOVector *qiov,
+                                int64_t bytes, QEMUIOVector *qiov,
                                 BdrvRequestFlags flags)
 {
     return blk_co_pwritev_part(blk, offset, bytes, qiov, 0, flags);
@@ -2208,7 +2208,7 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
 }
 
 int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
-                                      int bytes, BdrvRequestFlags flags)
+                                      int64_t bytes, BdrvRequestFlags flags)
 {
     return blk_co_pwritev(blk, offset, bytes, NULL,
                           flags | BDRV_REQ_ZERO_WRITE);
diff --git a/block/trace-events b/block/trace-events
index ff397ffff4..ab56edacb4 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -6,7 +6,7 @@ bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d"
 
 # block-backend.c
 blk_co_preadv(void *blk, void *bs, int64_t offset, int64_t bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %" PRId64 " flags 0x%x"
-blk_co_pwritev(void *blk, void *bs, int64_t offset, unsigned int bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %u flags 0x%x"
+blk_co_pwritev(void *blk, void *bs, int64_t offset, int64_t bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %" PRId64 " flags 0x%x"
 blk_root_attach(void *child, void *blk, void *bs) "child %p blk %p bs %p"
 blk_root_detach(void *child, void *blk, void *bs) "child %p blk %p bs %p"
 
-- 
2.31.1

Re: [PATCH 03/12] block-backend: convert blk_co_pwritev_part to int64_t bytes

[Eric Blake]

On Wed, Oct 06, 2021 at 03:17:09PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> We convert blk_do_pwritev_part() and some wrappers:
> blk_co_pwritev_part(), blk_co_pwritev(), blk_co_pwrite_zeroes().
> 
> All functions are converted so that parameter type becomes wider, so
> all callers should be OK with it.
> 
> Look at blk_do_pwritev_part() body:
> bytes is passed to:
> 
>  - trace_blk_co_pwritev (we update it here)
>  - blk_check_byte_request, throttle_group_co_io_limits_intercept,
>    bdrv_co_pwritev_part - all already has int64_t argument.
> 
> Note that requests exceeding INT_MAX are still restricted by
> blk_check_byte_request().
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/sysemu/block-backend.h | 6 +++---
>  block/block-backend.c          | 8 ++++----
>  block/trace-events             | 2 +-
>  3 files changed, 8 insertions(+), 8 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

[PATCH 04/12] block-backend: convert blk_co_pdiscard to int64_t bytes

[Vladimir Sementsov-Ogievskiy]

We updated blk_do_pdiscard() and its wrapper blk_co_pdiscard(). Both
functions are updated so that parameter type becomes wider, so all
callers should be OK with it.

Look at blk_do_pdiscard(): bytes passed only to
blk_check_byte_request() and bdrv_co_pdiscard(), which already has
int64_t bytes parameter, so we are OK.

Note that requests exceeding INT_MAX are still restricted by
blk_check_byte_request().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/sysemu/block-backend.h | 3 ++-
 block/block-backend.c          | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 844bb039c5..398e7abb02 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -181,7 +181,8 @@ void blk_aio_cancel_async(BlockAIOCB *acb);
 int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf);
 BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
                           BlockCompletionFunc *cb, void *opaque);
-int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
+int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset,
+                                 int64_t bytes);
 int blk_co_flush(BlockBackend *blk);
 int blk_flush(BlockBackend *blk);
 int blk_commit_all(void);
diff --git a/block/block-backend.c b/block/block-backend.c
index b09ec5a7c7..e408893985 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1626,7 +1626,7 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
 static int coroutine_fn
-blk_do_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
+blk_do_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes)
 {
     int ret;
 
@@ -1657,7 +1657,8 @@ BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk,
                         cb, opaque);
 }
 
-int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
+int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset,
+                                 int64_t bytes)
 {
     int ret;
 
-- 
2.31.1

Re: [PATCH 04/12] block-backend: convert blk_co_pdiscard to int64_t bytes

[Eric Blake]

On Wed, Oct 06, 2021 at 03:17:10PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> We updated blk_do_pdiscard() and its wrapper blk_co_pdiscard(). Both
> functions are updated so that parameter type becomes wider, so all
> callers should be OK with it.
> 
> Look at blk_do_pdiscard(): bytes passed only to
> blk_check_byte_request() and bdrv_co_pdiscard(), which already has
> int64_t bytes parameter, so we are OK.
> 
> Note that requests exceeding INT_MAX are still restricted by
> blk_check_byte_request().
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/sysemu/block-backend.h | 3 ++-
>  block/block-backend.c          | 5 +++--
>  2 files changed, 5 insertions(+), 3 deletions(-)

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

> 
> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
> index 844bb039c5..398e7abb02 100644
> --- a/include/sysemu/block-backend.h
> +++ b/include/sysemu/block-backend.h
> @@ -181,7 +181,8 @@ void blk_aio_cancel_async(BlockAIOCB *acb);
>  int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf);
>  BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
>                            BlockCompletionFunc *cb, void *opaque);
> -int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
> +int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset,
> +                                 int64_t bytes);

Commit message didn't mention the addition of this label, but it looks
correct.

>  int blk_co_flush(BlockBackend *blk);

I guess fixing blk_co_flush to have the coroutine_fn label would be a
separate patch.  Or, you could rearrange the patches to add the label
on multiple functions independently from type changes.

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

[PATCH 05/12] block-backend: rename _do_ helper functions to _co_do_

[Vladimir Sementsov-Ogievskiy]

This is a preparation to the following commit, to use automatic
coroutine wrapper generation.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/block-backend.c | 52 +++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index e408893985..8100d65b43 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1205,8 +1205,8 @@ static void coroutine_fn blk_wait_while_drained(BlockBackend *blk)
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
 static int coroutine_fn
-blk_do_preadv(BlockBackend *blk, int64_t offset, int64_t bytes,
-              QEMUIOVector *qiov, BdrvRequestFlags flags)
+blk_co_do_preadv(BlockBackend *blk, int64_t offset, int64_t bytes,
+                 QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     int ret;
     BlockDriverState *bs;
@@ -1242,7 +1242,7 @@ int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
     int ret;
 
     blk_inc_in_flight(blk);
-    ret = blk_do_preadv(blk, offset, bytes, qiov, flags);
+    ret = blk_co_do_preadv(blk, offset, bytes, qiov, flags);
     blk_dec_in_flight(blk);
 
     return ret;
@@ -1250,9 +1250,9 @@ int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
 static int coroutine_fn
-blk_do_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes,
-                    QEMUIOVector *qiov, size_t qiov_offset,
-                    BdrvRequestFlags flags)
+blk_co_do_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes,
+                       QEMUIOVector *qiov, size_t qiov_offset,
+                       BdrvRequestFlags flags)
 {
     int ret;
     BlockDriverState *bs;
@@ -1293,7 +1293,7 @@ int coroutine_fn blk_co_pwritev_part(BlockBackend *blk, int64_t offset,
     int ret;
 
     blk_inc_in_flight(blk);
-    ret = blk_do_pwritev_part(blk, offset, bytes, qiov, qiov_offset, flags);
+    ret = blk_co_do_pwritev_part(blk, offset, bytes, qiov, qiov_offset, flags);
     blk_dec_in_flight(blk);
 
     return ret;
@@ -1319,8 +1319,8 @@ static void blk_read_entry(void *opaque)
     BlkRwCo *rwco = opaque;
     QEMUIOVector *qiov = rwco->iobuf;
 
-    rwco->ret = blk_do_preadv(rwco->blk, rwco->offset, qiov->size,
-                              qiov, rwco->flags);
+    rwco->ret = blk_co_do_preadv(rwco->blk, rwco->offset, qiov->size,
+                                 qiov, rwco->flags);
     aio_wait_kick();
 }
 
@@ -1329,8 +1329,8 @@ static void blk_write_entry(void *opaque)
     BlkRwCo *rwco = opaque;
     QEMUIOVector *qiov = rwco->iobuf;
 
-    rwco->ret = blk_do_pwritev_part(rwco->blk, rwco->offset, qiov->size,
-                                    qiov, 0, rwco->flags);
+    rwco->ret = blk_co_do_pwritev_part(rwco->blk, rwco->offset, qiov->size,
+                                       qiov, 0, rwco->flags);
     aio_wait_kick();
 }
 
@@ -1483,8 +1483,8 @@ static void blk_aio_read_entry(void *opaque)
     QEMUIOVector *qiov = rwco->iobuf;
 
     assert(qiov->size == acb->bytes);
-    rwco->ret = blk_do_preadv(rwco->blk, rwco->offset, acb->bytes,
-                              qiov, rwco->flags);
+    rwco->ret = blk_co_do_preadv(rwco->blk, rwco->offset, acb->bytes,
+                                 qiov, rwco->flags);
     blk_aio_complete(acb);
 }
 
@@ -1495,8 +1495,8 @@ static void blk_aio_write_entry(void *opaque)
     QEMUIOVector *qiov = rwco->iobuf;
 
     assert(!qiov || qiov->size == acb->bytes);
-    rwco->ret = blk_do_pwritev_part(rwco->blk, rwco->offset, acb->bytes,
-                                    qiov, 0, rwco->flags);
+    rwco->ret = blk_co_do_pwritev_part(rwco->blk, rwco->offset, acb->bytes,
+                                       qiov, 0, rwco->flags);
     blk_aio_complete(acb);
 }
 
@@ -1583,7 +1583,7 @@ void blk_aio_cancel_async(BlockAIOCB *acb)
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
 static int coroutine_fn
-blk_do_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
+blk_co_do_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
 {
     blk_wait_while_drained(blk);
 
@@ -1599,7 +1599,7 @@ static void blk_ioctl_entry(void *opaque)
     BlkRwCo *rwco = opaque;
     QEMUIOVector *qiov = rwco->iobuf;
 
-    rwco->ret = blk_do_ioctl(rwco->blk, rwco->offset, qiov->iov[0].iov_base);
+    rwco->ret = blk_co_do_ioctl(rwco->blk, rwco->offset, qiov->iov[0].iov_base);
     aio_wait_kick();
 }
 
@@ -1613,7 +1613,7 @@ static void blk_aio_ioctl_entry(void *opaque)
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
 
-    rwco->ret = blk_do_ioctl(rwco->blk, rwco->offset, rwco->iobuf);
+    rwco->ret = blk_co_do_ioctl(rwco->blk, rwco->offset, rwco->iobuf);
 
     blk_aio_complete(acb);
 }
@@ -1626,7 +1626,7 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
 static int coroutine_fn
-blk_do_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes)
+blk_co_do_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes)
 {
     int ret;
 
@@ -1645,7 +1645,7 @@ static void blk_aio_pdiscard_entry(void *opaque)
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
 
-    rwco->ret = blk_do_pdiscard(rwco->blk, rwco->offset, acb->bytes);
+    rwco->ret = blk_co_do_pdiscard(rwco->blk, rwco->offset, acb->bytes);
     blk_aio_complete(acb);
 }
 
@@ -1663,7 +1663,7 @@ int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset,
     int ret;
 
     blk_inc_in_flight(blk);
-    ret = blk_do_pdiscard(blk, offset, bytes);
+    ret = blk_co_do_pdiscard(blk, offset, bytes);
     blk_dec_in_flight(blk);
 
     return ret;
@@ -1674,7 +1674,7 @@ static void blk_pdiscard_entry(void *opaque)
     BlkRwCo *rwco = opaque;
     QEMUIOVector *qiov = rwco->iobuf;
 
-    rwco->ret = blk_do_pdiscard(rwco->blk, rwco->offset, qiov->size);
+    rwco->ret = blk_co_do_pdiscard(rwco->blk, rwco->offset, qiov->size);
     aio_wait_kick();
 }
 
@@ -1684,7 +1684,7 @@ int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
 }
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
-static int coroutine_fn blk_do_flush(BlockBackend *blk)
+static int coroutine_fn blk_co_do_flush(BlockBackend *blk)
 {
     blk_wait_while_drained(blk);
 
@@ -1700,7 +1700,7 @@ static void blk_aio_flush_entry(void *opaque)
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
 
-    rwco->ret = blk_do_flush(rwco->blk);
+    rwco->ret = blk_co_do_flush(rwco->blk);
     blk_aio_complete(acb);
 }
 
@@ -1715,7 +1715,7 @@ int coroutine_fn blk_co_flush(BlockBackend *blk)
     int ret;
 
     blk_inc_in_flight(blk);
-    ret = blk_do_flush(blk);
+    ret = blk_co_do_flush(blk);
     blk_dec_in_flight(blk);
 
     return ret;
@@ -1724,7 +1724,7 @@ int coroutine_fn blk_co_flush(BlockBackend *blk)
 static void blk_flush_entry(void *opaque)
 {
     BlkRwCo *rwco = opaque;
-    rwco->ret = blk_do_flush(rwco->blk);
+    rwco->ret = blk_co_do_flush(rwco->blk);
     aio_wait_kick();
 }
 
-- 
2.31.1

Re: [PATCH 05/12] block-backend: rename _do_ helper functions to _co_do_

[Eric Blake]

On Wed, Oct 06, 2021 at 03:17:11PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> This is a preparation to the following commit, to use automatic
> coroutine wrapper generation.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/block-backend.c | 52 +++++++++++++++++++++----------------------
>  1 file changed, 26 insertions(+), 26 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

[PATCH 06/12] block-coroutine-wrapper.py: support BlockBackend first argument

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/coroutines.h                 |  3 +++
 scripts/block-coroutine-wrapper.py | 12 ++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/block/coroutines.h b/block/coroutines.h
index 514d169d23..35a6c49857 100644
--- a/block/coroutines.h
+++ b/block/coroutines.h
@@ -27,6 +27,9 @@
 
 #include "block/block_int.h"
 
+/* For blk_bs() in generated block/block-gen.c */
+#include "sysemu/block-backend.h"
+
 int coroutine_fn bdrv_co_check(BlockDriverState *bs,
                                BdrvCheckResult *res, BdrvCheckMode fix);
 int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp);
diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index 85dbeb9ecf..08be813407 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -100,12 +100,20 @@ def snake_to_camel(func_name: str) -> str:
 def gen_wrapper(func: FuncDecl) -> str:
     assert not '_co_' in func.name
     assert func.return_type == 'int'
-    assert func.args[0].type in ['BlockDriverState *', 'BdrvChild *']
+    assert func.args[0].type in ['BlockDriverState *', 'BdrvChild *',
+                                 'BlockBackend *']
 
     subsystem, subname = func.name.split('_', 1)
 
     name = f'{subsystem}_co_{subname}'
-    bs = 'bs' if func.args[0].type == 'BlockDriverState *' else 'child->bs'
+
+    t = func.args[0].type
+    if t == 'BlockDriverState *':
+        bs = 'bs'
+    elif t == 'BdrvChild *':
+        bs = 'child->bs'
+    else:
+        bs = 'blk_bs(blk)'
     struct_name = snake_to_camel(name)
 
     return f"""\
-- 
2.31.1

Re: [PATCH 06/12] block-coroutine-wrapper.py: support BlockBackend first argument

[Eric Blake]

On Wed, Oct 06, 2021 at 03:17:12PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/coroutines.h                 |  3 +++
>  scripts/block-coroutine-wrapper.py | 12 ++++++++++--
>  2 files changed, 13 insertions(+), 2 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

[PATCH 07/12] block-backend: drop blk_prw, use block-coroutine-wrapper

[Vladimir Sementsov-Ogievskiy]

Let's drop hand maid coroutine wrappers and use coroutine wrapper
generation like in block/io.c.

Now, blk_foo() functions are written in same way as blk_co_foo() ones,
but wrap blk_do_foo() instead of blk_co_do_foo().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/coroutines.h    |  30 ++++++++
 block/block-backend.c | 155 ++++++++++++++++--------------------------
 2 files changed, 90 insertions(+), 95 deletions(-)

diff --git a/block/coroutines.h b/block/coroutines.h
index 35a6c49857..c8c14a29c8 100644
--- a/block/coroutines.h
+++ b/block/coroutines.h
@@ -75,4 +75,34 @@ int coroutine_fn
 nbd_co_do_establish_connection(BlockDriverState *bs, Error **errp);
 
 
+int generated_co_wrapper
+blk_do_preadv(BlockBackend *blk, int64_t offset, int64_t bytes,
+              QEMUIOVector *qiov, BdrvRequestFlags flags);
+int coroutine_fn
+blk_co_do_preadv(BlockBackend *blk, int64_t offset, int64_t bytes,
+                 QEMUIOVector *qiov, BdrvRequestFlags flags);
+
+
+int generated_co_wrapper
+blk_do_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes,
+                    QEMUIOVector *qiov, size_t qiov_offset,
+                    BdrvRequestFlags flags);
+int coroutine_fn
+blk_co_do_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes,
+                       QEMUIOVector *qiov, size_t qiov_offset,
+                       BdrvRequestFlags flags);
+
+int generated_co_wrapper
+blk_do_ioctl(BlockBackend *blk, unsigned long int req, void *buf);
+int coroutine_fn
+blk_co_do_ioctl(BlockBackend *blk, unsigned long int req, void *buf);
+
+int generated_co_wrapper
+blk_do_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes);
+int coroutine_fn
+blk_co_do_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes);
+
+int generated_co_wrapper blk_do_flush(BlockBackend *blk);
+int coroutine_fn blk_co_do_flush(BlockBackend *blk);
+
 #endif /* BLOCK_COROUTINES_INT_H */
diff --git a/block/block-backend.c b/block/block-backend.c
index 8100d65b43..403cecea98 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -14,6 +14,7 @@
 #include "sysemu/block-backend.h"
 #include "block/block_int.h"
 #include "block/blockjob.h"
+#include "block/coroutines.h"
 #include "block/throttle-groups.h"
 #include "hw/qdev-core.h"
 #include "sysemu/blockdev.h"
@@ -1204,7 +1205,7 @@ static void coroutine_fn blk_wait_while_drained(BlockBackend *blk)
 }
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
-static int coroutine_fn
+int coroutine_fn
 blk_co_do_preadv(BlockBackend *blk, int64_t offset, int64_t bytes,
                  QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
@@ -1249,7 +1250,7 @@ int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
 }
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
-static int coroutine_fn
+int coroutine_fn
 blk_co_do_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes,
                        QEMUIOVector *qiov, size_t qiov_offset,
                        BdrvRequestFlags flags)
@@ -1306,6 +1307,20 @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
     return blk_co_pwritev_part(blk, offset, bytes, qiov, 0, flags);
 }
 
+static int coroutine_fn blk_pwritev_part(BlockBackend *blk, int64_t offset,
+                                         int64_t bytes,
+                                         QEMUIOVector *qiov, size_t qiov_offset,
+                                         BdrvRequestFlags flags)
+{
+    int ret;
+
+    blk_inc_in_flight(blk);
+    ret = blk_do_pwritev_part(blk, offset, bytes, qiov, qiov_offset, flags);
+    blk_dec_in_flight(blk);
+
+    return ret;
+}
+
 typedef struct BlkRwCo {
     BlockBackend *blk;
     int64_t offset;
@@ -1314,58 +1329,11 @@ typedef struct BlkRwCo {
     BdrvRequestFlags flags;
 } BlkRwCo;
 
-static void blk_read_entry(void *opaque)
-{
-    BlkRwCo *rwco = opaque;
-    QEMUIOVector *qiov = rwco->iobuf;
-
-    rwco->ret = blk_co_do_preadv(rwco->blk, rwco->offset, qiov->size,
-                                 qiov, rwco->flags);
-    aio_wait_kick();
-}
-
-static void blk_write_entry(void *opaque)
-{
-    BlkRwCo *rwco = opaque;
-    QEMUIOVector *qiov = rwco->iobuf;
-
-    rwco->ret = blk_co_do_pwritev_part(rwco->blk, rwco->offset, qiov->size,
-                                       qiov, 0, rwco->flags);
-    aio_wait_kick();
-}
-
-static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
-                   int64_t bytes, CoroutineEntry co_entry,
-                   BdrvRequestFlags flags)
-{
-    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
-    BlkRwCo rwco = {
-        .blk    = blk,
-        .offset = offset,
-        .iobuf  = &qiov,
-        .flags  = flags,
-        .ret    = NOT_DONE,
-    };
-
-    blk_inc_in_flight(blk);
-    if (qemu_in_coroutine()) {
-        /* Fast-path if already in coroutine context */
-        co_entry(&rwco);
-    } else {
-        Coroutine *co = qemu_coroutine_create(co_entry, &rwco);
-        bdrv_coroutine_enter(blk_bs(blk), co);
-        BDRV_POLL_WHILE(blk_bs(blk), rwco.ret == NOT_DONE);
-    }
-    blk_dec_in_flight(blk);
-
-    return rwco.ret;
-}
-
 int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                       int bytes, BdrvRequestFlags flags)
 {
-    return blk_prw(blk, offset, NULL, bytes, blk_write_entry,
-                   flags | BDRV_REQ_ZERO_WRITE);
+    return blk_pwritev_part(blk, offset, bytes, NULL, 0,
+                            flags | BDRV_REQ_ZERO_WRITE);
 }
 
 int blk_make_zero(BlockBackend *blk, BdrvRequestFlags flags)
@@ -1510,22 +1478,25 @@ BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset,
 
 int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int count)
 {
-    int ret = blk_prw(blk, offset, buf, count, blk_read_entry, 0);
-    if (ret < 0) {
-        return ret;
-    }
-    return count;
+    int ret;
+    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, count);
+
+    blk_inc_in_flight(blk);
+    ret = blk_do_preadv(blk, offset, count, &qiov, 0);
+    blk_dec_in_flight(blk);
+
+    return ret < 0 ? ret : count;
 }
 
 int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int count,
                BdrvRequestFlags flags)
 {
-    int ret = blk_prw(blk, offset, (void *) buf, count, blk_write_entry,
-                      flags);
-    if (ret < 0) {
-        return ret;
-    }
-    return count;
+    int ret;
+    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, count);
+
+    ret = blk_pwritev_part(blk, offset, count, &qiov, 0, flags);
+
+    return ret < 0 ? ret : count;
 }
 
 int64_t blk_getlength(BlockBackend *blk)
@@ -1582,7 +1553,7 @@ void blk_aio_cancel_async(BlockAIOCB *acb)
 }
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
-static int coroutine_fn
+int coroutine_fn
 blk_co_do_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
 {
     blk_wait_while_drained(blk);
@@ -1594,18 +1565,15 @@ blk_co_do_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
     return bdrv_co_ioctl(blk_bs(blk), req, buf);
 }
 
-static void blk_ioctl_entry(void *opaque)
-{
-    BlkRwCo *rwco = opaque;
-    QEMUIOVector *qiov = rwco->iobuf;
-
-    rwco->ret = blk_co_do_ioctl(rwco->blk, rwco->offset, qiov->iov[0].iov_base);
-    aio_wait_kick();
-}
-
 int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
 {
-    return blk_prw(blk, req, buf, 0, blk_ioctl_entry, 0);
+    int ret;
+
+    blk_inc_in_flight(blk);
+    ret = blk_do_ioctl(blk, req, buf);
+    blk_dec_in_flight(blk);
+
+    return ret;
 }
 
 static void blk_aio_ioctl_entry(void *opaque)
@@ -1625,7 +1593,7 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
 }
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
-static int coroutine_fn
+int coroutine_fn
 blk_co_do_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes)
 {
     int ret;
@@ -1669,22 +1637,19 @@ int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset,
     return ret;
 }
 
-static void blk_pdiscard_entry(void *opaque)
-{
-    BlkRwCo *rwco = opaque;
-    QEMUIOVector *qiov = rwco->iobuf;
-
-    rwco->ret = blk_co_do_pdiscard(rwco->blk, rwco->offset, qiov->size);
-    aio_wait_kick();
-}
-
 int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
 {
-    return blk_prw(blk, offset, NULL, bytes, blk_pdiscard_entry, 0);
+    int ret;
+
+    blk_inc_in_flight(blk);
+    ret = blk_do_pdiscard(blk, offset, bytes);
+    blk_dec_in_flight(blk);
+
+    return ret;
 }
 
 /* To be called between exactly one pair of blk_inc/dec_in_flight() */
-static int coroutine_fn blk_co_do_flush(BlockBackend *blk)
+int coroutine_fn blk_co_do_flush(BlockBackend *blk)
 {
     blk_wait_while_drained(blk);
 
@@ -1721,16 +1686,15 @@ int coroutine_fn blk_co_flush(BlockBackend *blk)
     return ret;
 }
 
-static void blk_flush_entry(void *opaque)
-{
-    BlkRwCo *rwco = opaque;
-    rwco->ret = blk_co_do_flush(rwco->blk);
-    aio_wait_kick();
-}
-
 int blk_flush(BlockBackend *blk)
 {
-    return blk_prw(blk, 0, NULL, 0, blk_flush_entry, 0);
+    int ret;
+
+    blk_inc_in_flight(blk);
+    ret = blk_do_flush(blk);
+    blk_dec_in_flight(blk);
+
+    return ret;
 }
 
 void blk_drain(BlockBackend *blk)
@@ -2218,8 +2182,9 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
 int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
                           int count)
 {
-    return blk_prw(blk, offset, (void *) buf, count, blk_write_entry,
-                   BDRV_REQ_WRITE_COMPRESSED);
+    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, count);
+    return blk_pwritev_part(blk, offset, count, &qiov, 0,
+                            BDRV_REQ_WRITE_COMPRESSED);
 }
 
 int blk_truncate(BlockBackend *blk, int64_t offset, bool exact,
-- 
2.31.1

Re: [PATCH 07/12] block-backend: drop blk_prw, use block-coroutine-wrapper

[Eric Blake]

On Wed, Oct 06, 2021 at 03:17:13PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> Let's drop hand maid coroutine wrappers and use coroutine wrapper

s/maid/made/

> generation like in block/io.c.
> 
> Now, blk_foo() functions are written in same way as blk_co_foo() ones,
> but wrap blk_do_foo() instead of blk_co_do_foo().
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/coroutines.h    |  30 ++++++++
>  block/block-backend.c | 155 ++++++++++++++++--------------------------
>  2 files changed, 90 insertions(+), 95 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

[PATCH 08/12] block-backend: convert blk_foo wrappers to use int64_t bytes parameter

[Vladimir Sementsov-Ogievskiy]

Convert blk_pdiscard, blk_pwrite_compressed, blk_pwrite_zeroes.
These are just wrappers for functions with int64_t argument, so allow
passing int64_t as well. Parameter type becomes wider so all callers
should be OK with it.

Note that requests exceeding INT_MAX are still restricted by
blk_check_byte_request().

Note also that we don't (and are not going to) convert blk_pwrite and
blk_pread: these functions returns number of bytes on success, so to
update them, we should change return type to int64_t as well, which
will lead to investigating and updating all callers which is too much.

So, blk_pread and blk_pwrite remain unchanged.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/sysemu/block-backend.h |  6 +++---
 block/block-backend.c          | 10 +++++-----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 398e7abb02..69f5cfb1c5 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -155,7 +155,7 @@ static inline int coroutine_fn blk_co_pwrite(BlockBackend *blk, int64_t offset,
 }
 
 int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
-                      int bytes, BdrvRequestFlags flags);
+                      int64_t bytes, BdrvRequestFlags flags);
 BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                                   int bytes, BdrvRequestFlags flags,
                                   BlockCompletionFunc *cb, void *opaque);
@@ -245,10 +245,10 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
 int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                                       int64_t bytes, BdrvRequestFlags flags);
 int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
-                          int bytes);
+                          int64_t bytes);
 int blk_truncate(BlockBackend *blk, int64_t offset, bool exact,
                  PreallocMode prealloc, BdrvRequestFlags flags, Error **errp);
-int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
+int blk_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes);
 int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
                      int64_t pos, int size);
 int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size);
diff --git a/block/block-backend.c b/block/block-backend.c
index 403cecea98..018684a203 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1330,7 +1330,7 @@ typedef struct BlkRwCo {
 } BlkRwCo;
 
 int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
-                      int bytes, BdrvRequestFlags flags)
+                      int64_t bytes, BdrvRequestFlags flags)
 {
     return blk_pwritev_part(blk, offset, bytes, NULL, 0,
                             flags | BDRV_REQ_ZERO_WRITE);
@@ -1637,7 +1637,7 @@ int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset,
     return ret;
 }
 
-int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
+int blk_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes)
 {
     int ret;
 
@@ -2180,10 +2180,10 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
 }
 
 int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
-                          int count)
+                          int64_t bytes)
 {
-    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, count);
-    return blk_pwritev_part(blk, offset, count, &qiov, 0,
+    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
+    return blk_pwritev_part(blk, offset, bytes, &qiov, 0,
                             BDRV_REQ_WRITE_COMPRESSED);
 }
 
-- 
2.31.1

Re: [PATCH 08/12] block-backend: convert blk_foo wrappers to use int64_t bytes parameter

[Eric Blake]

On Wed, Oct 06, 2021 at 03:17:14PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> Convert blk_pdiscard, blk_pwrite_compressed, blk_pwrite_zeroes.
> These are just wrappers for functions with int64_t argument, so allow
> passing int64_t as well. Parameter type becomes wider so all callers
> should be OK with it.
> 
> Note that requests exceeding INT_MAX are still restricted by
> blk_check_byte_request().
> 
> Note also that we don't (and are not going to) convert blk_pwrite and
> blk_pread: these functions returns number of bytes on success, so to
> update them, we should change return type to int64_t as well, which
> will lead to investigating and updating all callers which is too much.
> 
> So, blk_pread and blk_pwrite remain unchanged.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/sysemu/block-backend.h |  6 +++---
>  block/block-backend.c          | 10 +++++-----
>  2 files changed, 8 insertions(+), 8 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

[PATCH 09/12] block-backend: convert blk_co_copy_range to int64_t bytes

[Vladimir Sementsov-Ogievskiy]

Function is updated so that parameter type becomes wider, so all
callers should be OK with it.

Look at blk_co_copy_range() itself: bytes passed only to
blk_check_byte_request() and bdrv_co_copy_range(), which already has
int64_t bytes parameter, so we are OK.

Note that requests exceeding INT_MAX are still restricted by
blk_check_byte_request().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/sysemu/block-backend.h | 2 +-
 block/block-backend.c          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 69f5cfb1c5..134c442754 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -269,7 +269,7 @@ void blk_unregister_buf(BlockBackend *blk, void *host);
 
 int coroutine_fn blk_co_copy_range(BlockBackend *blk_in, int64_t off_in,
                                    BlockBackend *blk_out, int64_t off_out,
-                                   int bytes, BdrvRequestFlags read_flags,
+                                   int64_t bytes, BdrvRequestFlags read_flags,
                                    BdrvRequestFlags write_flags);
 
 const BdrvChild *blk_root(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index 018684a203..f051ea00e9 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2412,7 +2412,7 @@ void blk_unregister_buf(BlockBackend *blk, void *host)
 
 int coroutine_fn blk_co_copy_range(BlockBackend *blk_in, int64_t off_in,
                                    BlockBackend *blk_out, int64_t off_out,
-                                   int bytes, BdrvRequestFlags read_flags,
+                                   int64_t bytes, BdrvRequestFlags read_flags,
                                    BdrvRequestFlags write_flags)
 {
     int r;
-- 
2.31.1

Re: [PATCH 09/12] block-backend: convert blk_co_copy_range to int64_t bytes

[Eric Blake]

On Wed, Oct 06, 2021 at 03:17:15PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> Function is updated so that parameter type becomes wider, so all
> callers should be OK with it.
> 
> Look at blk_co_copy_range() itself: bytes passed only to
> blk_check_byte_request() and bdrv_co_copy_range(), which already has
> int64_t bytes parameter, so we are OK.
> 
> Note that requests exceeding INT_MAX are still restricted by
> blk_check_byte_request().
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/sysemu/block-backend.h | 2 +-
>  block/block-backend.c          | 2 +-
>  2 files changed, 2 insertions(+), 2 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

[PATCH 10/12] block-backend: convert blk_aio_ functions to int64_t bytes paramter

[Vladimir Sementsov-Ogievskiy]

1. Convert bytes in BlkAioEmAIOCB:
  aio->bytes is only passed to already int64_t interfaces, and set in
  blk_aio_prwv, which is updated here.

2. For all updated functions parameter type becomes wider so callers
   are safe.

3. In blk_aio_prwv we only store bytes to BlkAioEmAIOCB, which is
   updated here.

4. Other updated functions are wrappers on blk_aio_prwv.

Note that blk_aio_preadv and blk_aio_pwritev become safer: before this
commit, it's theoretically possible to pass qiov with size exceeding
INT_MAX, which than converted to int argument of blk_aio_prwv. Now it's
converted to int64_t which is a lot better. Still add assertions.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/sysemu/block-backend.h |  4 ++--
 block/block-backend.c          | 13 ++++++++-----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 134c442754..979829b325 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -157,7 +157,7 @@ static inline int coroutine_fn blk_co_pwrite(BlockBackend *blk, int64_t offset,
 int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                       int64_t bytes, BdrvRequestFlags flags);
 BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset,
-                                  int bytes, BdrvRequestFlags flags,
+                                  int64_t bytes, BdrvRequestFlags flags,
                                   BlockCompletionFunc *cb, void *opaque);
 int blk_make_zero(BlockBackend *blk, BdrvRequestFlags flags);
 int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int bytes);
@@ -174,7 +174,7 @@ BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t offset,
                             BlockCompletionFunc *cb, void *opaque);
 BlockAIOCB *blk_aio_flush(BlockBackend *blk,
                           BlockCompletionFunc *cb, void *opaque);
-BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk, int64_t offset, int bytes,
+BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes,
                              BlockCompletionFunc *cb, void *opaque);
 void blk_aio_cancel(BlockAIOCB *acb);
 void blk_aio_cancel_async(BlockAIOCB *acb);
diff --git a/block/block-backend.c b/block/block-backend.c
index f051ea00e9..ef0f65be4b 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1380,7 +1380,7 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk,
 typedef struct BlkAioEmAIOCB {
     BlockAIOCB common;
     BlkRwCo rwco;
-    int bytes;
+    int64_t bytes;
     bool has_returned;
 } BlkAioEmAIOCB;
 
@@ -1412,7 +1412,8 @@ static void blk_aio_complete_bh(void *opaque)
     blk_aio_complete(acb);
 }
 
-static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,
+static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset,
+                                int64_t bytes,
                                 void *iobuf, CoroutineEntry co_entry,
                                 BdrvRequestFlags flags,
                                 BlockCompletionFunc *cb, void *opaque)
@@ -1469,10 +1470,10 @@ static void blk_aio_write_entry(void *opaque)
 }
 
 BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset,
-                                  int count, BdrvRequestFlags flags,
+                                  int64_t bytes, BdrvRequestFlags flags,
                                   BlockCompletionFunc *cb, void *opaque)
 {
-    return blk_aio_prwv(blk, offset, count, NULL, blk_aio_write_entry,
+    return blk_aio_prwv(blk, offset, bytes, NULL, blk_aio_write_entry,
                         flags | BDRV_REQ_ZERO_WRITE, cb, opaque);
 }
 
@@ -1530,6 +1531,7 @@ BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
                            QEMUIOVector *qiov, BdrvRequestFlags flags,
                            BlockCompletionFunc *cb, void *opaque)
 {
+    assert(qiov->size <= INT64_MAX);
     return blk_aio_prwv(blk, offset, qiov->size, qiov,
                         blk_aio_read_entry, flags, cb, opaque);
 }
@@ -1538,6 +1540,7 @@ BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t offset,
                             QEMUIOVector *qiov, BdrvRequestFlags flags,
                             BlockCompletionFunc *cb, void *opaque)
 {
+    assert(qiov->size <= INT64_MAX);
     return blk_aio_prwv(blk, offset, qiov->size, qiov,
                         blk_aio_write_entry, flags, cb, opaque);
 }
@@ -1618,7 +1621,7 @@ static void blk_aio_pdiscard_entry(void *opaque)
 }
 
 BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk,
-                             int64_t offset, int bytes,
+                             int64_t offset, int64_t bytes,
                              BlockCompletionFunc *cb, void *opaque)
 {
     return blk_aio_prwv(blk, offset, bytes, NULL, blk_aio_pdiscard_entry, 0,
-- 
2.31.1

Re: [PATCH 10/12] block-backend: convert blk_aio_ functions to int64_t bytes paramter

[Eric Blake]

On Wed, Oct 06, 2021 at 03:17:16PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> 1. Convert bytes in BlkAioEmAIOCB:
>   aio->bytes is only passed to already int64_t interfaces, and set in
>   blk_aio_prwv, which is updated here.
> 
> 2. For all updated functions parameter type becomes wider so callers
>    are safe.
> 
> 3. In blk_aio_prwv we only store bytes to BlkAioEmAIOCB, which is
>    updated here.
> 
> 4. Other updated functions are wrappers on blk_aio_prwv.
> 
> Note that blk_aio_preadv and blk_aio_pwritev become safer: before this
> commit, it's theoretically possible to pass qiov with size exceeding
> INT_MAX, which than converted to int argument of blk_aio_prwv. Now it's
> converted to int64_t which is a lot better. Still add assertions.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/sysemu/block-backend.h |  4 ++--
>  block/block-backend.c          | 13 ++++++++-----
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> @@ -1530,6 +1531,7 @@ BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
>                             QEMUIOVector *qiov, BdrvRequestFlags flags,
>                             BlockCompletionFunc *cb, void *opaque)
>  {
> +    assert(qiov->size <= INT64_MAX);

I hope this doesn't cause 32-bit compilers to warn about an
always-true expression; but if it does, we'll figure something out.
That's not enough for me to ask for you to respin this, though, so:

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

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

Re: [PATCH 10/12] block-backend: convert blk_aio_ functions to int64_t bytes paramter

[Vladimir Sementsov-Ogievskiy]

10/6/21 23:29, Eric Blake wrote:
> On Wed, Oct 06, 2021 at 03:17:16PM +0200, Vladimir Sementsov-Ogievskiy wrote:
>> 1. Convert bytes in BlkAioEmAIOCB:
>>    aio->bytes is only passed to already int64_t interfaces, and set in
>>    blk_aio_prwv, which is updated here.
>>
>> 2. For all updated functions parameter type becomes wider so callers
>>     are safe.
>>
>> 3. In blk_aio_prwv we only store bytes to BlkAioEmAIOCB, which is
>>     updated here.
>>
>> 4. Other updated functions are wrappers on blk_aio_prwv.
>>
>> Note that blk_aio_preadv and blk_aio_pwritev become safer: before this
>> commit, it's theoretically possible to pass qiov with size exceeding
>> INT_MAX, which than converted to int argument of blk_aio_prwv. Now it's
>> converted to int64_t which is a lot better. Still add assertions.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   include/sysemu/block-backend.h |  4 ++--
>>   block/block-backend.c          | 13 ++++++++-----
>>   2 files changed, 10 insertions(+), 7 deletions(-)
>>
>> @@ -1530,6 +1531,7 @@ BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
>>                              QEMUIOVector *qiov, BdrvRequestFlags flags,
>>                              BlockCompletionFunc *cb, void *opaque)
>>   {
>> +    assert(qiov->size <= INT64_MAX);
> 
> I hope this doesn't cause 32-bit compilers to warn about an
> always-true expression; but if it does, we'll figure something out.
> That's not enough for me to ask for you to respin this, though, so:
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

So, here we need

assert((uint64_t)qiov->size <= INT64_MAX);

as in recent fix by Hanna.

Eric, will you stage this as continuation of 64bit series, or do we wait for Kevin or Hanna, or for me resending it with this small fix and your wording fixes?

-- 
Best regards,
Vladimir

Re: [PATCH 10/12] block-backend: convert blk_aio_ functions to int64_t bytes paramter

[Eric Blake]

On Tue, Oct 12, 2021 at 07:13:23PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > @@ -1530,6 +1531,7 @@ BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
> > >                              QEMUIOVector *qiov, BdrvRequestFlags flags,
> > >                              BlockCompletionFunc *cb, void *opaque)
> > >   {
> > > +    assert(qiov->size <= INT64_MAX);
> > 
> > I hope this doesn't cause 32-bit compilers to warn about an
> > always-true expression; but if it does, we'll figure something out.
> > That's not enough for me to ask for you to respin this, though, so:
> > 
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > 
> 
> So, here we need
> 
> assert((uint64_t)qiov->size <= INT64_MAX);
> 
> as in recent fix by Hanna.
>

Indeed.

> Eric, will you stage this as continuation of 64bit series, or do we wait for Kevin or Hanna, or for me resending it with this small fix and your wording fixes?

At this point, I think I'm fine touching up the series and staging it
through my tree.

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

Re: [PATCH 10/12] block-backend: convert blk_aio_ functions to int64_t bytes paramter

[Vladimir Sementsov-Ogievskiy]

13.10.2021 00:37, Eric Blake wrote:
> On Tue, Oct 12, 2021 at 07:13:23PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>>>> @@ -1530,6 +1531,7 @@ BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset,
>>>>                               QEMUIOVector *qiov, BdrvRequestFlags flags,
>>>>                               BlockCompletionFunc *cb, void *opaque)
>>>>    {
>>>> +    assert(qiov->size <= INT64_MAX);
>>>
>>> I hope this doesn't cause 32-bit compilers to warn about an
>>> always-true expression; but if it does, we'll figure something out.
>>> That's not enough for me to ask for you to respin this, though, so:
>>>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>>
>>
>> So, here we need
>>
>> assert((uint64_t)qiov->size <= INT64_MAX);
>>
>> as in recent fix by Hanna.
>>
> 
> Indeed.
> 
>> Eric, will you stage this as continuation of 64bit series, or do we wait for Kevin or Hanna, or for me resending it with this small fix and your wording fixes?
> 
> At this point, I think I'm fine touching up the series and staging it
> through my tree.
> 

Great, thanks!

-- 
Best regards,
Vladimir

[PATCH 11/12] block-backend: blk_pread, blk_pwrite: rename count parameter to bytes

[Vladimir Sementsov-Ogievskiy]

To be consistent with declarations in include/sysemu/block-backend.h.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/block-backend.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index ef0f65be4b..e2b363ff63 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1477,27 +1477,27 @@ BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                         flags | BDRV_REQ_ZERO_WRITE, cb, opaque);
 }
 
-int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int count)
+int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int bytes)
 {
     int ret;
-    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, count);
+    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
 
     blk_inc_in_flight(blk);
-    ret = blk_do_preadv(blk, offset, count, &qiov, 0);
+    ret = blk_do_preadv(blk, offset, bytes, &qiov, 0);
     blk_dec_in_flight(blk);
 
-    return ret < 0 ? ret : count;
+    return ret < 0 ? ret : bytes;
 }
 
-int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int count,
+int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int bytes,
                BdrvRequestFlags flags)
 {
     int ret;
-    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, count);
+    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
 
-    ret = blk_pwritev_part(blk, offset, count, &qiov, 0, flags);
+    ret = blk_pwritev_part(blk, offset, bytes, &qiov, 0, flags);
 
-    return ret < 0 ? ret : count;
+    return ret < 0 ? ret : bytes;
 }
 
 int64_t blk_getlength(BlockBackend *blk)
-- 
2.31.1

Re: [PATCH 11/12] block-backend: blk_pread, blk_pwrite: rename count parameter to bytes

[Eric Blake]

On Wed, Oct 06, 2021 at 03:17:17PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> To be consistent with declarations in include/sysemu/block-backend.h.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/block-backend.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 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

[PATCH 12/12] block-backend: drop INT_MAX restriction from blk_check_byte_request()

[Vladimir Sementsov-Ogievskiy]

blk_check_bytes_request is called from blk_co_do_preadv,
blk_co_do_pwritev_part, blk_co_do_pdiscard and blk_co_copy_range
before (maybe) calling throttle_group_co_io_limits_intercept() (which
has int64_t argument) and then calling corresponding bdrv_co_ function.
bdrv_co_ functions are OK with int64_t bytes as well.

So dropping the check for INT_MAX we just get same restrictions as in
bdrv_ layer: discard and write-zeroes goes through
bdrv_check_qiov_request() and are allowed to be 64bit. Other requests
go through bdrv_check_request32() and still restricted by INT_MAX
boundary.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/block-backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index e2b363ff63..21d8e88311 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1166,7 +1166,7 @@ static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
 {
     int64_t len;
 
-    if (bytes < 0 || bytes > INT_MAX) {
+    if (bytes < 0) {
         return -EIO;
     }
 
-- 
2.31.1

Re: [PATCH 12/12] block-backend: drop INT_MAX restriction from blk_check_byte_request()

[Eric Blake]

On Wed, Oct 06, 2021 at 03:17:18PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> blk_check_bytes_request is called from blk_co_do_preadv,
> blk_co_do_pwritev_part, blk_co_do_pdiscard and blk_co_copy_range
> before (maybe) calling throttle_group_co_io_limits_intercept() (which
> has int64_t argument) and then calling corresponding bdrv_co_ function.
> bdrv_co_ functions are OK with int64_t bytes as well.
> 
> So dropping the check for INT_MAX we just get same restrictions as in
> bdrv_ layer: discard and write-zeroes goes through
> bdrv_check_qiov_request() and are allowed to be 64bit. Other requests
> go through bdrv_check_request32() and still restricted by INT_MAX
> boundary.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/block-backend.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Deceptively short, but this is the one where a mistake would hurt.

Thankfully, I agree with your analysis that the call stack is still
checking 32-bit limits on read/write, and that discard/zero requests
can now go up to 63-bit sizing if everything else in the call stack is
ready; plus the fact that we are careful both in our drivers to
document actual limits (whether 32-bit or even smaller), and in the
block code to honor those limits (breaking larger requests into chunks
before reaching this far).

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

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

[PATCH 13/12] block-backend: fix blk_co_flush prototype to mention coroutine_fn

[Vladimir Sementsov-Ogievskiy]

We do have this marker for blk_co_flush function declaration in
block/block-backend.c. Add it in header too.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/sysemu/block-backend.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 979829b325..f3227098fc 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -183,7 +183,7 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
                           BlockCompletionFunc *cb, void *opaque);
 int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset,
                                  int64_t bytes);
-int blk_co_flush(BlockBackend *blk);
+int coroutine_fn blk_co_flush(BlockBackend *blk);
 int blk_flush(BlockBackend *blk);
 int blk_commit_all(void);
 void blk_inc_in_flight(BlockBackend *blk);
-- 
2.31.1

Re: [PATCH 13/12] block-backend: fix blk_co_flush prototype to mention coroutine_fn

[Eric Blake]

On Thu, Oct 07, 2021 at 07:52:42PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> We do have this marker for blk_co_flush function declaration in
> block/block-backend.c. Add it in header too.

Maybe:

We alreaddy have this marker for the blk_co_flush function declaration
in block/block-backend.c.  Add it in the header, too.

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/sysemu/block-backend.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

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

[PATCH 14/12] block-backend: update blk_co_pwrite() and blk_co_pread() wrappers

[Vladimir Sementsov-Ogievskiy]

Make bytes argument int64_t to be consistent with modern block-layer.
Callers should be OK with it as type becomes wider.

What is inside functions?

- Conversion from int64_t to size_t. Still, we
can't have a buffer larger than SIZE_MAX, therefore bytes should not be
larger than SIZE_MAX as well. Add and assertion.

- Passing to blk_co_pwritev() / blk_co_preadv() which already has
  int64_t bytes argument.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/sysemu/block-backend.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index f3227098fc..53c9b3271b 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -137,20 +137,24 @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
                                BdrvRequestFlags flags);
 
 static inline int coroutine_fn blk_co_pread(BlockBackend *blk, int64_t offset,
-                                            unsigned int bytes, void *buf,
+                                            int64_t bytes, void *buf,
                                             BdrvRequestFlags flags)
 {
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
 
+    assert(bytes <= SIZE_MAX);
+
     return blk_co_preadv(blk, offset, bytes, &qiov, flags);
 }
 
 static inline int coroutine_fn blk_co_pwrite(BlockBackend *blk, int64_t offset,
-                                             unsigned int bytes, void *buf,
+                                             int64_t bytes, void *buf,
                                              BdrvRequestFlags flags)
 {
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
 
+    assert(bytes <= SIZE_MAX);
+
     return blk_co_pwritev(blk, offset, bytes, &qiov, flags);
 }
 
-- 
2.31.1

Re: [PATCH 14/12] block-backend: update blk_co_pwrite() and blk_co_pread() wrappers

[Eric Blake]

On Thu, Oct 07, 2021 at 07:52:43PM +0200, Vladimir Sementsov-Ogievskiy wrote:
> Make bytes argument int64_t to be consistent with modern block-layer.
> Callers should be OK with it as type becomes wider.
> 
> What is inside functions?
> 
> - Conversion from int64_t to size_t. Still, we
> can't have a buffer larger than SIZE_MAX, therefore bytes should not be
> larger than SIZE_MAX as well. Add and assertion.

s/and/an/

> 
> - Passing to blk_co_pwritev() / blk_co_preadv() which already has
>   int64_t bytes argument.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/sysemu/block-backend.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 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