[PATCH v3 00/17] 64bit block-layer

[PATCH v3 00/17] 64bit block-layer

[Vladimir Sementsov-Ogievskiy]

Hi all!

We want 64bit write-zeroes, and for this, convert all io functions to
64bit.

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).

Please refer to initial cover-letter 
 https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg08723.html
for more info.

v3 is available at
 https://src.openvz.org/scm/~vsementsov/qemu.git #tag up-64bit-block-layer-v3

v3: Based on "[PATCH v2 0/9] block/io: safer inc/dec in_flight sections"
Add Eric's r-bs, improve commit message with short reasoning of the whole
thing, and Eric's audits (if you don't like something, I'll change or drop for
next series).

Add "Series:" tag to each patch. Just an idea, if it's inappropriate thing,
I'll drop it.

01: add assertion that bytes > 0
02: fix indentation
06: refactor calculations in bdrv_co_write_req_prepare
09,10: simple rebase conflicts solved

Also, cover more drivers by driver-updating patches and fix int flags
to be BdrvRequestFlags flags.

Based-on: <20200427143907.5710-1-vsementsov@virtuozzo.com>
Series: 64bit-block-status

Vladimir Sementsov-Ogievskiy (17):
  block/throttle-groups: throttle_group_co_io_limits_intercept(): 64bit
    bytes
  block: use int64_t as bytes type in tracked requests
  block/io: use int64_t bytes parameter in bdrv_check_byte_request()
  block/io: use int64_t bytes in driver wrappers
  block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes()
  block/io: support int64_t bytes in bdrv_aligned_pwritev()
  block/io: support int64_t bytes in bdrv_co_do_copy_on_readv()
  block/io: support int64_t bytes in bdrv_aligned_preadv()
  block/io: support int64_t bytes in bdrv_co_p{read,write}v_part()
  block/io: support int64_t bytes in read/write wrappers
  block/io: use int64_t bytes in copy_range
  block/block-backend: convert blk io path to use int64_t parameters
  block: use int64_t instead of uint64_t in driver read handlers
  block: use int64_t instead of uint64_t in driver write handlers
  block: use int64_t instead of uint64_t in copy_range driver handlers
  block: use int64_t instead of int in driver write_zeroes handlers
  block: use int64_t instead of int in driver discard handlers

 include/block/block.h           |  17 +++--
 include/block/block_int.h       |  67 ++++++++---------
 include/block/throttle-groups.h |   2 +-
 include/sysemu/block-backend.h  |  26 +++----
 block/backup-top.c              |  14 ++--
 block/blkdebug.c                |  12 +--
 block/blklogwrites.c            |  16 ++--
 block/blkreplay.c               |   8 +-
 block/blkverify.c               |  10 +--
 block/block-backend.c           |  60 +++++++--------
 block/bochs.c                   |   4 +-
 block/cloop.c                   |   4 +-
 block/commit.c                  |   2 +-
 block/copy-on-read.c            |  14 ++--
 block/crypto.c                  |   8 +-
 block/curl.c                    |   3 +-
 block/dmg.c                     |   4 +-
 block/file-posix.c              |  46 ++++++++----
 block/file-win32.c              |   8 +-
 block/filter-compress.c         |  15 ++--
 block/gluster.c                 |  14 ++--
 block/io.c                      | 126 +++++++++++++++++---------------
 block/iscsi.c                   |  34 ++++++---
 block/mirror.c                  |   8 +-
 block/nbd.c                     |  18 +++--
 block/nfs.c                     |  12 +--
 block/null.c                    |  18 +++--
 block/nvme.c                    |  38 +++++++---
 block/qcow.c                    |  16 ++--
 block/qcow2.c                   |  34 +++++----
 block/qed.c                     |  17 ++++-
 block/quorum.c                  |   9 ++-
 block/raw-format.c              |  36 ++++-----
 block/rbd.c                     |  10 ++-
 block/sheepdog.c                |  11 ++-
 block/throttle-groups.c         |   5 +-
 block/throttle.c                |  14 ++--
 block/vdi.c                     |   8 +-
 block/vmdk.c                    |  14 ++--
 block/vpc.c                     |   8 +-
 block/vvfat.c                   |  12 +--
 block/vxhs.c                    |   8 +-
 tests/test-bdrv-drain.c         |  16 ++--
 tests/test-block-iothread.c     |  19 +++--
 block/trace-events              |  14 ++--
 45 files changed, 488 insertions(+), 371 deletions(-)

-- 
2.21.0

[PATCH v3 01/17] block/throttle-groups: throttle_group_co_io_limits_intercept(): 64bit bytes

[Vladimir Sementsov-Ogievskiy]

The function is called from 64bit io handlers, and bytes is just passed
to throttle_account() which is 64bit too (unsigned though). So, let's
convert intermediate argument to 64bit too.

This patch is a first in the 64-bit-blocklayer series, so 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).

Patch-correctness audit by Eric Blake:

  Caller has 32-bit, this patch now causes widening which is safe:
  block/block-backend.c: blk_do_preadv() passes 'unsigned int'
  block/block-backend.c: blk_do_pwritev_part() passes 'unsigned int'
  block/throttle.c: throttle_co_pwrite_zeroes() passes 'int'
  block/throttle.c: throttle_co_pdiscard() passes 'int'

  Caller has 64-bit, this patch fixes potential bug where pre-patch
  could narrow, except it's easy enough to trace that callers are still
  capped at 2G actions:
  block/throttle.c: throttle_co_preadv() passes 'uint64_t'
  block/throttle.c: throttle_co_pwritev() passes 'uint64_t'

  Implementation in question: block/throttle-groups.c
  throttle_group_co_io_limits_intercept() takes 'unsigned int bytes'
  and uses it: argument to util/throttle.c throttle_account(uint64_t)

  All safe: it patches a latent bug, and does not introduce any 64-bit
  gotchas once throttle_co_p{read,write}v are relaxed, and assuming
  throttle_account() is not buggy.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 include/block/throttle-groups.h | 2 +-
 block/throttle-groups.c         | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index 712a8e64b4..f921994b8a 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -76,7 +76,7 @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm);
 void throttle_group_restart_tgm(ThrottleGroupMember *tgm);
 
 void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm,
-                                                        unsigned int bytes,
+                                                        int64_t bytes,
                                                         bool is_write);
 void throttle_group_attach_aio_context(ThrottleGroupMember *tgm,
                                        AioContext *new_context);
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 37695b0cd7..14b74b7b8f 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -358,12 +358,15 @@ static void schedule_next_request(ThrottleGroupMember *tgm, bool is_write)
  * @is_write:  the type of operation (read/write)
  */
 void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm,
-                                                        unsigned int bytes,
+                                                        int64_t bytes,
                                                         bool is_write)
 {
     bool must_wait;
     ThrottleGroupMember *token;
     ThrottleGroup *tg = container_of(tgm->throttle_state, ThrottleGroup, ts);
+
+    assert(bytes >= 0);
+
     qemu_mutex_lock(&tg->lock);
 
     /* First we check if this I/O has to be throttled. */
-- 
2.21.0

Re: [PATCH v3 01/17] block/throttle-groups: throttle_group_co_io_limits_intercept(): 64bit bytes

[Alberto Garcia]

On Thu 30 Apr 2020 01:10:17 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> The function is called from 64bit io handlers, and bytes is just passed
> to throttle_account() which is 64bit too (unsigned though). So, let's
> convert intermediate argument to 64bit too.
>
> This patch is a first in the 64-bit-blocklayer series, so 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).
>
> Patch-correctness audit by Eric Blake:
>
>   Caller has 32-bit, this patch now causes widening which is safe:
>   block/block-backend.c: blk_do_preadv() passes 'unsigned int'
>   block/block-backend.c: blk_do_pwritev_part() passes 'unsigned int'
>   block/throttle.c: throttle_co_pwrite_zeroes() passes 'int'
>   block/throttle.c: throttle_co_pdiscard() passes 'int'
>
>   Caller has 64-bit, this patch fixes potential bug where pre-patch
>   could narrow, except it's easy enough to trace that callers are still
>   capped at 2G actions:
>   block/throttle.c: throttle_co_preadv() passes 'uint64_t'
>   block/throttle.c: throttle_co_pwritev() passes 'uint64_t'
>
>   Implementation in question: block/throttle-groups.c
>   throttle_group_co_io_limits_intercept() takes 'unsigned int bytes'
>   and uses it: argument to util/throttle.c throttle_account(uint64_t)
>
>   All safe: it patches a latent bug, and does not introduce any 64-bit
>   gotchas once throttle_co_p{read,write}v are relaxed, and assuming
>   throttle_account() is not buggy.
>
> Series: 64bit-block-status
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

[PATCH v3 02/17] block: use int64_t as bytes type in tracked requests

[Vladimir Sementsov-Ogievskiy]

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 tracked requests now.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 include/block/block_int.h |  4 ++--
 block/io.c                | 11 ++++++-----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 4c3587ea19..c8daba608b 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -70,12 +70,12 @@ enum BdrvTrackedRequestType {
 typedef struct BdrvTrackedRequest {
     BlockDriverState *bs;
     int64_t offset;
-    uint64_t bytes;
+    int64_t bytes;
     enum BdrvTrackedRequestType type;
 
     bool serialising;
     int64_t overlap_offset;
-    uint64_t overlap_bytes;
+    int64_t overlap_bytes;
 
     QLIST_ENTRY(BdrvTrackedRequest) list;
     Coroutine *co; /* owner, used for deadlock detection */
diff --git a/block/io.c b/block/io.c
index cd5374e6c7..20ebf3c536 100644
--- a/block/io.c
+++ b/block/io.c
@@ -692,10 +692,11 @@ static void tracked_request_end(BdrvTrackedRequest *req)
 static void tracked_request_begin(BdrvTrackedRequest *req,
                                   BlockDriverState *bs,
                                   int64_t offset,
-                                  uint64_t bytes,
+                                  int64_t bytes,
                                   enum BdrvTrackedRequestType type)
 {
-    assert(bytes <= INT64_MAX && offset <= INT64_MAX - bytes);
+    assert(offset >= 0 && bytes >= 0 &&
+           bytes <= INT64_MAX && offset <= INT64_MAX - bytes);
 
     *req = (BdrvTrackedRequest){
         .bs = bs,
@@ -716,7 +717,7 @@ static void tracked_request_begin(BdrvTrackedRequest *req,
 }
 
 static bool tracked_request_overlaps(BdrvTrackedRequest *req,
-                                     int64_t offset, uint64_t bytes)
+                                     int64_t offset, int64_t bytes)
 {
     /*        aaaa   bbbb */
     if (offset >= req->overlap_offset + req->overlap_bytes) {
@@ -773,8 +774,8 @@ bool bdrv_mark_request_serialising(BdrvTrackedRequest *req, uint64_t align)
 {
     BlockDriverState *bs = req->bs;
     int64_t overlap_offset = req->offset & ~(align - 1);
-    uint64_t overlap_bytes = ROUND_UP(req->offset + req->bytes, align)
-                               - overlap_offset;
+    int64_t overlap_bytes =
+        ROUND_UP(req->offset + req->bytes, align) - overlap_offset;
     bool waited;
 
     qemu_co_mutex_lock(&bs->reqs_lock);
-- 
2.21.0

Re: [PATCH v3 02/17] block: use int64_t as bytes type in tracked requests

[Alberto Garcia]

On Thu 30 Apr 2020 01:10:18 PM CEST, 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 tracked requests now.
>
> Series: 64bit-block-status
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [PATCH v3 02/17] block: use int64_t as bytes type in tracked requests

[Eric Blake]

On 4/30/20 6:10 AM, 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 tracked requests now.
> 
> Series: 64bit-block-status
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---

>   static void tracked_request_begin(BdrvTrackedRequest *req,
>                                     BlockDriverState *bs,
>                                     int64_t offset,
> -                                  uint64_t bytes,
> +                                  int64_t bytes,
>                                     enum BdrvTrackedRequestType type)
>   {
> -    assert(bytes <= INT64_MAX && offset <= INT64_MAX - bytes);
> +    assert(offset >= 0 && bytes >= 0 &&
> +           bytes <= INT64_MAX && offset <= INT64_MAX - bytes);

'bytes <= INT64_MAX' was previously a real runtime check, but is now a 
tautology and therefore a dead branch; a picky compiler might complain. 
This assert could be compressed to:

assert(offset >= 0 && (uint64_t) bytes <= INT64_MAX - offset);

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

[PATCH v3 03/17] block/io: use int64_t bytes parameter in bdrv_check_byte_request()

[Vladimir Sementsov-Ogievskiy]

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 bdrv_check_byte_request() now.

Patch-correctness audit by Eric Blake:

  This changes an unsigned to signed value on 64-bit machines, and
  additionally widens the parameter on 32-bit machines.  Existing
  callers:

  bdrv_co_preadv_part() with 'unsigned int' - no impact
  bdrv_co_pwritev_part() with 'unsigned int' - no impact
  bdrv_co_copy_range_internal() with 'uint64_t' -
      potentially fixes a latent bug on 32-bit machines. Requires a
      larger audit to see how bdrv_co_copy_range() and friends are
      used:

  block/block-backend.c:blk_co_copy_range() - passes 'int', thus < 2G
  block/block-copy.c:block_copy_do_copy() -
      passes 'int64_t', but only after assert(nbytes < INT_MAX); also
      it has a BLOCK_COPY_MAX_COPY_RANGE set to 16M that factors into
      its calculations on how much to do per iteration

  So it looks like at present we are safe.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/io.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/io.c b/block/io.c
index 20ebf3c536..7a7d4e578d 100644
--- a/block/io.c
+++ b/block/io.c
@@ -875,9 +875,9 @@ static bool coroutine_fn bdrv_wait_serialising_requests(BdrvTrackedRequest *self
 }
 
 static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
-                                   size_t size)
+                                   int64_t bytes)
 {
-    if (size > BDRV_REQUEST_MAX_BYTES) {
+    if (bytes > BDRV_REQUEST_MAX_BYTES) {
         return -EIO;
     }
 
@@ -885,7 +885,7 @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
         return -ENOMEDIUM;
     }
 
-    if (offset < 0) {
+    if (offset < 0 || bytes < 0) {
         return -EIO;
     }
 
-- 
2.21.0

Re: [PATCH v3 03/17] block/io: use int64_t bytes parameter in bdrv_check_byte_request()

[Alberto Garcia]

On Thu 30 Apr 2020 01:10:19 PM CEST, 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 bdrv_check_byte_request() now.
>
> Patch-correctness audit by Eric Blake:
>
>   This changes an unsigned to signed value on 64-bit machines, and
>   additionally widens the parameter on 32-bit machines.  Existing
>   callers:
>
>   bdrv_co_preadv_part() with 'unsigned int' - no impact
>   bdrv_co_pwritev_part() with 'unsigned int' - no impact
>   bdrv_co_copy_range_internal() with 'uint64_t' -
>       potentially fixes a latent bug on 32-bit machines. Requires a
>       larger audit to see how bdrv_co_copy_range() and friends are
>       used:
>
>   block/block-backend.c:blk_co_copy_range() - passes 'int', thus < 2G
>   block/block-copy.c:block_copy_do_copy() -
>       passes 'int64_t', but only after assert(nbytes < INT_MAX); also
>       it has a BLOCK_COPY_MAX_COPY_RANGE set to 16M that factors into
>       its calculations on how much to do per iteration
>
>   So it looks like at present we are safe.
>
> Series: 64bit-block-status
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

[PATCH v3 04/17] block/io: use int64_t bytes in driver wrappers

[Vladimir Sementsov-Ogievskiy]

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 wrappers parameters which are already 64bit to
signed type.

Patch-correctness audit by Eric Blake:

  bdrv_driver_preadv

    Remains 64 bits, the question is whether any caller could pass in
    something larger than 63 bits.  Callers are:

    bdrv_co_do_copy_on_readv() - passes 'int64_t pnum', but sets pnum
      in a fragmenting loop, MAX_BOUNCE_BUFFER when copy-on-read is
      needed, or max_transfer bounded by BDRV_REQUEST_MAX_BYTES
      otherwise
    bdrv_aligned_preadv() - passes 'unsigned int bytes' further limited
      by fragmenting loop on max_transfer <= INT_MAX

    Input is bounded to < 2G, internal use of 'bytes' is:

    drv->bdrv_co_preadv_part(uint64_t) - safe
    qemu_iovec_init_slice(size_t) - potential truncation on 32-bit
      platform [*]
    drv->bdrv_co_preadv(uint64_t) - safe
    drv->bdrv_aio_preadv(uint64_t) - safe
    drv->bdrv_co_readv(int) after assertion of <2G - safe

  bdrv_driver_pwritev

    Remains 64 bits, the question is whether any caller could pass in
    something larger than 63.  Callers are:

    bdrv_co_do_copy_on_readv() - passes 'int64_t pnum', but set in a
      fragmenting loop bounded by MAX_BOUNCE_BUFFER
    bdrv_co_do_pwrite_zeroes() - passes 'int num'
    bdrv_aligned_pwritev() - passes 'unsigned int bytes' further
      limited by fragmenting loop on max_transfer <= INT_MAX

    Input is bounded to < 2G, internal use of 'bytes' is:

    drv->bdrv_co_pwritev_part(uint64_t) - safe
    qemu_iovec_init_slice(size_t) - potential truncation on 32-bit
      platform [*]
    drv->bdrv_co_pwritev(uint64_t) - safe
    drv->bdrv_aio_pwritev(uint64_t) - safe
    drv->bdrv_co_writev(int) after assertion of <2G - safe

  bdrv_driver_pwritev_compressed

    bdrv_aligned_pwritev() - passes 'unsigned int bytes'

    Input is bounded to < 4G, internal use of 'bytes' is:

    drv->bdrv_co_pwritev_compressed_part(uint64_t) - safe
    drv->bdrv_co_pwritev_compressed(uint64_t) - safe
    qemu_iovec_init_slice(size_t) - potential truncation on 32-bit
      platform [*]

[*]: QEMUIOVector is in-RAM data, so size_t seems a valid type for
it's operation. To avoid truncations we should require max_transfer to
be not greater than SIZE_MAX, limiting RAM-occupying operations and
QEMUIOVector usage. Still, 64bit discard and write_zeroes (which
doesn't use QEMUIOVector) should work even on 32bit machines, not being
limited by max_transfer.

For now, we safe anyway, as all input goes through
bdrv_aligned_pwritev() and bdrv_aligned_preadv(), which are limiting
max_transfer to INT_MAX.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/io.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/io.c b/block/io.c
index 7a7d4e578d..eeba3b828c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -906,7 +906,7 @@ static void bdrv_co_io_em_complete(void *opaque, int ret)
 }
 
 static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
-                                           uint64_t offset, uint64_t bytes,
+                                           int64_t offset, int64_t bytes,
                                            QEMUIOVector *qiov,
                                            size_t qiov_offset, int flags)
 {
@@ -975,7 +975,7 @@ out:
 }
 
 static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
-                                            uint64_t offset, uint64_t bytes,
+                                            int64_t offset, int64_t bytes,
                                             QEMUIOVector *qiov,
                                             size_t qiov_offset, int flags)
 {
@@ -1055,8 +1055,8 @@ emulate_flags:
 }
 
 static int coroutine_fn
-bdrv_driver_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
-                               uint64_t bytes, QEMUIOVector *qiov,
+bdrv_driver_pwritev_compressed(BlockDriverState *bs, int64_t offset,
+                               int64_t bytes, QEMUIOVector *qiov,
                                size_t qiov_offset)
 {
     BlockDriver *drv = bs->drv;
-- 
2.21.0

Re: [PATCH v3 04/17] block/io: use int64_t bytes in driver wrappers

[Alberto Garcia]

On Thu 30 Apr 2020 01:10:20 PM CEST, 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 wrappers parameters which are already 64bit to
> signed type.
>
> Patch-correctness audit by Eric Blake:
>
>   bdrv_driver_preadv
>
>     Remains 64 bits, the question is whether any caller could pass in
>     something larger than 63 bits.  Callers are:
>
>     bdrv_co_do_copy_on_readv() - passes 'int64_t pnum', but sets pnum
>       in a fragmenting loop, MAX_BOUNCE_BUFFER when copy-on-read is
>       needed, or max_transfer bounded by BDRV_REQUEST_MAX_BYTES
>       otherwise
>     bdrv_aligned_preadv() - passes 'unsigned int bytes' further limited
>       by fragmenting loop on max_transfer <= INT_MAX
>
>     Input is bounded to < 2G, internal use of 'bytes' is:
>
>     drv->bdrv_co_preadv_part(uint64_t) - safe
>     qemu_iovec_init_slice(size_t) - potential truncation on 32-bit
>       platform [*]
>     drv->bdrv_co_preadv(uint64_t) - safe
>     drv->bdrv_aio_preadv(uint64_t) - safe
>     drv->bdrv_co_readv(int) after assertion of <2G - safe
>
>   bdrv_driver_pwritev
>
>     Remains 64 bits, the question is whether any caller could pass in
>     something larger than 63.  Callers are:
>
>     bdrv_co_do_copy_on_readv() - passes 'int64_t pnum', but set in a
>       fragmenting loop bounded by MAX_BOUNCE_BUFFER
>     bdrv_co_do_pwrite_zeroes() - passes 'int num'
>     bdrv_aligned_pwritev() - passes 'unsigned int bytes' further
>       limited by fragmenting loop on max_transfer <= INT_MAX
>
>     Input is bounded to < 2G, internal use of 'bytes' is:
>
>     drv->bdrv_co_pwritev_part(uint64_t) - safe
>     qemu_iovec_init_slice(size_t) - potential truncation on 32-bit
>       platform [*]
>     drv->bdrv_co_pwritev(uint64_t) - safe
>     drv->bdrv_aio_pwritev(uint64_t) - safe
>     drv->bdrv_co_writev(int) after assertion of <2G - safe
>
>   bdrv_driver_pwritev_compressed
>
>     bdrv_aligned_pwritev() - passes 'unsigned int bytes'
>
>     Input is bounded to < 4G, internal use of 'bytes' is:
>
>     drv->bdrv_co_pwritev_compressed_part(uint64_t) - safe
>     drv->bdrv_co_pwritev_compressed(uint64_t) - safe
>     qemu_iovec_init_slice(size_t) - potential truncation on 32-bit
>       platform [*]
>
> [*]: QEMUIOVector is in-RAM data, so size_t seems a valid type for
> it's operation. To avoid truncations we should require max_transfer to
> be not greater than SIZE_MAX, limiting RAM-occupying operations and
> QEMUIOVector usage. Still, 64bit discard and write_zeroes (which
> doesn't use QEMUIOVector) should work even on 32bit machines, not being
> limited by max_transfer.
>
> For now, we safe anyway, as all input goes through
> bdrv_aligned_pwritev() and bdrv_aligned_preadv(), which are limiting
> max_transfer to INT_MAX.
>
> Series: 64bit-block-status
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

[PATCH v3 05/17] block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes()

[Vladimir Sementsov-Ogievskiy]

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_do_pwrite_zeroes() now.

Patch-correctness audit by Eric Blake:

  Widens from 32- to 64-bit.  Callers:

    bdrv_co_do_copy_on_readv() - passes 'int64_t pnum' bounded by
      fragmenting loop limited to MAX_BOUNCE_BUFFER
    bdrv_aligned_pwritev() - passes 'unsigned int bytes' - latent bug
      fix for sizes between 2G and 4G, if any

    to see if that bug could be tickled, look at callers of
    bdrv_aligned_pwritev:

    bdrv_co_do_zero_pwritev() - splits 'unsigned int bytes' into
      head|body|tail; head and tail are safe but body could be > 2G
    bdrv_co_pwritev_part() - gates with bdrv_check_byte_request()

    continuing the audit, callers of bdrv_co_do_zero_pwritev:

    bdrv_co_pwritev_part() - gates with bdrv_check_byte_request()

    okay, all callers pass < 2G per our current code in
    bdrv_check_byte_request(), so there is no actual bug.

  Use of 'bytes' within the function:

    compute 'int tail' via % 'int alignment' - safe
    fragmentation loop 'int num' - still fragments with a cap on
      max_transfer

    use of 'num' within the loop
    compute 'int head' via % 'int alignment' - safe
    clamp size by 'int max_write_zeroes' - safe
    drv->bdrv_co_pwrite_zeroes(int) - safe because of clamping
    clamp size by 'int max_transfer' - safe
    qemu_iovec_init_buf(size_t) - safe because of clamping
    bdrv_driver_pwritev(uint64_t) [well, int64_t after 4/17] - safe

    So even with the wider type, we aren't exceeding the contract of
    anything we pass it on to.  Later patches may improve
    drv->bdrv_co_pwrite_zeroes and qemu_iovec_init_buf to be 64-bit
    clean, at which point we would want to revisit this function to use
    64-bit clamping rather than 32-bit clamping, but it does not have
    to happen here.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/io.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/io.c b/block/io.c
index eeba3b828c..b83749cc50 100644
--- a/block/io.c
+++ b/block/io.c
@@ -42,7 +42,7 @@
 
 static void bdrv_parent_cb_resize(BlockDriverState *bs);
 static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
-    int64_t offset, int bytes, BdrvRequestFlags flags);
+    int64_t offset, int64_t bytes, BdrvRequestFlags flags);
 
 static void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore,
                                       bool ignore_bds_parents)
@@ -1575,7 +1575,7 @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
 }
 
 static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
-    int64_t offset, int bytes, BdrvRequestFlags flags)
+    int64_t offset, int64_t bytes, BdrvRequestFlags flags)
 {
     BlockDriver *drv = bs->drv;
     QEMUIOVector qiov;
@@ -1605,7 +1605,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
     assert(max_write_zeroes >= bs->bl.request_alignment);
 
     while (bytes > 0 && !ret) {
-        int num = bytes;
+        int64_t num = bytes;
 
         /* Align request.  Block drivers can expect the "bulk" of the request
          * to be aligned, and that unaligned requests do not cross cluster
-- 
2.21.0

Re: [PATCH v3 05/17] block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes()

[Eric Blake]

On 4/30/20 6:10 AM, 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_do_pwrite_zeroes() now.
> 
> Patch-correctness audit by Eric Blake:
> 

> 
>      use of 'num' within the loop
>      compute 'int head' via % 'int alignment' - safe
>      clamp size by 'int max_write_zeroes' - safe
>      drv->bdrv_co_pwrite_zeroes(int) - safe because of clamping
>      clamp size by 'int max_transfer' - safe
>      qemu_iovec_init_buf(size_t) - safe because of clamping
>      bdrv_driver_pwritev(uint64_t) [well, int64_t after 4/17] - safe

I know you were quoting me, but the [comment] can be dropped (I wrote my 
audit on patches in isolation while reviewing the pre-series state of 
the tree, but when this commit is finally applied, the previous patch 
will already be in place)

> 
>      So even with the wider type, we aren't exceeding the contract of
>      anything we pass it on to.  Later patches may improve
>      drv->bdrv_co_pwrite_zeroes and qemu_iovec_init_buf to be 64-bit
>      clean, at which point we would want to revisit this function to use
>      64-bit clamping rather than 32-bit clamping, but it does not have
>      to happen here.
> 
> Series: 64bit-block-status
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>   block/io.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 

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

Re: [PATCH v3 05/17] block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes()

[Alberto Garcia]

On Thu 30 Apr 2020 01:10:21 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
>     compute 'int tail' via % 'int alignment' - safe

    tail = (offset + bytes) % alignment;

both are int64_t, no chance of overflow here?

Berto

Re: [PATCH v3 05/17] block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes()

[Eric Blake]

On 5/11/20 12:17 PM, Alberto Garcia wrote:
> On Thu 30 Apr 2020 01:10:21 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
>>      compute 'int tail' via % 'int alignment' - safe
> 
>      tail = (offset + bytes) % alignment;
> 
> both are int64_t, no chance of overflow here?

Good question - I know several places check that offset+bytes does not 
overflow, but did not specifically audit if this one does.  Adding an 
assert() in this function may be easier than trying to prove all callers 
pass in safe values.

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

Re: [PATCH v3 05/17] block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes()

[Vladimir Sementsov-Ogievskiy]

11.05.2020 21:34, Eric Blake wrote:
> On 5/11/20 12:17 PM, Alberto Garcia wrote:
>> On Thu 30 Apr 2020 01:10:21 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
>>>      compute 'int tail' via % 'int alignment' - safe
>>
>>      tail = (offset + bytes) % alignment;
>>
>> both are int64_t, no chance of overflow here?
> 
> Good question - I know several places check that offset+bytes does not overflow, but did not specifically audit if this one does.  Adding an assert() in this function may be easier than trying to prove all callers pass in safe values.
> 

Hm, it's preexisting, as int64_t + int may overflow as well. Strange, but I don't see overflow check neither in blk_check_byte_request nor in bdrv_check_byte_request. Only discard, which recently dropped call of bdrv_check_byte_request() has this check.

I can add a patch for overflow check in blk_check_byte_request and bdrv_check_byte_request.. But what about alignment? There may be requests, for which bytes + offset doesn't overflow, but do overflow after aligning up. Refactor bdrv_pad_request() to return an error if we can't pad request due to overflow?

-- 
Best regards,
Vladimir

Re: [PATCH v3 05/17] block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes()

[Eric Blake]

On 6/23/20 5:20 AM, Vladimir Sementsov-Ogievskiy wrote:
> 11.05.2020 21:34, Eric Blake wrote:
>> On 5/11/20 12:17 PM, Alberto Garcia wrote:
>>> On Thu 30 Apr 2020 01:10:21 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
>>>>      compute 'int tail' via % 'int alignment' - safe
>>>
>>>      tail = (offset + bytes) % alignment;
>>>
>>> both are int64_t, no chance of overflow here?
>>
>> Good question - I know several places check that offset+bytes does not 
>> overflow, but did not specifically audit if this one does.  Adding an 
>> assert() in this function may be easier than trying to prove all 
>> callers pass in safe values.
>>
> 
> Hm, it's preexisting, as int64_t + int may overflow as well. Strange, 
> but I don't see overflow check neither in blk_check_byte_request nor in 
> bdrv_check_byte_request. Only discard, which recently dropped call of 
> bdrv_check_byte_request() has this check.

In fact, iotest 197 (see commit 461743390) is an instance of testing for 
a bug where we overflowed INT_MAX due to rounding up to cluster size, 
even with a transaction request smaller than limits.

> 
> I can add a patch for overflow check in blk_check_byte_request and 
> bdrv_check_byte_request.. But what about alignment? There may be 
> requests, for which bytes + offset doesn't overflow, but do overflow 
> after aligning up. Refactor bdrv_pad_request() to return an error if we 
> can't pad request due to overflow?

The only cases where int64_t + int can overflow due to rounding up for 
alignment are when the file size is extremely close to 2^63 bytes 
already.  The easiest fix is to reject opening a file that reports a 
size that would overflow when rounded up to alignment (that is, if size 
 > INT64_MAX - alignment, we should refuse to proceed).  Such images 
will never occur for actual disk images (because that is really a LOT of 
storage), but are possible over things like NBD (in fact, nbdkit has 
intentionally made it easy to provoke boundary testing near 2^63 bytes, 
and is already aware that anything larger than 2^63-512 is problematic 
in qemu).

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

[PATCH v3 06/17] block/io: support int64_t bytes in bdrv_aligned_pwritev()

[Vladimir Sementsov-Ogievskiy]

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_aligned_pwritev() now and convert the dependencies:
bdrv_co_write_req_prepare() and bdrv_co_write_req_finish() to signed
type bytes.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/io.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/block/io.c b/block/io.c
index b83749cc50..8bb4ea6285 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1686,12 +1686,11 @@ fail:
 }
 
 static inline int coroutine_fn
-bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, uint64_t bytes,
+bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes,
                           BdrvTrackedRequest *req, int flags)
 {
     BlockDriverState *bs = child->bs;
     bool waited;
-    int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
 
     if (bs->read_only) {
         return -EPERM;
@@ -1716,8 +1715,10 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, uint64_t bytes,
     }
 
     assert(req->overlap_offset <= offset);
+    assert(offset <= INT64_MAX - bytes);
     assert(offset + bytes <= req->overlap_offset + req->overlap_bytes);
-    assert(end_sector <= bs->total_sectors || child->perm & BLK_PERM_RESIZE);
+    assert(offset + bytes <= bs->total_sectors * BDRV_SECTOR_SIZE ||
+           child->perm & BLK_PERM_RESIZE);
 
     switch (req->type) {
     case BDRV_TRACKED_WRITE:
@@ -1738,7 +1739,7 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, uint64_t bytes,
 }
 
 static inline void coroutine_fn
-bdrv_co_write_req_finish(BdrvChild *child, int64_t offset, uint64_t bytes,
+bdrv_co_write_req_finish(BdrvChild *child, int64_t offset, int64_t bytes,
                          BdrvTrackedRequest *req, int ret)
 {
     int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
@@ -1780,14 +1781,14 @@ bdrv_co_write_req_finish(BdrvChild *child, int64_t offset, uint64_t bytes,
  * after possibly fragmenting it.
  */
 static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
-    BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
+    BdrvTrackedRequest *req, int64_t offset, int64_t bytes,
     int64_t align, QEMUIOVector *qiov, size_t qiov_offset, int flags)
 {
     BlockDriverState *bs = child->bs;
     BlockDriver *drv = bs->drv;
     int ret;
 
-    uint64_t bytes_remaining = bytes;
+    int64_t bytes_remaining = bytes;
     int max_transfer;
 
     if (!drv) {
@@ -1799,6 +1800,8 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
     }
 
     assert(is_power_of_2(align));
+    assert(offset >= 0);
+    assert(bytes >= 0);
     assert((offset & (align - 1)) == 0);
     assert((bytes & (align - 1)) == 0);
     assert(!qiov || qiov_offset + bytes <= qiov->size);
@@ -1899,7 +1902,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child,
     assert(!bytes || (offset & (align - 1)) == 0);
     if (bytes >= align) {
         /* Write the aligned part in the middle. */
-        uint64_t aligned_bytes = bytes & ~(align - 1);
+        int64_t aligned_bytes = bytes & ~(align - 1);
         ret = bdrv_aligned_pwritev(child, req, offset, aligned_bytes, align,
                                    NULL, 0, flags);
         if (ret < 0) {
-- 
2.21.0

Re: [PATCH v3 06/17] block/io: support int64_t bytes in bdrv_aligned_pwritev()

[Eric Blake]

On 4/30/20 6:10 AM, 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_aligned_pwritev() now and convert the dependencies:
> bdrv_co_write_req_prepare() and bdrv_co_write_req_finish() to signed
> type bytes.
> 
> Series: 64bit-block-status
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/io.c | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index b83749cc50..8bb4ea6285 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1686,12 +1686,11 @@ fail:
>   }
>   
>   static inline int coroutine_fn
> -bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, uint64_t bytes,
> +bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes,
>                             BdrvTrackedRequest *req, int flags)

Changes from unsigned to signed.  Audit of callers:

bdrv_aligned_pwritev() - adjusted this patch, safe
bdrv_do_pdiscard() - passes int64_t, safe
bdrv_co_copy_range_internal() - passes int64_t, safe
bdrv_do_truncate() - passes int64_t, safe

Internal usage:

>   {
>       BlockDriverState *bs = child->bs;
>       bool waited;
> -    int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);

Drops an old sector calculation, and replaces it with:

>   
>       if (bs->read_only) {
>           return -EPERM;
> @@ -1716,8 +1715,10 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, uint64_t bytes,
>       }
>   
>       assert(req->overlap_offset <= offset);
> +    assert(offset <= INT64_MAX - bytes);
>       assert(offset + bytes <= req->overlap_offset + req->overlap_bytes);
> -    assert(end_sector <= bs->total_sectors || child->perm & BLK_PERM_RESIZE);
> +    assert(offset + bytes <= bs->total_sectors * BDRV_SECTOR_SIZE ||
> +           child->perm & BLK_PERM_RESIZE);

assertions that things fit within 63 bits.  Safe

[The req->overlap_offset+ req->overlap_bytes calculation used to be 
unsigned, but was changed to be signed earlier in this series]

>   
>       switch (req->type) {
>       case BDRV_TRACKED_WRITE:
> @@ -1738,7 +1739,7 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, uint64_t bytes,
>   }
>   
>   static inline void coroutine_fn
> -bdrv_co_write_req_finish(BdrvChild *child, int64_t offset, uint64_t bytes,
> +bdrv_co_write_req_finish(BdrvChild *child, int64_t offset, int64_t bytes,
>                            BdrvTrackedRequest *req, int ret)
>   {

Similar to the above; same four callers, all pass int64_t.


>       int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);

This computation needs analysis.  Previously, we had:

DIV_ROUND_UP(int64_t + uint64_t, unsigned long long)
which expands to:
(((uint64_t) + (ull) - int) / (ull))
which simplifies to uint64_t.

Now we have:
DIV_ROUND_UP(int64_t + int64_t, ull)
Okay, in spite of our argument changing type, the macro still results in 
a 64-bit unsigned answer.  Either way, that answer fits within 63 bits, 
so it is safe when assigned to int64_t.

Also in this function:
             stat64_max(&bs->wr_highest_offset, offset + bytes);
in include/qemu/stats64.h, takes uint64_t parameter, but we're passing a 
positive 63-bit number - safe
             bdrv_set_dirty(bs, offset, bytes);
in block/dirty-bitmap.c, takes int64_t parameter - safe

> @@ -1780,14 +1781,14 @@ bdrv_co_write_req_finish(BdrvChild *child, int64_t offset, uint64_t bytes,
>    * after possibly fragmenting it.
>    */
>   static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
> -    BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
> +    BdrvTrackedRequest *req, int64_t offset, int64_t bytes,
>       int64_t align, QEMUIOVector *qiov, size_t qiov_offset, int flags)
>   {

changes signature from unsigned 32-bit to signed 64-bit.  callers:

bdrv_co_do_zero_pwritev() - passes int64_t, but that was clamped to 
either pad.buf_len [BdrvRequestPadding uses 'size_t buf_len', but 
initializes it in bdrv_init_padding() to at most 2*align] or align set 
from BlockLimits.request_alignment (naturally uint32_t, but documented 
as 'a power of 2 less than INT_MAX' which is at most 1G), so the old 
code never overflowed, and the new code introduces no change

Perhaps we should separately fix BdrvRequestPadding to use a saner type 
than size_t for continuity between 32- and 64-bit platforms (perhaps 
uint32_t rather than int64_t, since we know our padding is bounded by 
request_alignment), but it doesn't impact this patch

bdrv_do_pwritev_part() - still passes unsigned int at this point in the 
series, safe

Usage within the function:

>       BlockDriverState *bs = child->bs;
>       BlockDriver *drv = bs->drv;
>       int ret;
>   
> -    uint64_t bytes_remaining = bytes;
> +    int64_t bytes_remaining = bytes;

Previously we widened unsigned 32-bit into unsigned 64-bit; now we use 
signed 64-bit unchanged.

>       int max_transfer;
>   
>       if (!drv) {
> @@ -1799,6 +1800,8 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
>       }
>   
>       assert(is_power_of_2(align));
> +    assert(offset >= 0);
> +    assert(bytes >= 0);
>       assert((offset & (align - 1)) == 0);
>       assert((bytes & (align - 1)) == 0);
>       assert(!qiov || qiov_offset + bytes <= qiov->size);

qiov->size is only size_t, while 'qiov_offset + bytes' changed from 
'size_t + unsigned int' to 'size_t + int64_t'.  The resulting type of 
the computation changes for some platforms, but the assertion is proving 
that things still fit (including in 32 bits, when size_t is constrained).

     ret = bdrv_co_write_req_prepare(child, offset, bytes, req, flags);
also touched in this patch, safe

         qemu_iovec_is_zero(qiov, qiov_offset, bytes)) {
Passes an 'int64_t' to a 'size_t' parameter, which is possibly 
narrowing.  Fortunately, the assertions just above prove that by this 
point, we are constrained by qiov->size, which is also size_t. Safe.

         ret = bdrv_co_do_pwrite_zeroes(bs, offset, bytes, flags);
Passes to int64_t, safe

         ret = bdrv_driver_pwritev_compressed(bs, offset, bytes,
Passes to int64_t, safe

         ret = bdrv_driver_pwritev(bs, offset, bytes, qiov, qiov_offset, 
flags);
Passes to int64_t, safe

             ret = bdrv_driver_pwritev(bs, offset + bytes - bytes_remaining,
                                       num, qiov, bytes - bytes_remaining,
Passes int64_t to size_t parameter, but the previous assertion proved we 
did not overflow qiov->size which is size_t. Safe

     bdrv_co_write_req_finish(child, offset, bytes, req, ret);
also touched in this patch, safe

> @@ -1899,7 +1902,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child,
>       assert(!bytes || (offset & (align - 1)) == 0);
>       if (bytes >= align) {
>           /* Write the aligned part in the middle. */
> -        uint64_t aligned_bytes = bytes & ~(align - 1);
> +        int64_t aligned_bytes = bytes & ~(align - 1);
>           ret = bdrv_aligned_pwritev(child, req, offset, aligned_bytes, align,
>                                      NULL, 0, flags);
>           if (ret < 0) {
> 

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

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

Re: [PATCH v3 06/17] block/io: support int64_t bytes in bdrv_aligned_pwritev()

[Alberto Garcia]

On Thu 30 Apr 2020 01:10:22 PM CEST, 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_aligned_pwritev() now and convert the dependencies:
> bdrv_co_write_req_prepare() and bdrv_co_write_req_finish() to signed
> type bytes.
>
> Series: 64bit-block-status
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

[PATCH v3 07/17] block/io: support int64_t bytes in bdrv_co_do_copy_on_readv()

[Vladimir Sementsov-Ogievskiy]

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_do_copy_on_readv() now.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/io.c         | 6 +++---
 block/trace-events | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/io.c b/block/io.c
index 8bb4ea6285..6990d8cabe 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1088,7 +1088,7 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs, int64_t offset,
 }
 
 static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
-        int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov,
         size_t qiov_offset, int flags)
 {
     BlockDriverState *bs = child->bs;
@@ -1103,11 +1103,11 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
     BlockDriver *drv = bs->drv;
     int64_t cluster_offset;
     int64_t cluster_bytes;
-    size_t skip_bytes;
+    int64_t skip_bytes;
     int ret;
     int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
                                     BDRV_REQUEST_MAX_BYTES);
-    unsigned int progress = 0;
+    int64_t progress = 0;
     bool skip_write;
 
     if (!drv) {
diff --git a/block/trace-events b/block/trace-events
index 29dff8881c..179b47bf63 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -14,7 +14,7 @@ blk_root_detach(void *child, void *blk, void *bs) "child %p blk %p bs %p"
 bdrv_co_preadv(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset %"PRId64" nbytes %"PRId64" flags 0x%x"
 bdrv_co_pwritev(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset %"PRId64" nbytes %"PRId64" flags 0x%x"
 bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, int flags) "bs %p offset %"PRId64" count %d flags 0x%x"
-bdrv_co_do_copy_on_readv(void *bs, int64_t offset, unsigned int bytes, int64_t cluster_offset, int64_t cluster_bytes) "bs %p offset %"PRId64" bytes %u cluster_offset %"PRId64" cluster_bytes %"PRId64
+bdrv_co_do_copy_on_readv(void *bs, int64_t offset, int64_t bytes, int64_t cluster_offset, int64_t cluster_bytes) "bs %p offset %" PRId64 " bytes %" PRId64 " cluster_offset %" PRId64 " cluster_bytes %" PRId64
 bdrv_co_copy_range_from(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset %"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
 bdrv_co_copy_range_to(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset %"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
 
-- 
2.21.0

Re: [PATCH v3 07/17] block/io: support int64_t bytes in bdrv_co_do_copy_on_readv()

[Eric Blake]

On 4/30/20 6:10 AM, 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_do_copy_on_readv() now.
> 
> Series: 64bit-block-status
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/io.c         | 6 +++---
>   block/trace-events | 2 +-
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 8bb4ea6285..6990d8cabe 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1088,7 +1088,7 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs, int64_t offset,
>   }
>   
>   static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
> -        int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
> +        int64_t offset, int64_t bytes, QEMUIOVector *qiov,
>           size_t qiov_offset, int flags)

Widens from 32-bit to 63-bit.  One caller:

bdrv_aligned_preadv() passes unsigned int (for now) - safe

>   {
>       BlockDriverState *bs = child->bs;
> @@ -1103,11 +1103,11 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
>       BlockDriver *drv = bs->drv;
>       int64_t cluster_offset;
>       int64_t cluster_bytes;
> -    size_t skip_bytes;
> +    int64_t skip_bytes;
>       int ret;
>       int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
>                                       BDRV_REQUEST_MAX_BYTES);
> -    unsigned int progress = 0;
> +    int64_t progress = 0;
>       bool skip_write;

Use of 'bytes', 'sskip_bytes', and 'progress' within the function:
     bdrv_round_to_clusters(bs, offset, bytes, &cluster_offset, 
&cluster_bytes);
  - safe, takes int64_t. Pre-patch, cluster_bytes could be 33 bits (a 
misaligned request just under UINT_MAX can expand to > UINT_MAX when 
aligned to clusters), but only if bytes could be larger than our <2G cap 
that we use elsewhere.  But even if we relax that 2G cap in later 
patches, we should be okay even if 'bytes' starts at larger than 32 
bits, because we don't allow images that would overflow INT64_MAX when 
rounded up to cluster boundaries

     skip_bytes = offset - cluster_offset;
  - actually oversized - the difference is never going to be larger than 
a cluster (which is capped at 2M for qcow2, for example), but doesn't 
hurt that it is now a 64-bit value

     trace_bdrv_co_do_copy_on_readv(bs, offset, bytes,
  - safe, tweaked in this patch

                 assert(progress >= bytes);
  - safe: progress never exceeds pnum, and both variables are same type 
pre- and post-patch

             assert(skip_bytes < pnum);
  - safe

                 qemu_iovec_from_buf(qiov, qiov_offset + progress,
                                     bounce_buffer + skip_bytes,
                                     MIN(pnum - skip_bytes, bytes - 
progress));
  - tricky - pre-patch, pnum was int64_t, post-patch, we have three more 
int64_t entities.  Either way, we're passing int64_t to a size_t 
parameter, which narrows on 64-bit.  However, we're safe: this call is 
in a loop where pnum is clamped at MAX_BOUNCE_BUFFER which is less than 
32 bits, and the MIN() here means we never overflow

             ret = bdrv_driver_preadv(bs, offset + progress,
                                      MIN(pnum - skip_bytes, bytes - 
progress),
                                      qiov, qiov_offset + progress, 0);
  - safe - takes int64_t (earlier in this series), and same analysis 
about the MIN() picking something clamped at MAX_BOUNCE_BUFFER

         progress += pnum - skip_bytes;
         skip_bytes = 0;
  - safe

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

>   
>       if (!drv) {
> diff --git a/block/trace-events b/block/trace-events
> index 29dff8881c..179b47bf63 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -14,7 +14,7 @@ blk_root_detach(void *child, void *blk, void *bs) "child %p blk %p bs %p"
>   bdrv_co_preadv(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset %"PRId64" nbytes %"PRId64" flags 0x%x"
>   bdrv_co_pwritev(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset %"PRId64" nbytes %"PRId64" flags 0x%x"
>   bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, int flags) "bs %p offset %"PRId64" count %d flags 0x%x"
> -bdrv_co_do_copy_on_readv(void *bs, int64_t offset, unsigned int bytes, int64_t cluster_offset, int64_t cluster_bytes) "bs %p offset %"PRId64" bytes %u cluster_offset %"PRId64" cluster_bytes %"PRId64
> +bdrv_co_do_copy_on_readv(void *bs, int64_t offset, int64_t bytes, int64_t cluster_offset, int64_t cluster_bytes) "bs %p offset %" PRId64 " bytes %" PRId64 " cluster_offset %" PRId64 " cluster_bytes %" PRId64
>   bdrv_co_copy_range_from(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset %"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
>   bdrv_co_copy_range_to(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset %"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
>   
> 

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

Re: [PATCH v3 07/17] block/io: support int64_t bytes in bdrv_co_do_copy_on_readv()

[Vladimir Sementsov-Ogievskiy]

22.05.2020 01:29, Eric Blake wrote:
> On 4/30/20 6:10 AM, 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_do_copy_on_readv() now.
>>
>> Series: 64bit-block-status
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block/io.c         | 6 +++---
>>   block/trace-events | 2 +-
>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/block/io.c b/block/io.c
>> index 8bb4ea6285..6990d8cabe 100644
>> --- a/block/io.c
>> +++ b/block/io.c
>> @@ -1088,7 +1088,7 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs, int64_t offset,
>>   }
>>   static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
>> -        int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
>> +        int64_t offset, int64_t bytes, QEMUIOVector *qiov,
>>           size_t qiov_offset, int flags)
> 
> Widens from 32-bit to 63-bit.  One caller:
> 
> bdrv_aligned_preadv() passes unsigned int (for now) - safe
> 
>>   {
>>       BlockDriverState *bs = child->bs;
>> @@ -1103,11 +1103,11 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
>>       BlockDriver *drv = bs->drv;
>>       int64_t cluster_offset;
>>       int64_t cluster_bytes;
>> -    size_t skip_bytes;
>> +    int64_t skip_bytes;
>>       int ret;
>>       int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
>>                                       BDRV_REQUEST_MAX_BYTES);
>> -    unsigned int progress = 0;
>> +    int64_t progress = 0;
>>       bool skip_write;
> 
> Use of 'bytes', 'sskip_bytes', and 'progress' within the function:
>      bdrv_round_to_clusters(bs, offset, bytes, &cluster_offset, &cluster_bytes);
>   - safe, takes int64_t. Pre-patch, cluster_bytes could be 33 bits (a misaligned request just under UINT_MAX can expand to > UINT_MAX when aligned to clusters), but only if bytes could be larger than our <2G cap that we use elsewhere.  But even if we relax that 2G cap in later patches, we should be okay even if 'bytes' starts at larger than 32 bits, because we don't allow images that would overflow INT64_MAX when rounded up to cluster boundaries
> 
>      skip_bytes = offset - cluster_offset;
>   - actually oversized - the difference is never going to be larger than a cluster (which is capped at 2M for qcow2, for example), but doesn't hurt that it is now a 64-bit value
> 
>      trace_bdrv_co_do_copy_on_readv(bs, offset, bytes,
>   - safe, tweaked in this patch
> 
>                  assert(progress >= bytes);
>   - safe: progress never exceeds pnum, and both variables are same type pre- and post-patch
> 
>              assert(skip_bytes < pnum);
>   - safe
> 
>                  qemu_iovec_from_buf(qiov, qiov_offset + progress,
>                                      bounce_buffer + skip_bytes,
>                                      MIN(pnum - skip_bytes, bytes - progress));
>   - tricky - pre-patch, pnum was int64_t, post-patch, we have three more int64_t entities.  Either way, we're passing int64_t to a size_t parameter, which narrows on 64-bit.  However, we're safe: this call is in a loop where pnum is clamped at MAX_BOUNCE_BUFFER which is less than 32 bits, and the MIN() here means we never overflow
> 
>              ret = bdrv_driver_preadv(bs, offset + progress,
>                                       MIN(pnum - skip_bytes, bytes - progress),
>                                       qiov, qiov_offset + progress, 0);
>   - safe - takes int64_t (earlier in this series), and same analysis about the MIN() picking something clamped at MAX_BOUNCE_BUFFER
> 
>          progress += pnum - skip_bytes;
>          skip_bytes = 0;
>   - safe
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

Thanks! Hmm. I'm afraid that I'll rebase this back to master, postponing "[PATCH v2 0/9] block/io: safer inc/dec in_flight sections", as I doubt that it is needed..

-- 
Best regards,
Vladimir

[PATCH v3 08/17] block/io: support int64_t bytes in bdrv_aligned_preadv()

[Vladimir Sementsov-Ogievskiy]

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_aligned_preadv() now.

Make byte variable in bdrv_padding_rmw_read() int64_t, as it defined
only to be passed to bdrv_aligned_preadv().

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/io.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/io.c b/block/io.c
index 6990d8cabe..d336e4e691 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1250,16 +1250,17 @@ err:
  * reads; any other features must be implemented by the caller.
  */
 static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
-    BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
+    BdrvTrackedRequest *req, int64_t offset, int64_t bytes,
     int64_t align, QEMUIOVector *qiov, size_t qiov_offset, int flags)
 {
     BlockDriverState *bs = child->bs;
     int64_t total_bytes, max_bytes;
     int ret = 0;
-    uint64_t bytes_remaining = bytes;
+    int64_t bytes_remaining = bytes;
     int max_transfer;
 
     assert(is_power_of_2(align));
+    assert(offset >= 0 && bytes >= 0);
     assert((offset & (align - 1)) == 0);
     assert((bytes & (align - 1)) == 0);
     assert((bs->open_flags & BDRV_O_NO_IO) == 0);
@@ -1315,7 +1316,7 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
     }
 
     while (bytes_remaining) {
-        int num;
+        int64_t num;
 
         if (max_bytes) {
             num = MIN(bytes_remaining, MIN(max_bytes, max_transfer));
@@ -1416,7 +1417,7 @@ static int bdrv_padding_rmw_read(BdrvChild *child,
     assert(req->serialising && pad->buf);
 
     if (pad->head || pad->merge_reads) {
-        uint64_t bytes = pad->merge_reads ? pad->buf_len : align;
+        int64_t bytes = pad->merge_reads ? pad->buf_len : align;
 
         qemu_iovec_init_buf(&local_qiov, pad->buf, bytes);
 
-- 
2.21.0

Re: [PATCH v3 08/17] block/io: support int64_t bytes in bdrv_aligned_preadv()

[Eric Blake]

On 4/30/20 6:10 AM, 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_aligned_preadv() now.
> 
> Make byte variable in bdrv_padding_rmw_read() int64_t, as it defined
> only to be passed to bdrv_aligned_preadv().
> 
> Series: 64bit-block-status
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/io.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 6990d8cabe..d336e4e691 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1250,16 +1250,17 @@ err:
>    * reads; any other features must be implemented by the caller.
>    */
>   static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
> -    BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
> +    BdrvTrackedRequest *req, int64_t offset, int64_t bytes,
>       int64_t align, QEMUIOVector *qiov, size_t qiov_offset, int flags)
>   {

Callers:

bdrv_padding_rmw_read() - passes int64_t (uint64_t before this patch), 
which in turn is bounded by request_alignment (still 32-bit) or 
BdrvRequestPadding.buf_len (size_t, but also constrained by creation to 
32-bit) - safe

bdrv_do_preadv_part() - passes unsigned int - safe


>       BlockDriverState *bs = child->bs;
>       int64_t total_bytes, max_bytes;
>       int ret = 0;
> -    uint64_t bytes_remaining = bytes;
> +    int64_t bytes_remaining = bytes;
>       int max_transfer;
>   
>       assert(is_power_of_2(align));
> +    assert(offset >= 0 && bytes >= 0);

Use within the function:

the new assertion added here does not check for whether offset+bytes is 
positive; I would suggest we strengthen it to instead be:
assert(offset >= 0 && (uint64_t) bytes <= INT64_MAX - offset);

         ret = bdrv_is_allocated(bs, offset, bytes, &pnum);
  - takes int64_t, safe

         if (!ret || pnum != bytes) {
             ret = bdrv_co_do_copy_on_readv(child, offset, bytes,
                                            qiov, qiov_offset, flags);
  - takes int64_t, safe

     if (bytes <= max_bytes && bytes <= max_transfer) {
         ret = bdrv_driver_preadv(bs, offset, bytes, qiov, qiov_offset, 0);
  - takes int64_t, safe

>       assert((offset & (align - 1)) == 0);
>       assert((bytes & (align - 1)) == 0);
>       assert((bs->open_flags & BDRV_O_NO_IO) == 0);
> @@ -1315,7 +1316,7 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
>       }
>   
>       while (bytes_remaining) {
> -        int num;
> +        int64_t num;
>   
>           if (max_bytes) {
>               num = MIN(bytes_remaining, MIN(max_bytes, max_transfer));

  - safe, bounded by max_transfer which is <= INT_MAX earlier in the 
function

             ret = bdrv_driver_preadv(bs, offset + bytes - bytes_remaining,
                                      num, qiov, bytes - 
bytes_remaining, 0);
  - takes int64_t, and num is capped by max_transfer - safe

             ret = qemu_iovec_memset(qiov, bytes - bytes_remaining, 0,
                                     bytes_remaining);
  - takes size_t, risky for 32-bit platforms.  Works because we checked 
that all our callers are still bounded by 32-bits, but we should 
consider adding an assertion that bytes <= SIZE_MAX so that even when 
our callers are updated in later patches, we know that we are still 
fragmenting requests appropriately on 32-bit platforms

> @@ -1416,7 +1417,7 @@ static int bdrv_padding_rmw_read(BdrvChild *child,
>       assert(req->serialising && pad->buf);
>   
>       if (pad->head || pad->merge_reads) {
> -        uint64_t bytes = pad->merge_reads ? pad->buf_len : align;
> +        int64_t bytes = pad->merge_reads ? pad->buf_len : align;
>   
>           qemu_iovec_init_buf(&local_qiov, pad->buf, bytes);
>   
> 

Preferably with the suggested assertions added,
Reviewed-by: Eric Blake <eblake@redhat.com>

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

Re: [PATCH v3 08/17] block/io: support int64_t bytes in bdrv_aligned_preadv()

[Alberto Garcia]

On Fri 22 May 2020 05:14:36 PM CEST, Eric Blake wrote:
>>   static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
>> -    BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
>> +    BdrvTrackedRequest *req, int64_t offset, int64_t bytes,
>>       int64_t align, QEMUIOVector *qiov, size_t qiov_offset, int flags)
 [...]
>>       BlockDriverState *bs = child->bs;
>>       int64_t total_bytes, max_bytes;
>>       int ret = 0;
>> -    uint64_t bytes_remaining = bytes;
>> +    int64_t bytes_remaining = bytes;
>>       int max_transfer;
>>   
>>       assert(is_power_of_2(align));
>> +    assert(offset >= 0 && bytes >= 0);
>
> Use within the function:
>
> the new assertion added here does not check for whether offset+bytes is 
> positive; I would suggest we strengthen it to instead be:
> assert(offset >= 0 && (uint64_t) bytes <= INT64_MAX - offset);

But here you would be making 'bytes' unsigned without asserting that
it's not negative.

Berto

Re: [PATCH v3 08/17] block/io: support int64_t bytes in bdrv_aligned_preadv()

[Eric Blake]

On 6/18/20 9:35 AM, Alberto Garcia wrote:
> On Fri 22 May 2020 05:14:36 PM CEST, Eric Blake wrote:
>>>    static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
>>> -    BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
>>> +    BdrvTrackedRequest *req, int64_t offset, int64_t bytes,
>>>        int64_t align, QEMUIOVector *qiov, size_t qiov_offset, int flags)
>   [...]
>>>        BlockDriverState *bs = child->bs;
>>>        int64_t total_bytes, max_bytes;
>>>        int ret = 0;
>>> -    uint64_t bytes_remaining = bytes;
>>> +    int64_t bytes_remaining = bytes;
>>>        int max_transfer;
>>>    
>>>        assert(is_power_of_2(align));
>>> +    assert(offset >= 0 && bytes >= 0);
>>
>> Use within the function:
>>
>> the new assertion added here does not check for whether offset+bytes is
>> positive; I would suggest we strengthen it to instead be:
>> assert(offset >= 0 && (uint64_t) bytes <= INT64_MAX - offset);
> 
> But here you would be making 'bytes' unsigned without asserting that
> it's not negative.

'offset >= 0' proves that offset is nonnegative.  'INT64_MAX - offset' 
is still a signed integer, and is also necessarily non-negative (the 
maximum positive integer minus any other positive integer cannot go 
negative).  We then coerce that into an unsigned comparison, which means 
if bytes was negative, the coercion will result in something larger than 
(unsigned)INT64_MAX, and the overall assert() will fail.  Thus, as 
written, the assertion works just fine at proving both bytes and offset 
were non-negative, and that bytes+offset did not overflow a signed integer.

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

[PATCH v3 09/17] block/io: support int64_t bytes in bdrv_co_p{read, write}v_part()

[Vladimir Sementsov-Ogievskiy]

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.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block_int.h |  4 ++--
 block/io.c                | 16 ++++++++--------
 block/trace-events        |  4 ++--
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index c8daba608b..3c2a1d741a 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -975,13 +975,13 @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child,
     int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
     BdrvRequestFlags flags);
 int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
-    int64_t offset, unsigned int bytes,
+    int64_t offset, int64_t bytes,
     QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
 int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
     int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
     BdrvRequestFlags flags);
 int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
-    int64_t offset, unsigned int bytes,
+    int64_t offset, int64_t bytes,
     QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
 
 static inline int coroutine_fn bdrv_co_pread(BdrvChild *child,
diff --git a/block/io.c b/block/io.c
index d336e4e691..d7fd429345 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1488,7 +1488,7 @@ static void bdrv_padding_destroy(BdrvRequestPadding *pad)
  */
 static bool bdrv_pad_request(BlockDriverState *bs,
                              QEMUIOVector **qiov, size_t *qiov_offset,
-                             int64_t *offset, unsigned int *bytes,
+                             int64_t *offset, int64_t *bytes,
                              BdrvRequestPadding *pad)
 {
     if (!bdrv_init_padding(bs, *offset, *bytes, pad)) {
@@ -1515,7 +1515,7 @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child,
 
 /* To be called between exactly one pair of bdrv_inc/dec_in_flight() */
 static int coroutine_fn bdrv_do_preadv_part(BdrvChild *child,
-    int64_t offset, unsigned int bytes,
+    int64_t offset, int64_t bytes,
     QEMUIOVector *qiov, size_t qiov_offset,
     BdrvRequestFlags flags)
 {
@@ -1524,7 +1524,7 @@ static int coroutine_fn bdrv_do_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);
 
     ret = bdrv_check_byte_request(bs, offset, bytes);
     if (ret < 0) {
@@ -1562,7 +1562,7 @@ static int coroutine_fn bdrv_do_preadv_part(BdrvChild *child,
 }
 
 int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
-    int64_t offset, unsigned int bytes,
+    int64_t offset, int64_t bytes,
     QEMUIOVector *qiov, size_t qiov_offset,
     BdrvRequestFlags flags)
 {
@@ -1866,7 +1866,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
 
 static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child,
                                                 int64_t offset,
-                                                unsigned int bytes,
+                                                int64_t bytes,
                                                 BdrvRequestFlags flags,
                                                 BdrvTrackedRequest *req)
 {
@@ -1941,7 +1941,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
 
 /* To be called between exactly one pair of bdrv_inc/dec_in_flight() */
 static int coroutine_fn bdrv_do_pwritev_part(BdrvChild *child,
-    int64_t offset, unsigned int bytes, QEMUIOVector *qiov, size_t qiov_offset,
+    int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset,
     BdrvRequestFlags flags)
 {
     BlockDriverState *bs = child->bs;
@@ -1950,7 +1950,7 @@ static int coroutine_fn bdrv_do_pwritev_part(BdrvChild *child,
     BdrvRequestPadding pad;
     int ret;
 
-    trace_bdrv_co_pwritev(child->bs, offset, bytes, flags);
+    trace_bdrv_co_pwritev_part(child->bs, offset, bytes, flags);
 
     if (!bs->drv) {
         return -ENOMEDIUM;
@@ -2009,7 +2009,7 @@ out:
 }
 
 int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
-    int64_t offset, unsigned int bytes, QEMUIOVector *qiov, size_t qiov_offset,
+    int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset,
     BdrvRequestFlags flags)
 {
     int ret;
diff --git a/block/trace-events b/block/trace-events
index 179b47bf63..dd367a9b19 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -11,8 +11,8 @@ 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"
 
 # io.c
-bdrv_co_preadv(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset %"PRId64" nbytes %"PRId64" flags 0x%x"
-bdrv_co_pwritev(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset %"PRId64" nbytes %"PRId64" flags 0x%x"
+bdrv_co_preadv_part(void *bs, int64_t offset, int64_t bytes, int flags) "bs %p offset %" PRId64 " bytes %" PRId64 " flags 0x%x"
+bdrv_co_pwritev_part(void *bs, int64_t offset, int64_t bytes, int flags) "bs %p offset %" PRId64 " bytes %" PRId64 " flags 0x%x"
 bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, int flags) "bs %p offset %"PRId64" count %d flags 0x%x"
 bdrv_co_do_copy_on_readv(void *bs, int64_t offset, int64_t bytes, int64_t cluster_offset, int64_t cluster_bytes) "bs %p offset %" PRId64 " bytes %" PRId64 " cluster_offset %" PRId64 " cluster_bytes %" PRId64
 bdrv_co_copy_range_from(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset %"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
-- 
2.21.0

Re: [PATCH v3 09/17] block/io: support int64_t bytes in bdrv_co_p{read,write}v_part()

[Eric Blake]

On 4/30/20 6:10 AM, 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.
> 
> Series: 64bit-block-status
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   include/block/block_int.h |  4 ++--
>   block/io.c                | 16 ++++++++--------
>   block/trace-events        |  4 ++--
>   3 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index c8daba608b..3c2a1d741a 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -975,13 +975,13 @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child,
>       int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
>       BdrvRequestFlags flags);
>   int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
> -    int64_t offset, unsigned int bytes,
> +    int64_t offset, int64_t bytes,
>       QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
>   int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
>       int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
>       BdrvRequestFlags flags);
>   int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
> -    int64_t offset, unsigned int bytes,
> +    int64_t offset, int64_t bytes,
>       QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);

Callers for these two functions:

block-backend.c:blk_do_pwritev_part() - currently passes unsigned int

filter-compress.c:compress_co_preadv_part() - passes uint64_t from 
.bdrv_co_preadv_part, which in turn is called from:
  - io.c:bdrv_driver_preadv() - callers analyzed earlier this series, 
where we know we are currently capped at <2G
  - qcow2-cluster.c:do_perform_cow_read() - passes size_t qiov->size, 
but we further know qcow2 cow is limited to cluster size of 2M
  - qcow2.c:qcow2_load_vmstate() - passes size_t qiov->size, tracing 
whether that ever exceeds 32 bits (on 64-bit platforms) is harder

filter-compress.c:compress_co_pwritev_part() - ditto, but for 
.bdrv_co_pwritev_part, which in turn is called from:
  - io.c:bdrv_driver_pwritev() - callers analyzed earlier this series, 
where we know we are currently capped at <2G
  - qcow2.c:qcow2_save_vmstate() - passes size_t qiov->size, tracing 
whether that ever exceeds 32 bits (on 64-bit platforms) is harder

io.c:bdrv_co_preadv() - currently passes unsigned int

io.c:bdrv_co_pwritev() - currently passes unsigned int

qcow2.c:qcow2_co_preadv_task() - passes uint64_t from 
qcow2_co_preadv_part(), which in turn is called from:
  - .bdrv_co_preadv_part - analyzed above

qcow2.c:qcow2_co_pwritev_task() - passes uint64_t from 
qcow2_co_pwritev_part(), which in turn is called from:
  - .bdrv_co_pwritev_part - analyzed above
  - qcow2_co_truncate() - passes uint64_t but it is clamped to 1 
cluster, at most 2M

In summary, it looks like even with our new 64-bit bytes parameter, most 
(all?) callers are still clamped to 32 bits.  But if we later relax 
callers, we want to see how bytes is used within these functions.

>   
>   static inline int coroutine_fn bdrv_co_pread(BdrvChild *child,
> diff --git a/block/io.c b/block/io.c
> index d336e4e691..d7fd429345 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1488,7 +1488,7 @@ static void bdrv_padding_destroy(BdrvRequestPadding *pad)
>    */
>   static bool bdrv_pad_request(BlockDriverState *bs,
>                                QEMUIOVector **qiov, size_t *qiov_offset,
> -                             int64_t *offset, unsigned int *bytes,
> +                             int64_t *offset, int64_t *bytes,
>                                BdrvRequestPadding *pad)
>   {
>       if (!bdrv_init_padding(bs, *offset, *bytes, pad)) {

Callers:
bdrv_do_preadv_part() - adjusted to int64_t in this patch
bdrv_do_pwritev_part() - adjusted to int64_t in this patch

Usage:
     if (!bdrv_init_padding(bs, *offset, *bytes, pad)) {
  - takes int64_t, but now has to be checked for 64-bit safety below

     qemu_iovec_init_extended(&pad->local_qiov, pad->buf, pad->head,
                              *qiov, *qiov_offset, *bytes,
                              pad->buf + pad->buf_len - pad->tail, 
pad->tail);
  - takes size_t, risky on 32-bit platforms if any of our callers ever 
pass in a value larger than 32 bits.  I'd feel much better with an 
assertion that bytes <= SIZE_MAX.

     *bytes += pad->head + pad->tail;
  - corner-case risk of overflow for an image near 63-bit limits (nbdkit 
can generate such an image, but real images do not tickle this); the 
risk can be mitigated if we insist that no images are larger than 
QEMU_ALIGN_DOWN(INT64_MAX, request_alignment), as we would be unable to 
access the unaligned tail bytes of such an image


bdrv_init_padding():
     sum = pad->head + bytes + pad->tail;
  - ouch: this sets 'size_t sum' to what has always been an int64_t 
parameter, but which prior to this patch was initialized by callers 
passing 32-bit values (with this patch, callers now pass in actual 
int64_t).  But even pre-patch, there are values of bytes that fit in 32 
bits but where this sum can overflow on 32-bit platforms, with that 
potential overflow not changed here.  We need a separate patch to fix 
this (sum needs to be int64_t), preferably earlier in the series. (patch 
6/17 also mentioned bdrv_init_padding as needing a fix)


> @@ -1515,7 +1515,7 @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child,
>   
>   /* To be called between exactly one pair of bdrv_inc/dec_in_flight() */
>   static int coroutine_fn bdrv_do_preadv_part(BdrvChild *child,
> -    int64_t offset, unsigned int bytes,
> +    int64_t offset, int64_t bytes,
>       QEMUIOVector *qiov, size_t qiov_offset,
>       BdrvRequestFlags flags)
>   {

Callers:
bdrv_co_preadv_part() - adjusted this patch
bdrv_rw_co_entry() called by bdrv_prwv_co called by:
  - bdrv_pwrite_zeroes() - int bytes
  - bdrv_pwritev() - size_t bytes from qiov
  - bdrv_preadv() - size_t bytes from qiov

This fixes a latent issue where pre-patch callers could pass size_t and 
suffer truncation on a 64-bit platform; but in practice we never tickled 
that bug because of our insistence on <2G read/write.

Usage:

> @@ -1524,7 +1524,7 @@ static int coroutine_fn bdrv_do_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);
>   
>       ret = bdrv_check_byte_request(bs, offset, bytes);

takes int64_t (patch 3/17 in this series), and ensures the transaction 
is <2G for the rest of the function. If we ever relax 
bdrv_check_byte_request, we also have to worry about:

     tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_READ);
  - takes int64_t, audited earlier in this series

     ret = bdrv_aligned_preadv(child, &req, offset, bytes,
                               bs->bl.request_alignment,
                               qiov, qiov_offset, flags);
  - takes int64_t, audited earlier in this series


>       if (ret < 0) {
> @@ -1562,7 +1562,7 @@ static int coroutine_fn bdrv_do_preadv_part(BdrvChild *child,
>   }
>   
>   int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
> -    int64_t offset, unsigned int bytes,
> +    int64_t offset, int64_t bytes,
>       QEMUIOVector *qiov, size_t qiov_offset,
>       BdrvRequestFlags flags)
>   {

Callers analyzed above, usage within the function:

     ret = bdrv_do_preadv_part(child, offset, bytes, qiov, qiov_offset, 
flags);
  - analyzed above

> @@ -1866,7 +1866,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
>   
>   static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child,
>                                                   int64_t offset,
> -                                                unsigned int bytes,
> +                                                int64_t bytes,
>                                                   BdrvRequestFlags flags,
>                                                   BdrvTrackedRequest *req)
>   {

Caller: bdrv_do_pwritev_part(), adjusted in this patch

Usage:
     padding = bdrv_init_padding(bs, offset, bytes, &pad);
  - analyzed above

         ret = bdrv_aligned_pwritev(child, req, offset, aligned_bytes, 
align,
                                    NULL, 0, flags);
  - takes int64_t, audited earlier in the series
         assert(align == pad.tail + bytes);
  - changes from 'uint64_t == size_t + unsigned int' to 'uint64_t == 
size_t + int64_t', but does not change in semantics


> @@ -1941,7 +1941,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
>   
>   /* To be called between exactly one pair of bdrv_inc/dec_in_flight() */
>   static int coroutine_fn bdrv_do_pwritev_part(BdrvChild *child,
> -    int64_t offset, unsigned int bytes, QEMUIOVector *qiov, size_t qiov_offset,
> +    int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset,
>       BdrvRequestFlags flags)
>   {
>       BlockDriverState *bs = child->bs;

Callers:
bdrv_co_pwritev_part() - adjusted this patch
bdrv_do_pwrite_zeroes() - passes 'int'
bdrv_rw_co_entry() analyzed above

Usage:

> @@ -1950,7 +1950,7 @@ static int coroutine_fn bdrv_do_pwritev_part(BdrvChild *child,
>       BdrvRequestPadding pad;
>       int ret;
>   
> -    trace_bdrv_co_pwritev(child->bs, offset, bytes, flags);
> +    trace_bdrv_co_pwritev_part(child->bs, offset, bytes, flags);
>   
>       if (!bs->drv) {
>           return -ENOMEDIUM;

     ret = bdrv_check_byte_request(bs, offset, bytes);
  - analyzed above; if we ever relax it, we must also worry about:

     tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_WRITE);
  - analyzed above

         ret = bdrv_co_do_zero_pwritev(child, offset, bytes, flags, &req);
  - analyzed above

     if (bdrv_pad_request(bs, &qiov, &qiov_offset, &offset, &bytes, &pad)) {
  - analyzed above

     ret = bdrv_aligned_pwritev(child, &req, offset, bytes, align,
                                qiov, qiov_offset, flags);
  - takes int64_t, analyzed earlier this series


> @@ -2009,7 +2009,7 @@ out:
>   }
>   
>   int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
> -    int64_t offset, unsigned int bytes, QEMUIOVector *qiov, size_t qiov_offset,
> +    int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset,
>       BdrvRequestFlags flags)
>   {
>       int ret;

Callers analzyed above, usage:

     ret = bdrv_do_pwritev_part(child, offset, bytes, qiov, qiov_offset, 
flags);
  - analyzed above

> diff --git a/block/trace-events b/block/trace-events
> index 179b47bf63..dd367a9b19 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -11,8 +11,8 @@ 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"
>   
>   # io.c
> -bdrv_co_preadv(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset %"PRId64" nbytes %"PRId64" flags 0x%x"
> -bdrv_co_pwritev(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset %"PRId64" nbytes %"PRId64" flags 0x%x"
> +bdrv_co_preadv_part(void *bs, int64_t offset, int64_t bytes, int flags) "bs %p offset %" PRId64 " bytes %" PRId64 " flags 0x%x"
> +bdrv_co_pwritev_part(void *bs, int64_t offset, int64_t bytes, int flags) "bs %p offset %" PRId64 " bytes %" PRId64 " flags 0x%x"

Interesting that the trace always took 64-bit numbers, but you are 
renaming it to match the fact that they are now called from different 
functions (since commit 1acc3466a2).  I'd feel better if 'flags' were 
kept unsigned int, even though semantically it doesn't matter.

>   bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, int flags) "bs %p offset %"PRId64" count %d flags 0x%x"
>   bdrv_co_do_copy_on_readv(void *bs, int64_t offset, int64_t bytes, int64_t cluster_offset, int64_t cluster_bytes) "bs %p offset %" PRId64 " bytes %" PRId64 " cluster_offset %" PRId64 " cluster_bytes %" PRId64
>   bdrv_co_copy_range_from(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset %"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
> 

I think if you fix bdrv_init_padding in a separate patch, then 
everything else here is safe.
Reviewed-by: Eric Blake <eblake@redhat.com>

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

[PATCH v3 10/17] block/io: support int64_t bytes in read/write wrappers

[Vladimir Sementsov-Ogievskiy]

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).

Now, when bdrv_co_preadv_part() and bdrv_co_pwritev_part() updated,
update all their wrappers.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block.h     | 11 ++++++-----
 include/block/block_int.h |  4 ++--
 block/blkverify.c         |  2 +-
 block/io.c                | 19 ++++++++++---------
 block/trace-events        |  2 +-
 5 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index b05995fe9c..1688e7fa55 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -318,14 +318,15 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
 void bdrv_reopen_commit(BDRVReopenState *reopen_state);
 void bdrv_reopen_abort(BDRVReopenState *reopen_state);
 int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset,
-                       int bytes, BdrvRequestFlags flags);
+                       int64_t bytes, BdrvRequestFlags flags);
 int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags);
-int bdrv_pread(BdrvChild *child, int64_t offset, void *buf, int bytes);
+int bdrv_pread(BdrvChild *child, int64_t offset, void *buf, int64_t bytes);
 int bdrv_preadv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov);
-int bdrv_pwrite(BdrvChild *child, int64_t offset, const void *buf, int bytes);
+int bdrv_pwrite(BdrvChild *child, int64_t offset, const void *buf,
+                int64_t bytes);
 int bdrv_pwritev(BdrvChild *child, int64_t offset, QEMUIOVector *qiov);
 int bdrv_pwrite_sync(BdrvChild *child, int64_t offset,
-                     const void *buf, int count);
+                     const void *buf, int64_t bytes);
 /*
  * Efficiently zero a region of the disk image.  Note that this is a regular
  * I/O request like read or write and should have a reasonable size.  This
@@ -333,7 +334,7 @@ int bdrv_pwrite_sync(BdrvChild *child, int64_t offset,
  * because it may allocate memory for the entire region.
  */
 int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset,
-                                       int bytes, BdrvRequestFlags flags);
+                                       int64_t bytes, BdrvRequestFlags flags);
 BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
     const char *backing_file);
 void bdrv_refresh_filename(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3c2a1d741a..b4b42d0cd6 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -972,13 +972,13 @@ extern BlockDriver bdrv_raw;
 extern BlockDriver bdrv_qcow2;
 
 int coroutine_fn bdrv_co_preadv(BdrvChild *child,
-    int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
+    int64_t offset, int64_t bytes, QEMUIOVector *qiov,
     BdrvRequestFlags flags);
 int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
     int64_t offset, int64_t bytes,
     QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
 int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
-    int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
+    int64_t offset, int64_t bytes, QEMUIOVector *qiov,
     BdrvRequestFlags flags);
 int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
     int64_t offset, int64_t bytes,
diff --git a/block/blkverify.c b/block/blkverify.c
index ba6b1853ae..667e60d832 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -31,7 +31,7 @@ typedef struct BlkverifyRequest {
     uint64_t bytes;
     int flags;
 
-    int (*request_fn)(BdrvChild *, int64_t, unsigned int, QEMUIOVector *,
+    int (*request_fn)(BdrvChild *, int64_t, int64_t, QEMUIOVector *,
                       BdrvRequestFlags);
 
     int ret;                    /* test image result */
diff --git a/block/io.c b/block/io.c
index d7fd429345..7564153dbe 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1507,7 +1507,7 @@ static bool bdrv_pad_request(BlockDriverState *bs,
 }
 
 int coroutine_fn bdrv_co_preadv(BdrvChild *child,
-    int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
+    int64_t offset, int64_t bytes, QEMUIOVector *qiov,
     BdrvRequestFlags flags)
 {
     return bdrv_co_preadv_part(child, offset, bytes, qiov, 0, flags);
@@ -1933,7 +1933,7 @@ out:
  * Handle a write request in coroutine context
  */
 int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
-    int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
+    int64_t offset, int64_t bytes, QEMUIOVector *qiov,
     BdrvRequestFlags flags)
 {
     return bdrv_co_pwritev_part(child, offset, bytes, qiov, 0, flags);
@@ -2023,7 +2023,7 @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
 
 /* To be called between exactly one pair of bdrv_inc/dec_in_flight() */
 static int coroutine_fn
-bdrv_do_pwrite_zeroes(BdrvChild *child, int64_t offset, int bytes,
+bdrv_do_pwrite_zeroes(BdrvChild *child, int64_t offset, int64_t bytes,
                       BdrvRequestFlags flags)
 {
     trace_bdrv_co_pwrite_zeroes(child->bs, offset, bytes, flags);
@@ -2037,7 +2037,7 @@ bdrv_do_pwrite_zeroes(BdrvChild *child, int64_t offset, int bytes,
 }
 
 int coroutine_fn
-bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset, int bytes,
+bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset, int64_t bytes,
                       BdrvRequestFlags flags)
 {
     int ret;
@@ -2109,7 +2109,7 @@ static int bdrv_prwv_co(BdrvChild *child, int64_t offset,
 }
 
 int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset,
-                       int bytes, BdrvRequestFlags flags)
+                       int64_t bytes, BdrvRequestFlags flags)
 {
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, bytes);
 
@@ -2118,7 +2118,7 @@ int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset,
 }
 
 /* See bdrv_pwrite() for the return codes */
-int bdrv_pread(BdrvChild *child, int64_t offset, void *buf, int bytes)
+int bdrv_pread(BdrvChild *child, int64_t offset, void *buf, int64_t bytes)
 {
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
 
@@ -2148,7 +2148,8 @@ int bdrv_pwritev(BdrvChild *child, int64_t offset, QEMUIOVector *qiov)
  * -EINVAL      Invalid offset or number of bytes
  * -EACCES      Trying to write a read-only device
  */
-int bdrv_pwrite(BdrvChild *child, int64_t offset, const void *buf, int bytes)
+int bdrv_pwrite(BdrvChild *child, int64_t offset, const void *buf,
+                int64_t bytes)
 {
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
 
@@ -2166,11 +2167,11 @@ int bdrv_pwrite(BdrvChild *child, int64_t offset, const void *buf, int bytes)
  * Returns 0 on success, -errno in error cases.
  */
 int bdrv_pwrite_sync(BdrvChild *child, int64_t offset,
-                     const void *buf, int count)
+                     const void *buf, int64_t bytes)
 {
     int ret;
 
-    ret = bdrv_pwrite(child, offset, buf, count);
+    ret = bdrv_pwrite(child, offset, buf, bytes);
     if (ret < 0) {
         return ret;
     }
diff --git a/block/trace-events b/block/trace-events
index dd367a9b19..13ad9af0d8 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -13,7 +13,7 @@ blk_root_detach(void *child, void *blk, void *bs) "child %p blk %p bs %p"
 # io.c
 bdrv_co_preadv_part(void *bs, int64_t offset, int64_t bytes, int flags) "bs %p offset %" PRId64 " bytes %" PRId64 " flags 0x%x"
 bdrv_co_pwritev_part(void *bs, int64_t offset, int64_t bytes, int flags) "bs %p offset %" PRId64 " bytes %" PRId64 " flags 0x%x"
-bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, int flags) "bs %p offset %"PRId64" count %d flags 0x%x"
+bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int64_t bytes, int flags) "bs %p offset %" PRId64 " bytes %" PRId64 " flags 0x%x"
 bdrv_co_do_copy_on_readv(void *bs, int64_t offset, int64_t bytes, int64_t cluster_offset, int64_t cluster_bytes) "bs %p offset %" PRId64 " bytes %" PRId64 " cluster_offset %" PRId64 " cluster_bytes %" PRId64
 bdrv_co_copy_range_from(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset %"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
 bdrv_co_copy_range_to(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset %"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
-- 
2.21.0

[PATCH v3 11/17] block/io: use int64_t bytes in copy_range

[Vladimir Sementsov-Ogievskiy]

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 now copy_range parameters which are already 64bit to signed
type.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block.h     |  6 +++---
 include/block/block_int.h | 12 ++++++------
 block/io.c                | 22 +++++++++++-----------
 block/trace-events        |  4 ++--
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 1688e7fa55..4bc90c493e 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -732,8 +732,8 @@ void bdrv_unregister_buf(BlockDriverState *bs, void *host);
  *
  * Returns: 0 if succeeded; negative error code if failed.
  **/
-int coroutine_fn bdrv_co_copy_range(BdrvChild *src, uint64_t src_offset,
-                                    BdrvChild *dst, uint64_t dst_offset,
-                                    uint64_t bytes, BdrvRequestFlags read_flags,
+int coroutine_fn bdrv_co_copy_range(BdrvChild *src, int64_t src_offset,
+                                    BdrvChild *dst, int64_t dst_offset,
+                                    int64_t bytes, BdrvRequestFlags read_flags,
                                     BdrvRequestFlags write_flags);
 #endif
diff --git a/include/block/block_int.h b/include/block/block_int.h
index b4b42d0cd6..28aea2bcfd 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1315,14 +1315,14 @@ void bdrv_dec_in_flight(BlockDriverState *bs);
 
 void blockdev_close_all_bdrv_states(void);
 
-int coroutine_fn bdrv_co_copy_range_from(BdrvChild *src, uint64_t src_offset,
-                                         BdrvChild *dst, uint64_t dst_offset,
-                                         uint64_t bytes,
+int coroutine_fn bdrv_co_copy_range_from(BdrvChild *src, int64_t src_offset,
+                                         BdrvChild *dst, int64_t dst_offset,
+                                         int64_t bytes,
                                          BdrvRequestFlags read_flags,
                                          BdrvRequestFlags write_flags);
-int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src, uint64_t src_offset,
-                                       BdrvChild *dst, uint64_t dst_offset,
-                                       uint64_t bytes,
+int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src, int64_t src_offset,
+                                       BdrvChild *dst, int64_t dst_offset,
+                                       int64_t bytes,
                                        BdrvRequestFlags read_flags,
                                        BdrvRequestFlags write_flags);
 
diff --git a/block/io.c b/block/io.c
index 7564153dbe..db8b3b153d 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3451,8 +3451,8 @@ void bdrv_unregister_buf(BlockDriverState *bs, void *host)
 }
 
 static int coroutine_fn bdrv_co_copy_range_internal(
-        BdrvChild *src, uint64_t src_offset, BdrvChild *dst,
-        uint64_t dst_offset, uint64_t bytes,
+        BdrvChild *src, int64_t src_offset, BdrvChild *dst,
+        int64_t dst_offset, int64_t bytes,
         BdrvRequestFlags read_flags, BdrvRequestFlags write_flags,
         bool recurse_src)
 {
@@ -3530,9 +3530,9 @@ static int coroutine_fn bdrv_co_copy_range_internal(
  *
  * See the comment of bdrv_co_copy_range for the parameter and return value
  * semantics. */
-int coroutine_fn bdrv_co_copy_range_from(BdrvChild *src, uint64_t src_offset,
-                                         BdrvChild *dst, uint64_t dst_offset,
-                                         uint64_t bytes,
+int coroutine_fn bdrv_co_copy_range_from(BdrvChild *src, int64_t src_offset,
+                                         BdrvChild *dst, int64_t dst_offset,
+                                         int64_t bytes,
                                          BdrvRequestFlags read_flags,
                                          BdrvRequestFlags write_flags)
 {
@@ -3546,9 +3546,9 @@ int coroutine_fn bdrv_co_copy_range_from(BdrvChild *src, uint64_t src_offset,
  *
  * See the comment of bdrv_co_copy_range for the parameter and return value
  * semantics. */
-int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src, uint64_t src_offset,
-                                       BdrvChild *dst, uint64_t dst_offset,
-                                       uint64_t bytes,
+int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src, int64_t src_offset,
+                                       BdrvChild *dst, int64_t dst_offset,
+                                       int64_t bytes,
                                        BdrvRequestFlags read_flags,
                                        BdrvRequestFlags write_flags)
 {
@@ -3558,9 +3558,9 @@ int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src, uint64_t src_offset,
                                        bytes, read_flags, write_flags, false);
 }
 
-int coroutine_fn bdrv_co_copy_range(BdrvChild *src, uint64_t src_offset,
-                                    BdrvChild *dst, uint64_t dst_offset,
-                                    uint64_t bytes, BdrvRequestFlags read_flags,
+int coroutine_fn bdrv_co_copy_range(BdrvChild *src, int64_t src_offset,
+                                    BdrvChild *dst, int64_t dst_offset,
+                                    int64_t bytes, BdrvRequestFlags read_flags,
                                     BdrvRequestFlags write_flags)
 {
     return bdrv_co_copy_range_from(src, src_offset,
diff --git a/block/trace-events b/block/trace-events
index 13ad9af0d8..e4d68438af 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -15,8 +15,8 @@ bdrv_co_preadv_part(void *bs, int64_t offset, int64_t bytes, int flags) "bs %p o
 bdrv_co_pwritev_part(void *bs, int64_t offset, int64_t bytes, int flags) "bs %p offset %" PRId64 " bytes %" PRId64 " flags 0x%x"
 bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int64_t bytes, int flags) "bs %p offset %" PRId64 " bytes %" PRId64 " flags 0x%x"
 bdrv_co_do_copy_on_readv(void *bs, int64_t offset, int64_t bytes, int64_t cluster_offset, int64_t cluster_bytes) "bs %p offset %" PRId64 " bytes %" PRId64 " cluster_offset %" PRId64 " cluster_bytes %" PRId64
-bdrv_co_copy_range_from(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset %"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
-bdrv_co_copy_range_to(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset %"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
+bdrv_co_copy_range_from(void *src, int64_t src_offset, void *dst, int64_t dst_offset, int64_t bytes, int read_flags, int write_flags) "src %p offset %" PRId64 " dst %p offset %" PRId64 " bytes %" PRId64 " rw flags 0x%x 0x%x"
+bdrv_co_copy_range_to(void *src, int64_t src_offset, void *dst, int64_t dst_offset, int64_t bytes, int read_flags, int write_flags) "src %p offset %" PRId64 " dst %p offset %" PRId64 " bytes %" PRId64 " rw flags 0x%x 0x%x"
 
 # stream.c
 stream_one_iteration(void *s, int64_t offset, uint64_t bytes, int is_allocated) "s %p offset %" PRId64 " bytes %" PRIu64 " is_allocated %d"
-- 
2.21.0

[PATCH v3 12/17] block/block-backend: convert blk io path to use int64_t parameters

[Vladimir Sementsov-Ogievskiy]

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).

Now bdrv layer is converted, convert blk layer too.

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

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 9bbdbd63d7..0c1eee1778 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -119,14 +119,14 @@ 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,
+                                     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,
@@ -148,13 +148,13 @@ 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,
+                                  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);
-int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int bytes,
+int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int64_t bytes);
+int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int64_t bytes,
                BdrvRequestFlags flags);
 int64_t blk_getlength(BlockBackend *blk);
 void blk_get_geometry(BlockBackend *blk, uint64_t *nb_sectors_ptr);
@@ -167,14 +167,14 @@ 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);
 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 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);
@@ -233,12 +233,12 @@ 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);
+                          int64_t bytes);
 int blk_truncate(BlockBackend *blk, int64_t offset, bool exact,
                  PreallocMode prealloc, 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);
@@ -259,7 +259,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 38ae413826..b3f6edfc70 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1110,11 +1110,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 > INT_MAX) {
         return -EIO;
     }
 
@@ -1122,7 +1122,7 @@ static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
         return -ENOMEDIUM;
     }
 
-    if (offset < 0) {
+    if (offset < 0 || bytes < 0) {
         return -EIO;
     }
 
@@ -1132,7 +1132,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;
         }
     }
@@ -1154,7 +1154,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;
@@ -1185,7 +1185,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;
@@ -1199,7 +1199,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)
 {
@@ -1235,7 +1235,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)
 {
@@ -1249,7 +1249,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);
@@ -1311,7 +1311,7 @@ static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
 }
 
 int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
-                      int bytes, BdrvRequestFlags flags)
+                      int64_t bytes, BdrvRequestFlags flags)
 {
     return blk_prw(blk, offset, NULL, bytes, blk_write_entry,
                    flags | BDRV_REQ_ZERO_WRITE);
@@ -1361,7 +1361,7 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk,
 typedef struct BlkAioEmAIOCB {
     BlockAIOCB common;
     BlkRwCo rwco;
-    int bytes;
+    int64_t bytes;
     bool has_returned;
 } BlkAioEmAIOCB;
 
@@ -1385,7 +1385,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)
@@ -1442,31 +1443,31 @@ 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);
 }
 
-int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int count)
+int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int64_t bytes)
 {
-    int ret = blk_prw(blk, offset, buf, count, blk_read_entry, 0);
+    int ret = blk_prw(blk, offset, buf, bytes, blk_read_entry, 0);
     if (ret < 0) {
         return ret;
     }
-    return count;
+    return bytes;
 }
 
-int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int count,
-               BdrvRequestFlags flags)
+int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf,
+               int64_t bytes, BdrvRequestFlags flags)
 {
-    int ret = blk_prw(blk, offset, (void *) buf, count, blk_write_entry,
+    int ret = blk_prw(blk, offset, (void *) buf, bytes, blk_write_entry,
                       flags);
     if (ret < 0) {
         return ret;
     }
-    return count;
+    return bytes;
 }
 
 int64_t blk_getlength(BlockBackend *blk)
@@ -1567,7 +1568,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;
 
@@ -1591,14 +1592,15 @@ 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,
                         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;
 
@@ -1618,7 +1620,7 @@ static void blk_pdiscard_entry(void *opaque)
     aio_wait_kick();
 }
 
-int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
+int blk_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes)
 {
     return blk_prw(blk, offset, NULL, bytes, blk_pdiscard_entry, 0);
 }
@@ -2123,16 +2125,16 @@ 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);
 }
 
 int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
-                          int count)
+                          int64_t bytes)
 {
-    return blk_prw(blk, offset, (void *) buf, count, blk_write_entry,
+    return blk_prw(blk, offset, (void *) buf, bytes, blk_write_entry,
                    BDRV_REQ_WRITE_COMPRESSED);
 }
 
@@ -2358,7 +2360,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.21.0

Re: [PATCH v3 12/17] block/block-backend: convert blk io path to use int64_t parameters

[Eric Blake]

On 4/30/20 6:10 AM, 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).
> 
> Now bdrv layer is converted, convert blk layer too.

In fact, I just discovered thanks to 
https://bugs.launchpad.net/qemu/+bug/1884831 that NBD is a case of a 
client that can currently pass values larger than 2G into 
blk_co_pdiscard() which in turn appears as a negative value and instant 
EIO failure.  So this is a bug fix visible to NBD clients.

$ gdb --args ./qemu-nbd --trace=nbd_\* -f raw f --port 10810
...
(gdb) b blk_co_pdiscard
(gdb) r
...
$ nbdsh -u nbd://localhost:10810 -c 'h.trim(3*1024*1024*1024,0)'
...
Thread 1 "qemu-nbd" hit Breakpoint 3, blk_co_pdiscard (blk=0x555555832dc0,
     offset=0, bytes=-1073741824)

Looks like I now have even more reason to accelerate my review of the 
remainder of this series, and to take some (if not all) of it through 
the NBD tree.


> +++ b/include/sysemu/block-backend.h
> @@ -119,14 +119,14 @@ 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,
> +                                     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);

pread and pwrite weren't necessarily problems for NBD (since our NBD 
implementation caps things to 32M per packet).

>   
>   static inline int coroutine_fn blk_co_pread(BlockBackend *blk, int64_t offset,
> @@ -148,13 +148,13 @@ 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,
> +                                  int64_t bytes, BdrvRequestFlags flags,
>                                     BlockCompletionFunc *cb, void *opaque);

But this change to writing zeroes,

>   int blk_make_zero(BlockBackend *blk, BdrvRequestFlags flags);
> -int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int bytes);
> -int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int bytes,
> +int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int64_t bytes);
> +int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int64_t bytes,
>                  BdrvRequestFlags flags);
>   int64_t blk_getlength(BlockBackend *blk);
>   void blk_get_geometry(BlockBackend *blk, uint64_t *nb_sectors_ptr);
> @@ -167,14 +167,14 @@ 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);

and this change to discard are definitely both bug fixes for NBD 
clients, especially now that we have a real-world case of a client 
(namely the blkdiscard app triggering ioctl(BLKDISCARD) handling through 
nbd.ko as client) that actually triggers a >2G trim request.

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

[PATCH v3 13/17] block: use int64_t instead of uint64_t in driver read handlers

[Vladimir Sementsov-Ogievskiy]

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.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block_int.h   | 11 ++++++-----
 block/backup-top.c          |  4 ++--
 block/blkdebug.c            |  4 ++--
 block/blklogwrites.c        |  4 ++--
 block/blkreplay.c           |  2 +-
 block/blkverify.c           |  4 ++--
 block/bochs.c               |  4 ++--
 block/cloop.c               |  4 ++--
 block/commit.c              |  2 +-
 block/copy-on-read.c        |  5 +++--
 block/crypto.c              |  4 ++--
 block/curl.c                |  3 ++-
 block/dmg.c                 |  4 ++--
 block/file-posix.c          |  6 +++---
 block/file-win32.c          |  4 ++--
 block/filter-compress.c     |  4 ++--
 block/mirror.c              |  2 +-
 block/nbd.c                 |  5 +++--
 block/nfs.c                 |  6 +++---
 block/null.c                |  9 +++++----
 block/nvme.c                |  5 +++--
 block/qcow.c                |  6 +++---
 block/qcow2.c               |  5 +++--
 block/quorum.c              |  4 ++--
 block/raw-format.c          | 20 ++++++++++----------
 block/rbd.c                 |  5 +++--
 block/throttle.c            |  5 +++--
 block/vdi.c                 |  4 ++--
 block/vmdk.c                |  4 ++--
 block/vpc.c                 |  4 ++--
 block/vvfat.c               |  4 ++--
 block/vxhs.c                |  4 ++--
 tests/test-bdrv-drain.c     | 16 +++++++++-------
 tests/test-block-iothread.c | 19 ++++++++++++++-----
 34 files changed, 108 insertions(+), 88 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 28aea2bcfd..7b5f936269 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -180,8 +180,8 @@ struct BlockDriver {
 
     /* aio */
     BlockAIOCB *(*bdrv_aio_preadv)(BlockDriverState *bs,
-        uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
-        BlockCompletionFunc *cb, void *opaque);
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov,
+        BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
     BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs,
         uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
         BlockCompletionFunc *cb, void *opaque);
@@ -210,10 +210,11 @@ struct BlockDriver {
      * The buffer in @qiov may point directly to guest memory.
      */
     int coroutine_fn (*bdrv_co_preadv)(BlockDriverState *bs,
-        uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov,
+        BdrvRequestFlags flags);
     int coroutine_fn (*bdrv_co_preadv_part)(BlockDriverState *bs,
-        uint64_t offset, uint64_t bytes,
-        QEMUIOVector *qiov, size_t qiov_offset, int flags);
+        int64_t offset, int64_t bytes,
+        QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
     int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int flags);
     /**
diff --git a/block/backup-top.c b/block/backup-top.c
index 3b50c06e2c..289fee5a92 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -42,8 +42,8 @@ typedef struct BDRVBackupTopState {
 } BDRVBackupTopState;
 
 static coroutine_fn int backup_top_co_preadv(
-        BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-        QEMUIOVector *qiov, int flags)
+        BlockDriverState *bs, int64_t offset, int64_t bytes,
+        QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
 }
diff --git a/block/blkdebug.c b/block/blkdebug.c
index af44aa973f..7e02156c63 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -619,8 +619,8 @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
 }
 
 static int coroutine_fn
-blkdebug_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                   QEMUIOVector *qiov, int flags)
+blkdebug_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                   QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     int err;
 
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index 04d8b33607..db60b396bc 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -307,8 +307,8 @@ static void blk_log_writes_refresh_limits(BlockDriverState *bs, Error **errp)
 }
 
 static int coroutine_fn
-blk_log_writes_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                         QEMUIOVector *qiov, int flags)
+blk_log_writes_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                         QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
 }
diff --git a/block/blkreplay.c b/block/blkreplay.c
index c96ac8f4bc..c8dd17bfff 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -73,7 +73,7 @@ static void block_request_create(uint64_t reqid, BlockDriverState *bs,
 }
 
 static int coroutine_fn blkreplay_co_preadv(BlockDriverState *bs,
-    uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
+    int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     uint64_t reqid = blkreplay_next_id();
     int ret = bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
diff --git a/block/blkverify.c b/block/blkverify.c
index 667e60d832..6bc2982a95 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -224,8 +224,8 @@ blkverify_co_prwv(BlockDriverState *bs, BlkverifyRequest *r, uint64_t offset,
 }
 
 static int coroutine_fn
-blkverify_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                    QEMUIOVector *qiov, int flags)
+blkverify_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                    QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BlkverifyRequest r;
     QEMUIOVector raw_qiov;
diff --git a/block/bochs.c b/block/bochs.c
index 32bb83b268..7c8c181705 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -238,8 +238,8 @@ static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
 }
 
 static int coroutine_fn
-bochs_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                QEMUIOVector *qiov, int flags)
+bochs_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BDRVBochsState *s = bs->opaque;
     uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
diff --git a/block/cloop.c b/block/cloop.c
index 4de94876d4..5322f720db 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -245,8 +245,8 @@ static inline int cloop_read_block(BlockDriverState *bs, int block_num)
 }
 
 static int coroutine_fn
-cloop_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                QEMUIOVector *qiov, int flags)
+cloop_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BDRVCloopState *s = bs->opaque;
     uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
diff --git a/block/commit.c b/block/commit.c
index 8e672799af..517f9aa7a0 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -211,7 +211,7 @@ static const BlockJobDriver commit_job_driver = {
 };
 
 static int coroutine_fn bdrv_commit_top_preadv(BlockDriverState *bs,
-    uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
+    int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
 }
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 242d3ff055..c2a0916b66 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -74,8 +74,9 @@ static int64_t cor_getlength(BlockDriverState *bs)
 
 
 static int coroutine_fn cor_co_preadv(BlockDriverState *bs,
-                                      uint64_t offset, uint64_t bytes,
-                                      QEMUIOVector *qiov, int flags)
+                                      int64_t offset, int64_t bytes,
+                                      QEMUIOVector *qiov,
+                                      BdrvRequestFlags flags)
 {
     return bdrv_co_preadv(bs->file, offset, bytes, qiov,
                           flags | BDRV_REQ_COPY_ON_READ);
diff --git a/block/crypto.c b/block/crypto.c
index d577f89659..4df71b25f1 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -335,8 +335,8 @@ static int block_crypto_reopen_prepare(BDRVReopenState *state,
 #define BLOCK_CRYPTO_MAX_IO_SIZE (1024 * 1024)
 
 static coroutine_fn int
-block_crypto_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                       QEMUIOVector *qiov, int flags)
+block_crypto_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                       QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BlockCrypto *crypto = bs->opaque;
     uint64_t cur_bytes; /* number of bytes in current iteration */
diff --git a/block/curl.c b/block/curl.c
index 6e325901dc..39086e83c3 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -922,7 +922,8 @@ out:
 }
 
 static int coroutine_fn curl_co_preadv(BlockDriverState *bs,
-        uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov,
+        BdrvRequestFlags flags)
 {
     CURLAIOCB acb = {
         .co = qemu_coroutine_self(),
diff --git a/block/dmg.c b/block/dmg.c
index 4a045f2b3e..0e3ac05371 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -689,8 +689,8 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
 }
 
 static int coroutine_fn
-dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-              QEMUIOVector *qiov, int flags)
+dmg_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+              QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BDRVDMGState *s = bs->opaque;
     uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
diff --git a/block/file-posix.c b/block/file-posix.c
index 7e19bbff5f..1026e6040b 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1941,9 +1941,9 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
     return raw_thread_pool_submit(bs, handle_aiocb_rw, &acb);
 }
 
-static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset,
-                                      uint64_t bytes, QEMUIOVector *qiov,
-                                      int flags)
+static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
+                                      int64_t bytes, QEMUIOVector *qiov,
+                                      BdrvRequestFlags flags)
 {
     return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_READ);
 }
diff --git a/block/file-win32.c b/block/file-win32.c
index 15859839a1..8a163837ab 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -415,8 +415,8 @@ fail:
 }
 
 static BlockAIOCB *raw_aio_preadv(BlockDriverState *bs,
-                                  uint64_t offset, uint64_t bytes,
-                                  QEMUIOVector *qiov, int flags,
+                                  int64_t offset, int64_t bytes,
+                                  QEMUIOVector *qiov, BdrvRequestFlags flags,
                                   BlockCompletionFunc *cb, void *opaque)
 {
     BDRVRawState *s = bs->opaque;
diff --git a/block/filter-compress.c b/block/filter-compress.c
index 82c315b298..e61de56b96 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -62,10 +62,10 @@ static int64_t compress_getlength(BlockDriverState *bs)
 
 
 static int coroutine_fn compress_co_preadv_part(BlockDriverState *bs,
-                                                uint64_t offset, uint64_t bytes,
+                                                int64_t offset, int64_t bytes,
                                                 QEMUIOVector *qiov,
                                                 size_t qiov_offset,
-                                                int flags)
+                                                BdrvRequestFlags flags)
 {
     return bdrv_co_preadv_part(bs->file, offset, bytes, qiov, qiov_offset,
                                flags);
diff --git a/block/mirror.c b/block/mirror.c
index c26fd9260d..7cd720ac32 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1365,7 +1365,7 @@ static void coroutine_fn active_write_settle(MirrorOp *op)
 }
 
 static int coroutine_fn bdrv_mirror_top_preadv(BlockDriverState *bs,
-    uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
+    int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
 }
diff --git a/block/nbd.c b/block/nbd.c
index 2160859f64..fedad8843f 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1160,8 +1160,9 @@ static int nbd_co_request(BlockDriverState *bs, NBDRequest *request,
     return ret ? ret : request_ret;
 }
 
-static int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
-                                uint64_t bytes, QEMUIOVector *qiov, int flags)
+static int nbd_client_co_preadv(BlockDriverState *bs, int64_t offset,
+                                int64_t bytes, QEMUIOVector *qiov,
+                                BdrvRequestFlags flags)
 {
     int ret, request_ret;
     Error *local_err = NULL;
diff --git a/block/nfs.c b/block/nfs.c
index cc2413d5ab..647250e4d8 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -263,9 +263,9 @@ nfs_co_generic_cb(int ret, struct nfs_context *nfs, void *data,
                                      nfs_co_generic_bh_cb, task);
 }
 
-static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, uint64_t offset,
-                                      uint64_t bytes, QEMUIOVector *iov,
-                                      int flags)
+static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, int64_t offset,
+                                      int64_t bytes, QEMUIOVector *iov,
+                                      BdrvRequestFlags flags)
 {
     NFSClient *client = bs->opaque;
     NFSRPC task;
diff --git a/block/null.c b/block/null.c
index 15e1d56746..e2a59caf46 100644
--- a/block/null.c
+++ b/block/null.c
@@ -116,8 +116,9 @@ static coroutine_fn int null_co_common(BlockDriverState *bs)
 }
 
 static coroutine_fn int null_co_preadv(BlockDriverState *bs,
-                                       uint64_t offset, uint64_t bytes,
-                                       QEMUIOVector *qiov, int flags)
+                                       int64_t offset, int64_t bytes,
+                                       QEMUIOVector *qiov,
+                                       BdrvRequestFlags flags)
 {
     BDRVNullState *s = bs->opaque;
 
@@ -187,8 +188,8 @@ static inline BlockAIOCB *null_aio_common(BlockDriverState *bs,
 }
 
 static BlockAIOCB *null_aio_preadv(BlockDriverState *bs,
-                                   uint64_t offset, uint64_t bytes,
-                                   QEMUIOVector *qiov, int flags,
+                                   int64_t offset, int64_t bytes,
+                                   QEMUIOVector *qiov, BdrvRequestFlags flags,
                                    BlockCompletionFunc *cb,
                                    void *opaque)
 {
diff --git a/block/nvme.c b/block/nvme.c
index 7b7c0cc5d6..c79e398bff 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1062,8 +1062,9 @@ static int nvme_co_prw(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
 }
 
 static coroutine_fn int nvme_co_preadv(BlockDriverState *bs,
-                                       uint64_t offset, uint64_t bytes,
-                                       QEMUIOVector *qiov, int flags)
+                                       int64_t offset, int64_t bytes,
+                                       QEMUIOVector *qiov,
+                                       BdrvRequestFlags flags)
 {
     return nvme_co_prw(bs, offset, bytes, qiov, false, flags);
 }
diff --git a/block/qcow.c b/block/qcow.c
index 8973e4e565..40fc5b96a4 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -628,9 +628,9 @@ static void qcow_refresh_limits(BlockDriverState *bs, Error **errp)
     bs->bl.request_alignment = BDRV_SECTOR_SIZE;
 }
 
-static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, uint64_t offset,
-                                       uint64_t bytes, QEMUIOVector *qiov,
-                                       int flags)
+static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, int64_t offset,
+                                       int64_t bytes, QEMUIOVector *qiov,
+                                       BdrvRequestFlags flags)
 {
     BDRVQcowState *s = bs->opaque;
     int offset_in_cluster;
diff --git a/block/qcow2.c b/block/qcow2.c
index b524b0c53f..817b0bfd2b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2202,9 +2202,10 @@ static coroutine_fn int qcow2_co_preadv_task_entry(AioTask *task)
 }
 
 static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
-                                             uint64_t offset, uint64_t bytes,
+                                             int64_t offset, int64_t bytes,
                                              QEMUIOVector *qiov,
-                                             size_t qiov_offset, int flags)
+                                             size_t qiov_offset,
+                                             BdrvRequestFlags flags)
 {
     BDRVQcow2State *s = bs->opaque;
     int ret = 0;
diff --git a/block/quorum.c b/block/quorum.c
index 6d7a56bd93..3c65c05d17 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -660,8 +660,8 @@ static int read_fifo_child(QuorumAIOCB *acb)
     return ret;
 }
 
-static int quorum_co_preadv(BlockDriverState *bs, uint64_t offset,
-                            uint64_t bytes, QEMUIOVector *qiov, int flags)
+static int quorum_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                            QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BDRVQuorumState *s = bs->opaque;
     QuorumAIOCB *acb = quorum_aio_get(bs, qiov, offset, bytes, flags);
diff --git a/block/raw-format.c b/block/raw-format.c
index 93b25e1b6b..709b8aecdb 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -171,8 +171,8 @@ static void raw_reopen_abort(BDRVReopenState *state)
 }
 
 /* Check and adjust the offset, against 'offset' and 'size' options. */
-static inline int raw_adjust_offset(BlockDriverState *bs, uint64_t *offset,
-                                    uint64_t bytes, bool is_write)
+static inline int raw_adjust_offset(BlockDriverState *bs, int64_t *offset,
+                                    int64_t bytes, bool is_write)
 {
     BDRVRawState *s = bs->opaque;
 
@@ -191,9 +191,9 @@ static inline int raw_adjust_offset(BlockDriverState *bs, uint64_t *offset,
     return 0;
 }
 
-static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset,
-                                      uint64_t bytes, QEMUIOVector *qiov,
-                                      int flags)
+static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
+                                      int64_t bytes, QEMUIOVector *qiov,
+                                      BdrvRequestFlags flags)
 {
     int ret;
 
@@ -249,7 +249,7 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
         qiov = &local_qiov;
     }
 
-    ret = raw_adjust_offset(bs, &offset, bytes, true);
+    ret = raw_adjust_offset(bs, (int64_t *)&offset, bytes, true);
     if (ret) {
         goto fail;
     }
@@ -284,7 +284,7 @@ static int coroutine_fn raw_co_pwrite_zeroes(BlockDriverState *bs,
 {
     int ret;
 
-    ret = raw_adjust_offset(bs, (uint64_t *)&offset, bytes, true);
+    ret = raw_adjust_offset(bs, (int64_t *)&offset, bytes, true);
     if (ret) {
         return ret;
     }
@@ -296,7 +296,7 @@ static int coroutine_fn raw_co_pdiscard(BlockDriverState *bs,
 {
     int ret;
 
-    ret = raw_adjust_offset(bs, (uint64_t *)&offset, bytes, true);
+    ret = raw_adjust_offset(bs, &offset, bytes, true);
     if (ret) {
         return ret;
     }
@@ -516,7 +516,7 @@ static int coroutine_fn raw_co_copy_range_from(BlockDriverState *bs,
 {
     int ret;
 
-    ret = raw_adjust_offset(bs, &src_offset, bytes, false);
+    ret = raw_adjust_offset(bs, (int64_t *)&src_offset, bytes, false);
     if (ret) {
         return ret;
     }
@@ -535,7 +535,7 @@ static int coroutine_fn raw_co_copy_range_to(BlockDriverState *bs,
 {
     int ret;
 
-    ret = raw_adjust_offset(bs, &dst_offset, bytes, true);
+    ret = raw_adjust_offset(bs, (int64_t *)&dst_offset, bytes, true);
     if (ret) {
         return ret;
     }
diff --git a/block/rbd.c b/block/rbd.c
index e637639a07..b7edd685f6 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1034,8 +1034,9 @@ failed:
 }
 
 static BlockAIOCB *qemu_rbd_aio_preadv(BlockDriverState *bs,
-                                       uint64_t offset, uint64_t bytes,
-                                       QEMUIOVector *qiov, int flags,
+                                       int64_t offset, int64_t bytes,
+                                       QEMUIOVector *qiov,
+                                       BdrvRequestFlags flags,
                                        BlockCompletionFunc *cb,
                                        void *opaque)
 {
diff --git a/block/throttle.c b/block/throttle.c
index 71f4bb0ad1..63d1956074 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -114,8 +114,9 @@ static int64_t throttle_getlength(BlockDriverState *bs)
 }
 
 static int coroutine_fn throttle_co_preadv(BlockDriverState *bs,
-                                           uint64_t offset, uint64_t bytes,
-                                           QEMUIOVector *qiov, int flags)
+                                           int64_t offset, int64_t bytes,
+                                           QEMUIOVector *qiov,
+                                           BdrvRequestFlags flags)
 {
 
     ThrottleGroupMember *tgm = bs->opaque;
diff --git a/block/vdi.c b/block/vdi.c
index e1a11f2aa0..1b5365f34c 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -547,8 +547,8 @@ static int coroutine_fn vdi_co_block_status(BlockDriverState *bs,
 }
 
 static int coroutine_fn
-vdi_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-              QEMUIOVector *qiov, int flags)
+vdi_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+              QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BDRVVdiState *s = bs->opaque;
     QEMUIOVector local_qiov;
diff --git a/block/vmdk.c b/block/vmdk.c
index 218d9c9800..a1a0a77fe9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1868,8 +1868,8 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
 }
 
 static int coroutine_fn
-vmdk_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-               QEMUIOVector *qiov, int flags)
+vmdk_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+               QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BDRVVmdkState *s = bs->opaque;
     int ret;
diff --git a/block/vpc.c b/block/vpc.c
index d8141b52da..45c254c3f7 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -611,8 +611,8 @@ static int vpc_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
 }
 
 static int coroutine_fn
-vpc_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-              QEMUIOVector *qiov, int flags)
+vpc_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+              QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BDRVVPCState *s = bs->opaque;
     int ret;
diff --git a/block/vvfat.c b/block/vvfat.c
index ab800c4887..80d488f556 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1538,8 +1538,8 @@ static int vvfat_read(BlockDriverState *bs, int64_t sector_num,
 }
 
 static int coroutine_fn
-vvfat_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                QEMUIOVector *qiov, int flags)
+vvfat_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     int ret;
     BDRVVVFATState *s = bs->opaque;
diff --git a/block/vxhs.c b/block/vxhs.c
index d79fc97df6..a1e437b2cb 100644
--- a/block/vxhs.c
+++ b/block/vxhs.c
@@ -480,8 +480,8 @@ errout:
 }
 
 static BlockAIOCB *vxhs_aio_preadv(BlockDriverState *bs,
-                                   uint64_t offset, uint64_t bytes,
-                                   QEMUIOVector *qiov, int flags,
+                                   int64_t offset, int64_t bytes,
+                                   QEMUIOVector *qiov, BdrvRequestFlags flags,
                                    BlockCompletionFunc *cb, void *opaque)
 {
     return vxhs_aio_rw(bs, offset, qiov, bytes, cb, opaque, VDISK_AIO_READ);
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
index fa0e6a648b..f384e0eb32 100644
--- a/tests/test-bdrv-drain.c
+++ b/tests/test-bdrv-drain.c
@@ -65,8 +65,9 @@ static void co_reenter_bh(void *opaque)
 }
 
 static int coroutine_fn bdrv_test_co_preadv(BlockDriverState *bs,
-                                            uint64_t offset, uint64_t bytes,
-                                            QEMUIOVector *qiov, int flags)
+                                            int64_t offset, int64_t bytes,
+                                            QEMUIOVector *qiov,
+                                            BdrvRequestFlags flags)
 {
     BDRVTestState *s = bs->opaque;
 
@@ -1120,8 +1121,9 @@ static void bdrv_test_top_close(BlockDriverState *bs)
 }
 
 static int coroutine_fn bdrv_test_top_co_preadv(BlockDriverState *bs,
-                                                uint64_t offset, uint64_t bytes,
-                                                QEMUIOVector *qiov, int flags)
+                                                int64_t offset, int64_t bytes,
+                                                QEMUIOVector *qiov,
+                                                BdrvRequestFlags flags)
 {
     BDRVTestTopState *tts = bs->opaque;
     return bdrv_co_preadv(tts->wait_child, offset, bytes, qiov, flags);
@@ -1862,10 +1864,10 @@ static void bdrv_replace_test_close(BlockDriverState *bs)
  *   Set .has_read to true and return success.
  */
 static int coroutine_fn bdrv_replace_test_co_preadv(BlockDriverState *bs,
-                                                    uint64_t offset,
-                                                    uint64_t bytes,
+                                                    int64_t offset,
+                                                    int64_t bytes,
                                                     QEMUIOVector *qiov,
-                                                    int flags)
+                                                    BdrvRequestFlags flags)
 {
     BDRVReplaceTestState *s = bs->opaque;
 
diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c
index 0c861809f0..9567617c50 100644
--- a/tests/test-block-iothread.c
+++ b/tests/test-block-iothread.c
@@ -31,9 +31,18 @@
 #include "qemu/main-loop.h"
 #include "iothread.h"
 
-static int coroutine_fn bdrv_test_co_prwv(BlockDriverState *bs,
-                                          uint64_t offset, uint64_t bytes,
-                                          QEMUIOVector *qiov, int flags)
+static int coroutine_fn bdrv_test_co_preadv(BlockDriverState *bs,
+                                            int64_t offset, int64_t bytes,
+                                            QEMUIOVector *qiov,
+                                            BdrvRequestFlags flags)
+{
+    return 0;
+}
+
+static int coroutine_fn bdrv_test_co_pwritev(BlockDriverState *bs,
+                                             uint64_t offset, uint64_t bytes,
+                                             QEMUIOVector *qiov,
+                                             BdrvRequestFlags flags)
 {
     return 0;
 }
@@ -65,8 +74,8 @@ static BlockDriver bdrv_test = {
     .format_name            = "test",
     .instance_size          = 1,
 
-    .bdrv_co_preadv         = bdrv_test_co_prwv,
-    .bdrv_co_pwritev        = bdrv_test_co_prwv,
+    .bdrv_co_preadv         = bdrv_test_co_preadv,
+    .bdrv_co_pwritev        = bdrv_test_co_pwritev,
     .bdrv_co_pdiscard       = bdrv_test_co_pdiscard,
     .bdrv_co_truncate       = bdrv_test_co_truncate,
     .bdrv_co_block_status   = bdrv_test_co_block_status,
-- 
2.21.0

[PATCH v3 14/17] block: use int64_t instead of uint64_t in driver write handlers

[Vladimir Sementsov-Ogievskiy]

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 write handlers parameters which are already 64bit to
signed type.

While being here, convert also flags parameter to be BdrvRequestFlags.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block_int.h   | 16 ++++++++--------
 block/backup-top.c          |  6 +++---
 block/blkdebug.c            |  4 ++--
 block/blklogwrites.c        |  4 ++--
 block/blkreplay.c           |  2 +-
 block/blkverify.c           |  4 ++--
 block/copy-on-read.c        |  5 +++--
 block/crypto.c              |  4 ++--
 block/file-posix.c          |  6 +++---
 block/file-win32.c          |  4 ++--
 block/filter-compress.c     |  7 ++++---
 block/io.c                  |  6 ++++--
 block/mirror.c              |  2 +-
 block/nbd.c                 |  5 +++--
 block/nfs.c                 |  6 +++---
 block/null.c                |  9 +++++----
 block/nvme.c                |  4 ++--
 block/qcow.c                | 10 +++++-----
 block/qcow2.c               |  6 +++---
 block/quorum.c              |  5 +++--
 block/raw-format.c          | 10 +++++-----
 block/rbd.c                 |  5 +++--
 block/throttle.c            |  5 +++--
 block/vdi.c                 |  4 ++--
 block/vmdk.c                |  8 ++++----
 block/vpc.c                 |  4 ++--
 block/vvfat.c               |  8 ++++----
 block/vxhs.c                |  4 ++--
 tests/test-block-iothread.c |  2 +-
 block/trace-events          |  2 +-
 30 files changed, 88 insertions(+), 79 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7b5f936269..9e8819bc28 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -183,8 +183,8 @@ struct BlockDriver {
         int64_t offset, int64_t bytes, QEMUIOVector *qiov,
         BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
     BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs,
-        uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
-        BlockCompletionFunc *cb, void *opaque);
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov,
+        BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
     BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
         BlockCompletionFunc *cb, void *opaque);
     BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
@@ -233,10 +233,11 @@ struct BlockDriver {
      * The buffer in @qiov may point directly to guest memory.
      */
     int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs,
-        uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov,
+        BdrvRequestFlags flags);
     int coroutine_fn (*bdrv_co_pwritev_part)(BlockDriverState *bs,
-        uint64_t offset, uint64_t bytes,
-        QEMUIOVector *qiov, size_t qiov_offset, int flags);
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset,
+        BdrvRequestFlags flags);
 
     /*
      * Efficiently zero a region of the disk image.  Typically an image format
@@ -365,10 +366,9 @@ struct BlockDriver {
                                       Error **errp);
 
     int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
-        uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov);
     int coroutine_fn (*bdrv_co_pwritev_compressed_part)(BlockDriverState *bs,
-        uint64_t offset, uint64_t bytes, QEMUIOVector *qiov,
-        size_t qiov_offset);
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset);
 
     int (*bdrv_snapshot_create)(BlockDriverState *bs,
                                 QEMUSnapshotInfo *sn_info);
diff --git a/block/backup-top.c b/block/backup-top.c
index 289fee5a92..519c7ea5ad 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -87,9 +87,9 @@ static int coroutine_fn backup_top_co_pwrite_zeroes(BlockDriverState *bs,
 }
 
 static coroutine_fn int backup_top_co_pwritev(BlockDriverState *bs,
-                                              uint64_t offset,
-                                              uint64_t bytes,
-                                              QEMUIOVector *qiov, int flags)
+                                              int64_t offset, int64_t bytes,
+                                              QEMUIOVector *qiov,
+                                              BdrvRequestFlags flags)
 {
     int ret = backup_top_cbw(bs, offset, bytes, flags);
     if (ret < 0) {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 7e02156c63..e47a686628 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -640,8 +640,8 @@ blkdebug_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
 }
 
 static int coroutine_fn
-blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                    QEMUIOVector *qiov, int flags)
+blkdebug_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                    QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     int err;
 
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index db60b396bc..f9697aaa75 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -466,8 +466,8 @@ blk_log_writes_co_do_file_pdiscard(BlkLogWritesFileReq *fr)
 }
 
 static int coroutine_fn
-blk_log_writes_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                          QEMUIOVector *qiov, int flags)
+blk_log_writes_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                          QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     return blk_log_writes_co_log(bs, offset, bytes, qiov, flags,
                                  blk_log_writes_co_do_file_pwritev, 0, false);
diff --git a/block/blkreplay.c b/block/blkreplay.c
index c8dd17bfff..98b8dff860 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -84,7 +84,7 @@ static int coroutine_fn blkreplay_co_preadv(BlockDriverState *bs,
 }
 
 static int coroutine_fn blkreplay_co_pwritev(BlockDriverState *bs,
-    uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
+    int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     uint64_t reqid = blkreplay_next_id();
     int ret = bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
diff --git a/block/blkverify.c b/block/blkverify.c
index 6bc2982a95..796e100eda 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -253,8 +253,8 @@ blkverify_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
 }
 
 static int coroutine_fn
-blkverify_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                     QEMUIOVector *qiov, int flags)
+blkverify_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                     QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BlkverifyRequest r;
     return blkverify_co_prwv(bs, &r, offset, bytes, qiov, qiov, flags, true);
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index c2a0916b66..174e711393 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -84,8 +84,9 @@ static int coroutine_fn cor_co_preadv(BlockDriverState *bs,
 
 
 static int coroutine_fn cor_co_pwritev(BlockDriverState *bs,
-                                       uint64_t offset, uint64_t bytes,
-                                       QEMUIOVector *qiov, int flags)
+                                       int64_t offset, int64_t bytes,
+                                       QEMUIOVector *qiov,
+                                       BdrvRequestFlags flags)
 {
 
     return bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
diff --git a/block/crypto.c b/block/crypto.c
index 4df71b25f1..c873f42f79 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -398,8 +398,8 @@ block_crypto_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
 
 
 static coroutine_fn int
-block_crypto_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                        QEMUIOVector *qiov, int flags)
+block_crypto_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                        QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BlockCrypto *crypto = bs->opaque;
     uint64_t cur_bytes; /* number of bytes in current iteration */
diff --git a/block/file-posix.c b/block/file-posix.c
index 1026e6040b..2c21f785b4 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1948,9 +1948,9 @@ static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
     return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_READ);
 }
 
-static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
-                                       uint64_t bytes, QEMUIOVector *qiov,
-                                       int flags)
+static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset,
+                                       int64_t bytes, QEMUIOVector *qiov,
+                                       BdrvRequestFlags flags)
 {
     assert(flags == 0);
     return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_WRITE);
diff --git a/block/file-win32.c b/block/file-win32.c
index 8a163837ab..f4d2e7af3a 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -430,8 +430,8 @@ static BlockAIOCB *raw_aio_preadv(BlockDriverState *bs,
 }
 
 static BlockAIOCB *raw_aio_pwritev(BlockDriverState *bs,
-                                   uint64_t offset, uint64_t bytes,
-                                   QEMUIOVector *qiov, int flags,
+                                   int64_t offset, int64_t bytes,
+                                   QEMUIOVector *qiov, BdrvRequestFlags flags,
                                    BlockCompletionFunc *cb, void *opaque)
 {
     BDRVRawState *s = bs->opaque;
diff --git a/block/filter-compress.c b/block/filter-compress.c
index e61de56b96..09c0201c51 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -73,10 +73,11 @@ static int coroutine_fn compress_co_preadv_part(BlockDriverState *bs,
 
 
 static int coroutine_fn compress_co_pwritev_part(BlockDriverState *bs,
-                                                 uint64_t offset,
-                                                 uint64_t bytes,
+                                                 int64_t offset,
+                                                 int64_t bytes,
                                                  QEMUIOVector *qiov,
-                                                 size_t qiov_offset, int flags)
+                                                 size_t qiov_offset,
+                                                 BdrvRequestFlags flags)
 {
     return bdrv_co_pwritev_part(bs->file, offset, bytes, qiov, qiov_offset,
                                 flags | BDRV_REQ_WRITE_COMPRESSED);
diff --git a/block/io.c b/block/io.c
index db8b3b153d..bec5a3143f 100644
--- a/block/io.c
+++ b/block/io.c
@@ -977,7 +977,8 @@ out:
 static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
                                             int64_t offset, int64_t bytes,
                                             QEMUIOVector *qiov,
-                                            size_t qiov_offset, int flags)
+                                            size_t qiov_offset,
+                                            BdrvRequestFlags flags)
 {
     BlockDriver *drv = bs->drv;
     int64_t sector_num;
@@ -1783,7 +1784,8 @@ bdrv_co_write_req_finish(BdrvChild *child, int64_t offset, int64_t bytes,
  */
 static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
     BdrvTrackedRequest *req, int64_t offset, int64_t bytes,
-    int64_t align, QEMUIOVector *qiov, size_t qiov_offset, int flags)
+    int64_t align, QEMUIOVector *qiov, size_t qiov_offset,
+    BdrvRequestFlags flags)
 {
     BlockDriverState *bs = child->bs;
     BlockDriver *drv = bs->drv;
diff --git a/block/mirror.c b/block/mirror.c
index 7cd720ac32..ac3d4bc8c5 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1419,7 +1419,7 @@ out:
 }
 
 static int coroutine_fn bdrv_mirror_top_pwritev(BlockDriverState *bs,
-    uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
+    int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     MirrorBDSOpaque *s = bs->opaque;
     QEMUIOVector bounce_qiov;
diff --git a/block/nbd.c b/block/nbd.c
index fedad8843f..8cd417f018 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1219,8 +1219,9 @@ static int nbd_client_co_preadv(BlockDriverState *bs, int64_t offset,
     return ret ? ret : request_ret;
 }
 
-static int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
-                                 uint64_t bytes, QEMUIOVector *qiov, int flags)
+static int nbd_client_co_pwritev(BlockDriverState *bs, int64_t offset,
+                                 int64_t bytes, QEMUIOVector *qiov,
+                                 BdrvRequestFlags flags)
 {
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
     NBDRequest request = {
diff --git a/block/nfs.c b/block/nfs.c
index 647250e4d8..9f049e8c2e 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -298,9 +298,9 @@ static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, int64_t offset,
     return 0;
 }
 
-static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
-                                       uint64_t bytes, QEMUIOVector *iov,
-                                       int flags)
+static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, int64_t offset,
+                                       int64_t bytes, QEMUIOVector *iov,
+                                       BdrvRequestFlags flags)
 {
     NFSClient *client = bs->opaque;
     NFSRPC task;
diff --git a/block/null.c b/block/null.c
index e2a59caf46..30d9f5c6b7 100644
--- a/block/null.c
+++ b/block/null.c
@@ -130,8 +130,9 @@ static coroutine_fn int null_co_preadv(BlockDriverState *bs,
 }
 
 static coroutine_fn int null_co_pwritev(BlockDriverState *bs,
-                                        uint64_t offset, uint64_t bytes,
-                                        QEMUIOVector *qiov, int flags)
+                                        int64_t offset, int64_t bytes,
+                                        QEMUIOVector *qiov,
+                                        BdrvRequestFlags flags)
 {
     return null_co_common(bs);
 }
@@ -203,8 +204,8 @@ static BlockAIOCB *null_aio_preadv(BlockDriverState *bs,
 }
 
 static BlockAIOCB *null_aio_pwritev(BlockDriverState *bs,
-                                    uint64_t offset, uint64_t bytes,
-                                    QEMUIOVector *qiov, int flags,
+                                    int64_t offset, int64_t bytes,
+                                    QEMUIOVector *qiov, BdrvRequestFlags flags,
                                     BlockCompletionFunc *cb,
                                     void *opaque)
 {
diff --git a/block/nvme.c b/block/nvme.c
index c79e398bff..d91bbe60bf 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1070,8 +1070,8 @@ static coroutine_fn int nvme_co_preadv(BlockDriverState *bs,
 }
 
 static coroutine_fn int nvme_co_pwritev(BlockDriverState *bs,
-                                        uint64_t offset, uint64_t bytes,
-                                        QEMUIOVector *qiov, int flags)
+                                        int64_t offset, int64_t bytes,
+                                        QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     return nvme_co_prw(bs, offset, bytes, qiov, true, flags);
 }
diff --git a/block/qcow.c b/block/qcow.c
index 40fc5b96a4..b7c01415e3 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -725,9 +725,9 @@ static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, int64_t offset,
     return ret;
 }
 
-static coroutine_fn int qcow_co_pwritev(BlockDriverState *bs, uint64_t offset,
-                                        uint64_t bytes, QEMUIOVector *qiov,
-                                        int flags)
+static coroutine_fn int qcow_co_pwritev(BlockDriverState *bs, int64_t offset,
+                                        int64_t bytes, QEMUIOVector *qiov,
+                                        BdrvRequestFlags flags)
 {
     BDRVQcowState *s = bs->opaque;
     int offset_in_cluster;
@@ -1049,8 +1049,8 @@ static int qcow_make_empty(BlockDriverState *bs)
 /* XXX: put compressed sectors first, then all the cluster aligned
    tables to avoid losing bytes in alignment */
 static coroutine_fn int
-qcow_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
-                           uint64_t bytes, QEMUIOVector *qiov)
+qcow_co_pwritev_compressed(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                           QEMUIOVector *qiov)
 {
     BDRVQcowState *s = bs->opaque;
     z_stream strm;
diff --git a/block/qcow2.c b/block/qcow2.c
index 817b0bfd2b..ce1c123cb7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2476,8 +2476,8 @@ static coroutine_fn int qcow2_co_pwritev_task_entry(AioTask *task)
 }
 
 static coroutine_fn int qcow2_co_pwritev_part(
-        BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-        QEMUIOVector *qiov, size_t qiov_offset, int flags)
+        BlockDriverState *bs, int64_t offset, int64_t bytes,
+        QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags)
 {
     BDRVQcow2State *s = bs->opaque;
     int offset_in_cluster;
@@ -4329,7 +4329,7 @@ static coroutine_fn int qcow2_co_pwritev_compressed_task_entry(AioTask *task)
  */
 static coroutine_fn int
 qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
-                                 uint64_t offset, uint64_t bytes,
+                                 int64_t offset, int64_t bytes,
                                  QEMUIOVector *qiov, size_t qiov_offset)
 {
     BDRVQcow2State *s = bs->opaque;
diff --git a/block/quorum.c b/block/quorum.c
index 3c65c05d17..2c515659c6 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -706,8 +706,9 @@ static void write_quorum_entry(void *opaque)
     }
 }
 
-static int quorum_co_pwritev(BlockDriverState *bs, uint64_t offset,
-                             uint64_t bytes, QEMUIOVector *qiov, int flags)
+static int quorum_co_pwritev(BlockDriverState *bs, int64_t offset,
+                             int64_t bytes, QEMUIOVector *qiov,
+                             BdrvRequestFlags flags)
 {
     BDRVQuorumState *s = bs->opaque;
     QuorumAIOCB *acb = quorum_aio_get(bs, qiov, offset, bytes, flags);
diff --git a/block/raw-format.c b/block/raw-format.c
index 709b8aecdb..3a393a31c7 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -206,9 +206,9 @@ static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
     return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
 }
 
-static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
-                                       uint64_t bytes, QEMUIOVector *qiov,
-                                       int flags)
+static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset,
+                                       int64_t bytes, QEMUIOVector *qiov,
+                                       BdrvRequestFlags flags)
 {
     void *buf = NULL;
     BlockDriver *drv;
@@ -249,7 +249,7 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
         qiov = &local_qiov;
     }
 
-    ret = raw_adjust_offset(bs, (int64_t *)&offset, bytes, true);
+    ret = raw_adjust_offset(bs, &offset, bytes, true);
     if (ret) {
         goto fail;
     }
@@ -284,7 +284,7 @@ static int coroutine_fn raw_co_pwrite_zeroes(BlockDriverState *bs,
 {
     int ret;
 
-    ret = raw_adjust_offset(bs, (int64_t *)&offset, bytes, true);
+    ret = raw_adjust_offset(bs, &offset, bytes, true);
     if (ret) {
         return ret;
     }
diff --git a/block/rbd.c b/block/rbd.c
index b7edd685f6..705c8d8ff4 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1045,8 +1045,9 @@ static BlockAIOCB *qemu_rbd_aio_preadv(BlockDriverState *bs,
 }
 
 static BlockAIOCB *qemu_rbd_aio_pwritev(BlockDriverState *bs,
-                                        uint64_t offset, uint64_t bytes,
-                                        QEMUIOVector *qiov, int flags,
+                                        int64_t offset, int64_t bytes,
+                                        QEMUIOVector *qiov,
+                                        BdrvRequestFlags flags,
                                         BlockCompletionFunc *cb,
                                         void *opaque)
 {
diff --git a/block/throttle.c b/block/throttle.c
index 63d1956074..af8cc3b2dd 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -126,8 +126,9 @@ static int coroutine_fn throttle_co_preadv(BlockDriverState *bs,
 }
 
 static int coroutine_fn throttle_co_pwritev(BlockDriverState *bs,
-                                            uint64_t offset, uint64_t bytes,
-                                            QEMUIOVector *qiov, int flags)
+                                            int64_t offset, int64_t bytes,
+                                            QEMUIOVector *qiov,
+                                            BdrvRequestFlags flags)
 {
     ThrottleGroupMember *tgm = bs->opaque;
     throttle_group_co_io_limits_intercept(tgm, bytes, true);
diff --git a/block/vdi.c b/block/vdi.c
index 1b5365f34c..8b00f29544 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -603,8 +603,8 @@ vdi_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
 }
 
 static int coroutine_fn
-vdi_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-               QEMUIOVector *qiov, int flags)
+vdi_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+               QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BDRVVdiState *s = bs->opaque;
     QEMUIOVector local_qiov;
diff --git a/block/vmdk.c b/block/vmdk.c
index a1a0a77fe9..61b409ebc7 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2048,8 +2048,8 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
 }
 
 static int coroutine_fn
-vmdk_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                QEMUIOVector *qiov, int flags)
+vmdk_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     int ret;
     BDRVVmdkState *s = bs->opaque;
@@ -2060,8 +2060,8 @@ vmdk_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
 }
 
 static int coroutine_fn
-vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
-                           uint64_t bytes, QEMUIOVector *qiov)
+vmdk_co_pwritev_compressed(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                           QEMUIOVector *qiov)
 {
     if (bytes == 0) {
         /* The caller will write bytes 0 to signal EOF.
diff --git a/block/vpc.c b/block/vpc.c
index 45c254c3f7..1babc21462 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -662,8 +662,8 @@ fail:
 }
 
 static int coroutine_fn
-vpc_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-               QEMUIOVector *qiov, int flags)
+vpc_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+               QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BDRVVPCState *s = bs->opaque;
     int64_t image_offset;
diff --git a/block/vvfat.c b/block/vvfat.c
index 80d488f556..81c86c9243 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3073,8 +3073,8 @@ DLOG(checkpoint());
 }
 
 static int coroutine_fn
-vvfat_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                 QEMUIOVector *qiov, int flags)
+vvfat_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                 QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     int ret;
     BDRVVVFATState *s = bs->opaque;
@@ -3111,8 +3111,8 @@ static int coroutine_fn vvfat_co_block_status(BlockDriverState *bs,
 }
 
 static int coroutine_fn
-write_target_commit(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                    QEMUIOVector *qiov, int flags)
+write_target_commit(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                    QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     int ret;
 
diff --git a/block/vxhs.c b/block/vxhs.c
index a1e437b2cb..e9abef1819 100644
--- a/block/vxhs.c
+++ b/block/vxhs.c
@@ -488,8 +488,8 @@ static BlockAIOCB *vxhs_aio_preadv(BlockDriverState *bs,
 }
 
 static BlockAIOCB *vxhs_aio_pwritev(BlockDriverState *bs,
-                                    uint64_t offset, uint64_t bytes,
-                                    QEMUIOVector *qiov, int flags,
+                                    int64_t offset, int64_t bytes,
+                                    QEMUIOVector *qiov, BdrvRequestFlags flags,
                                     BlockCompletionFunc *cb, void *opaque)
 {
     return vxhs_aio_rw(bs, offset, qiov, bytes, cb, opaque, VDISK_AIO_WRITE);
diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c
index 9567617c50..f5eb80d21b 100644
--- a/tests/test-block-iothread.c
+++ b/tests/test-block-iothread.c
@@ -40,7 +40,7 @@ static int coroutine_fn bdrv_test_co_preadv(BlockDriverState *bs,
 }
 
 static int coroutine_fn bdrv_test_co_pwritev(BlockDriverState *bs,
-                                             uint64_t offset, uint64_t bytes,
+                                             int64_t offset, int64_t bytes,
                                              QEMUIOVector *qiov,
                                              BdrvRequestFlags flags)
 {
diff --git a/block/trace-events b/block/trace-events
index e4d68438af..b7fc837015 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -78,7 +78,7 @@ luring_resubmit_short_read(void *s, void *luringcb, int nread) "LuringState %p l
 
 # qcow2.c
 qcow2_add_task(void *co, void *bs, void *pool, const char *action, int cluster_type, uint64_t file_cluster_offset, uint64_t offset, uint64_t bytes, void *qiov, size_t qiov_offset) "co %p bs %p pool %p: %s: cluster_type %d file_cluster_offset %" PRIu64 " offset %" PRIu64 " bytes %" PRIu64 " qiov %p qiov_offset %zu"
-qcow2_writev_start_req(void *co, int64_t offset, int bytes) "co %p offset 0x%" PRIx64 " bytes %d"
+qcow2_writev_start_req(void *co, int64_t offset, int64_t bytes) "co %p offset 0x%" PRIx64 " bytes %" PRId64
 qcow2_writev_done_req(void *co, int ret) "co %p ret %d"
 qcow2_writev_start_part(void *co) "co %p"
 qcow2_writev_done_part(void *co, int cur_bytes) "co %p cur_bytes %d"
-- 
2.21.0

[PATCH v3 15/17] block: use int64_t instead of uint64_t in copy_range driver handlers

[Vladimir Sementsov-Ogievskiy]

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 copy_range handlers parameters which are already
64bit to signed type.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block_int.h | 12 ++++++------
 block/file-posix.c        | 10 +++++-----
 block/iscsi.c             | 12 ++++++------
 block/qcow2.c             | 12 ++++++------
 block/raw-format.c        | 16 ++++++++--------
 5 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 9e8819bc28..3f08baeff0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -259,10 +259,10 @@ struct BlockDriver {
      */
     int coroutine_fn (*bdrv_co_copy_range_from)(BlockDriverState *bs,
                                                 BdrvChild *src,
-                                                uint64_t offset,
+                                                int64_t offset,
                                                 BdrvChild *dst,
-                                                uint64_t dst_offset,
-                                                uint64_t bytes,
+                                                int64_t dst_offset,
+                                                int64_t bytes,
                                                 BdrvRequestFlags read_flags,
                                                 BdrvRequestFlags write_flags);
 
@@ -276,10 +276,10 @@ struct BlockDriver {
      */
     int coroutine_fn (*bdrv_co_copy_range_to)(BlockDriverState *bs,
                                               BdrvChild *src,
-                                              uint64_t src_offset,
+                                              int64_t src_offset,
                                               BdrvChild *dst,
-                                              uint64_t dst_offset,
-                                              uint64_t bytes,
+                                              int64_t dst_offset,
+                                              int64_t bytes,
                                               BdrvRequestFlags read_flags,
                                               BdrvRequestFlags write_flags);
 
diff --git a/block/file-posix.c b/block/file-posix.c
index 2c21f785b4..38c1398494 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3034,8 +3034,8 @@ static void raw_abort_perm_update(BlockDriverState *bs)
 }
 
 static int coroutine_fn raw_co_copy_range_from(
-        BlockDriverState *bs, BdrvChild *src, uint64_t src_offset,
-        BdrvChild *dst, uint64_t dst_offset, uint64_t bytes,
+        BlockDriverState *bs, BdrvChild *src, int64_t src_offset,
+        BdrvChild *dst, int64_t dst_offset, int64_t bytes,
         BdrvRequestFlags read_flags, BdrvRequestFlags write_flags)
 {
     return bdrv_co_copy_range_to(src, src_offset, dst, dst_offset, bytes,
@@ -3044,10 +3044,10 @@ static int coroutine_fn raw_co_copy_range_from(
 
 static int coroutine_fn raw_co_copy_range_to(BlockDriverState *bs,
                                              BdrvChild *src,
-                                             uint64_t src_offset,
+                                             int64_t src_offset,
                                              BdrvChild *dst,
-                                             uint64_t dst_offset,
-                                             uint64_t bytes,
+                                             int64_t dst_offset,
+                                             int64_t bytes,
                                              BdrvRequestFlags read_flags,
                                              BdrvRequestFlags write_flags)
 {
diff --git a/block/iscsi.c b/block/iscsi.c
index 0b4b7210df..861a70c823 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2180,10 +2180,10 @@ static void coroutine_fn iscsi_co_invalidate_cache(BlockDriverState *bs,
 
 static int coroutine_fn iscsi_co_copy_range_from(BlockDriverState *bs,
                                                  BdrvChild *src,
-                                                 uint64_t src_offset,
+                                                 int64_t src_offset,
                                                  BdrvChild *dst,
-                                                 uint64_t dst_offset,
-                                                 uint64_t bytes,
+                                                 int64_t dst_offset,
+                                                 int64_t bytes,
                                                  BdrvRequestFlags read_flags,
                                                  BdrvRequestFlags write_flags)
 {
@@ -2321,10 +2321,10 @@ static void iscsi_xcopy_data(struct iscsi_data *data,
 
 static int coroutine_fn iscsi_co_copy_range_to(BlockDriverState *bs,
                                                BdrvChild *src,
-                                               uint64_t src_offset,
+                                               int64_t src_offset,
                                                BdrvChild *dst,
-                                               uint64_t dst_offset,
-                                               uint64_t bytes,
+                                               int64_t dst_offset,
+                                               int64_t bytes,
                                                BdrvRequestFlags read_flags,
                                                BdrvRequestFlags write_flags)
 {
diff --git a/block/qcow2.c b/block/qcow2.c
index ce1c123cb7..15a6d3c6ef 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3810,9 +3810,9 @@ static coroutine_fn int qcow2_co_pdiscard(BlockDriverState *bs,
 
 static int coroutine_fn
 qcow2_co_copy_range_from(BlockDriverState *bs,
-                         BdrvChild *src, uint64_t src_offset,
-                         BdrvChild *dst, uint64_t dst_offset,
-                         uint64_t bytes, BdrvRequestFlags read_flags,
+                         BdrvChild *src, int64_t src_offset,
+                         BdrvChild *dst, int64_t dst_offset,
+                         int64_t bytes, BdrvRequestFlags read_flags,
                          BdrvRequestFlags write_flags)
 {
     BDRVQcow2State *s = bs->opaque;
@@ -3891,9 +3891,9 @@ out:
 
 static int coroutine_fn
 qcow2_co_copy_range_to(BlockDriverState *bs,
-                       BdrvChild *src, uint64_t src_offset,
-                       BdrvChild *dst, uint64_t dst_offset,
-                       uint64_t bytes, BdrvRequestFlags read_flags,
+                       BdrvChild *src, int64_t src_offset,
+                       BdrvChild *dst, int64_t dst_offset,
+                       int64_t bytes, BdrvRequestFlags read_flags,
                        BdrvRequestFlags write_flags)
 {
     BDRVQcow2State *s = bs->opaque;
diff --git a/block/raw-format.c b/block/raw-format.c
index 3a393a31c7..fdf024705b 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -507,16 +507,16 @@ static int raw_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
 
 static int coroutine_fn raw_co_copy_range_from(BlockDriverState *bs,
                                                BdrvChild *src,
-                                               uint64_t src_offset,
+                                               int64_t src_offset,
                                                BdrvChild *dst,
-                                               uint64_t dst_offset,
-                                               uint64_t bytes,
+                                               int64_t dst_offset,
+                                               int64_t bytes,
                                                BdrvRequestFlags read_flags,
                                                BdrvRequestFlags write_flags)
 {
     int ret;
 
-    ret = raw_adjust_offset(bs, (int64_t *)&src_offset, bytes, false);
+    ret = raw_adjust_offset(bs, &src_offset, bytes, false);
     if (ret) {
         return ret;
     }
@@ -526,16 +526,16 @@ static int coroutine_fn raw_co_copy_range_from(BlockDriverState *bs,
 
 static int coroutine_fn raw_co_copy_range_to(BlockDriverState *bs,
                                              BdrvChild *src,
-                                             uint64_t src_offset,
+                                             int64_t src_offset,
                                              BdrvChild *dst,
-                                             uint64_t dst_offset,
-                                             uint64_t bytes,
+                                             int64_t dst_offset,
+                                             int64_t bytes,
                                              BdrvRequestFlags read_flags,
                                              BdrvRequestFlags write_flags)
 {
     int ret;
 
-    ret = raw_adjust_offset(bs, (int64_t *)&dst_offset, bytes, true);
+    ret = raw_adjust_offset(bs, &dst_offset, bytes, true);
     if (ret) {
         return ret;
     }
-- 
2.21.0

[PATCH v3 16/17] block: use int64_t instead of int in driver write_zeroes handlers

[Vladimir Sementsov-Ogievskiy]

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 write_zeroes handlers bytes parameter to int64_t.

This patch just converts handlers where it is obvious that bytes
parameter is passed further to 64bit interfaces, and add simple
wrappers where it is not obvious.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block_int.h |  2 +-
 block/backup-top.c        |  2 +-
 block/blkdebug.c          |  2 +-
 block/blklogwrites.c      |  4 ++--
 block/blkreplay.c         |  2 +-
 block/copy-on-read.c      |  2 +-
 block/file-posix.c        |  6 +++---
 block/filter-compress.c   |  2 +-
 block/gluster.c           |  8 +++++---
 block/iscsi.c             | 12 ++++++++++--
 block/mirror.c            |  2 +-
 block/nbd.c               |  4 +++-
 block/nvme.c              | 16 ++++++++++++----
 block/qcow2.c             |  9 ++++++++-
 block/qed.c               | 17 +++++++++++++----
 block/raw-format.c        |  2 +-
 block/throttle.c          |  2 +-
 block/vmdk.c              |  2 +-
 18 files changed, 66 insertions(+), 30 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3f08baeff0..c98d591a56 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -246,7 +246,7 @@ struct BlockDriver {
      * will be called instead.
      */
     int coroutine_fn (*bdrv_co_pwrite_zeroes)(BlockDriverState *bs,
-        int64_t offset, int bytes, BdrvRequestFlags flags);
+        int64_t offset, int64_t bytes, BdrvRequestFlags flags);
     int coroutine_fn (*bdrv_co_pdiscard)(BlockDriverState *bs,
         int64_t offset, int bytes);
 
diff --git a/block/backup-top.c b/block/backup-top.c
index 519c7ea5ad..2484293fb0 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -76,7 +76,7 @@ static int coroutine_fn backup_top_co_pdiscard(BlockDriverState *bs,
 }
 
 static int coroutine_fn backup_top_co_pwrite_zeroes(BlockDriverState *bs,
-        int64_t offset, int bytes, BdrvRequestFlags flags)
+        int64_t offset, int64_t bytes, BdrvRequestFlags flags)
 {
     int ret = backup_top_cbw(bs, offset, bytes, flags);
     if (ret < 0) {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index e47a686628..d593d6c85d 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -672,7 +672,7 @@ static int blkdebug_co_flush(BlockDriverState *bs)
 }
 
 static int coroutine_fn blkdebug_co_pwrite_zeroes(BlockDriverState *bs,
-                                                  int64_t offset, int bytes,
+                                                  int64_t offset, int64_t bytes,
                                                   BdrvRequestFlags flags)
 {
     uint32_t align = MAX(bs->bl.request_alignment,
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index f9697aaa75..8ca41d09cd 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -474,8 +474,8 @@ blk_log_writes_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
 }
 
 static int coroutine_fn
-blk_log_writes_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes,
-                                BdrvRequestFlags flags)
+blk_log_writes_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
+                                int64_t bytes, BdrvRequestFlags flags)
 {
     return blk_log_writes_co_log(bs, offset, bytes, NULL, flags,
                                  blk_log_writes_co_do_file_pwrite_zeroes, 0,
diff --git a/block/blkreplay.c b/block/blkreplay.c
index 98b8dff860..186d28cc6a 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -95,7 +95,7 @@ static int coroutine_fn blkreplay_co_pwritev(BlockDriverState *bs,
 }
 
 static int coroutine_fn blkreplay_co_pwrite_zeroes(BlockDriverState *bs,
-    int64_t offset, int bytes, BdrvRequestFlags flags)
+    int64_t offset, int64_t bytes, BdrvRequestFlags flags)
 {
     uint64_t reqid = blkreplay_next_id();
     int ret = bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags);
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 174e711393..d99e07e99f 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -94,7 +94,7 @@ static int coroutine_fn cor_co_pwritev(BlockDriverState *bs,
 
 
 static int coroutine_fn cor_co_pwrite_zeroes(BlockDriverState *bs,
-                                             int64_t offset, int bytes,
+                                             int64_t offset, int64_t bytes,
                                              BdrvRequestFlags flags)
 {
     return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags);
diff --git a/block/file-posix.c b/block/file-posix.c
index 38c1398494..fa9ac5b13e 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2797,7 +2797,7 @@ raw_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
 }
 
 static int coroutine_fn
-raw_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes,
+raw_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes,
                      BdrvRequestFlags flags, bool blkdev)
 {
     BDRVRawState *s = bs->opaque;
@@ -2866,7 +2866,7 @@ raw_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes,
 
 static int coroutine_fn raw_co_pwrite_zeroes(
     BlockDriverState *bs, int64_t offset,
-    int bytes, BdrvRequestFlags flags)
+    int64_t bytes, BdrvRequestFlags flags)
 {
     return raw_do_pwrite_zeroes(bs, offset, bytes, flags, false);
 }
@@ -3489,7 +3489,7 @@ hdev_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
 }
 
 static coroutine_fn int hdev_co_pwrite_zeroes(BlockDriverState *bs,
-    int64_t offset, int bytes, BdrvRequestFlags flags)
+    int64_t offset, int64_t bytes, BdrvRequestFlags flags)
 {
     int rc;
 
diff --git a/block/filter-compress.c b/block/filter-compress.c
index 09c0201c51..7cf47608b5 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -85,7 +85,7 @@ static int coroutine_fn compress_co_pwritev_part(BlockDriverState *bs,
 
 
 static int coroutine_fn compress_co_pwrite_zeroes(BlockDriverState *bs,
-                                                  int64_t offset, int bytes,
+                                                  int64_t offset, int64_t bytes,
                                                   BdrvRequestFlags flags)
 {
     return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags);
diff --git a/block/gluster.c b/block/gluster.c
index 0aa1f2cda4..88130c3d2d 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1017,19 +1017,21 @@ static void qemu_gluster_reopen_abort(BDRVReopenState *state)
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
 static coroutine_fn int qemu_gluster_co_pwrite_zeroes(BlockDriverState *bs,
                                                       int64_t offset,
-                                                      int size,
+                                                      int64_t bytes,
                                                       BdrvRequestFlags flags)
 {
     int ret;
     GlusterAIOCB acb;
     BDRVGlusterState *s = bs->opaque;
 
-    acb.size = size;
+    assert(bytes < INT_MAX);
+
+    acb.size = bytes;
     acb.ret = 0;
     acb.coroutine = qemu_coroutine_self();
     acb.aio_context = bdrv_get_aio_context(bs);
 
-    ret = glfs_zerofill_async(s->fd, offset, size, gluster_finish_aiocb, &acb);
+    ret = glfs_zerofill_async(s->fd, offset, bytes, gluster_finish_aiocb, &acb);
     if (ret < 0) {
         return -errno;
     }
diff --git a/block/iscsi.c b/block/iscsi.c
index 861a70c823..c4183ef12f 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1204,8 +1204,8 @@ out_unlock:
 }
 
 static int
-coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
-                                    int bytes, BdrvRequestFlags flags)
+coroutine_fn iscsi_co_pwrite_zeroes_old(BlockDriverState *bs, int64_t offset,
+                                        int bytes, BdrvRequestFlags flags)
 {
     IscsiLun *iscsilun = bs->opaque;
     struct IscsiTask iTask;
@@ -1308,6 +1308,14 @@ out_unlock:
     return r;
 }
 
+static int
+coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
+                                    int64_t bytes, BdrvRequestFlags flags)
+{
+    assert(bytes < INT_MAX);
+    return iscsi_co_pwrite_zeroes_old(bs, offset, bytes, flags);
+}
+
 static void apply_chap(struct iscsi_context *iscsi, QemuOpts *opts,
                        Error **errp)
 {
diff --git a/block/mirror.c b/block/mirror.c
index ac3d4bc8c5..7f4fdfb470 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1464,7 +1464,7 @@ static int coroutine_fn bdrv_mirror_top_flush(BlockDriverState *bs)
 }
 
 static int coroutine_fn bdrv_mirror_top_pwrite_zeroes(BlockDriverState *bs,
-    int64_t offset, int bytes, BdrvRequestFlags flags)
+    int64_t offset, int64_t bytes, BdrvRequestFlags flags)
 {
     return bdrv_mirror_top_do_write(bs, MIRROR_METHOD_ZERO, offset, bytes, NULL,
                                     flags);
diff --git a/block/nbd.c b/block/nbd.c
index 8cd417f018..0e52b76d72 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1245,7 +1245,7 @@ static int nbd_client_co_pwritev(BlockDriverState *bs, int64_t offset,
 }
 
 static int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
-                                       int bytes, BdrvRequestFlags flags)
+                                       int64_t bytes, BdrvRequestFlags flags)
 {
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
     NBDRequest request = {
@@ -1254,6 +1254,8 @@ static int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
         .len = bytes,
     };
 
+    assert(bytes < INT_MAX);
+
     assert(!(s->info.flags & NBD_FLAG_READ_ONLY));
     if (!(s->info.flags & NBD_FLAG_SEND_WRITE_ZEROES)) {
         return -ENOTSUP;
diff --git a/block/nvme.c b/block/nvme.c
index d91bbe60bf..724f894b00 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1104,10 +1104,10 @@ static coroutine_fn int nvme_co_flush(BlockDriverState *bs)
 }
 
 
-static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
-                                              int64_t offset,
-                                              int bytes,
-                                              BdrvRequestFlags flags)
+static coroutine_fn int nvme_co_pwrite_zeroes_old(BlockDriverState *bs,
+                                                  int64_t offset,
+                                                  int bytes,
+                                                  BdrvRequestFlags flags)
 {
     BDRVNVMeState *s = bs->opaque;
     NVMeQueuePair *ioq = s->queues[1];
@@ -1157,6 +1157,14 @@ static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
     return data.ret;
 }
 
+static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
+                                              int64_t offset,
+                                              int64_t bytes,
+                                              BdrvRequestFlags flags)
+{
+    assert(bytes <= INT_MAX);
+    return nvme_co_pwrite_zeroes_old(bs, offset, bytes, flags);
+}
 
 static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
                                          int64_t offset,
diff --git a/block/qcow2.c b/block/qcow2.c
index 15a6d3c6ef..0800d0378a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3727,7 +3727,7 @@ static bool is_zero(BlockDriverState *bs, int64_t offset, int64_t bytes)
     return res >= 0 && (res & BDRV_BLOCK_ZERO) && nr == bytes;
 }
 
-static coroutine_fn int qcow2_co_pwrite_zeroes(BlockDriverState *bs,
+static coroutine_fn int qcow2_co_pwrite_zeroes_old(BlockDriverState *bs,
     int64_t offset, int bytes, BdrvRequestFlags flags)
 {
     int ret;
@@ -3779,6 +3779,13 @@ static coroutine_fn int qcow2_co_pwrite_zeroes(BlockDriverState *bs,
     return ret;
 }
 
+static coroutine_fn int qcow2_co_pwrite_zeroes(BlockDriverState *bs,
+    int64_t offset, int64_t bytes, BdrvRequestFlags flags)
+{
+    assert(bytes < INT_MAX);
+    return qcow2_co_pwrite_zeroes_old(bs, offset, bytes, flags);
+}
+
 static coroutine_fn int qcow2_co_pdiscard(BlockDriverState *bs,
                                           int64_t offset, int bytes)
 {
diff --git a/block/qed.c b/block/qed.c
index 1af9b3cb1d..fe00dbbff5 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1439,10 +1439,10 @@ static int coroutine_fn bdrv_qed_co_writev(BlockDriverState *bs,
     return qed_co_request(bs, sector_num, qiov, nb_sectors, QED_AIOCB_WRITE);
 }
 
-static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs,
-                                                  int64_t offset,
-                                                  int bytes,
-                                                  BdrvRequestFlags flags)
+static int coroutine_fn bdrv_qed_co_pwrite_zeroes_old(BlockDriverState *bs,
+                                                      int64_t offset,
+                                                      int bytes,
+                                                      BdrvRequestFlags flags)
 {
     BDRVQEDState *s = bs->opaque;
 
@@ -1463,6 +1463,15 @@ static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs,
                           QED_AIOCB_WRITE | QED_AIOCB_ZERO);
 }
 
+static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs,
+                                                  int64_t offset,
+                                                  int64_t bytes,
+                                                  BdrvRequestFlags flags)
+{
+    assert(bytes <= INT_MAX);
+    return bdrv_qed_co_pwrite_zeroes_old(bs, offset, bytes, flags);
+}
+
 static int coroutine_fn bdrv_qed_co_truncate(BlockDriverState *bs,
                                              int64_t offset,
                                              bool exact,
diff --git a/block/raw-format.c b/block/raw-format.c
index fdf024705b..0996741056 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -279,7 +279,7 @@ static int coroutine_fn raw_co_block_status(BlockDriverState *bs,
 }
 
 static int coroutine_fn raw_co_pwrite_zeroes(BlockDriverState *bs,
-                                             int64_t offset, int bytes,
+                                             int64_t offset, int64_t bytes,
                                              BdrvRequestFlags flags)
 {
     int ret;
diff --git a/block/throttle.c b/block/throttle.c
index af8cc3b2dd..c97da1d6a7 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -137,7 +137,7 @@ static int coroutine_fn throttle_co_pwritev(BlockDriverState *bs,
 }
 
 static int coroutine_fn throttle_co_pwrite_zeroes(BlockDriverState *bs,
-                                                  int64_t offset, int bytes,
+                                                  int64_t offset, int64_t bytes,
                                                   BdrvRequestFlags flags)
 {
     ThrottleGroupMember *tgm = bs->opaque;
diff --git a/block/vmdk.c b/block/vmdk.c
index 61b409ebc7..9611bb191e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2089,7 +2089,7 @@ vmdk_co_pwritev_compressed(BlockDriverState *bs, int64_t offset, int64_t bytes,
 
 static int coroutine_fn vmdk_co_pwrite_zeroes(BlockDriverState *bs,
                                               int64_t offset,
-                                              int bytes,
+                                              int64_t bytes,
                                               BdrvRequestFlags flags)
 {
     int ret;
-- 
2.21.0

[PATCH v3 17/17] block: use int64_t instead of int in driver discard handlers

[Vladimir Sementsov-Ogievskiy]

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.

This patch just converts handlers where it is obvious that bytes
parameter is passed further to 64bit interfaces, and add simple
wrappers where it is not obvious.

Series: 64bit-block-status
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block_int.h |  2 +-
 block/backup-top.c        |  2 +-
 block/blkdebug.c          |  2 +-
 block/blklogwrites.c      |  4 ++--
 block/blkreplay.c         |  2 +-
 block/copy-on-read.c      |  2 +-
 block/file-posix.c        | 18 ++++++++++++++++--
 block/filter-compress.c   |  2 +-
 block/gluster.c           |  6 ++++--
 block/iscsi.c             | 10 +++++++++-
 block/mirror.c            |  2 +-
 block/nbd.c               |  4 +++-
 block/nvme.c              | 13 ++++++++++---
 block/qcow2.c             |  2 +-
 block/raw-format.c        |  2 +-
 block/sheepdog.c          | 11 +++++++++--
 block/throttle.c          |  2 +-
 17 files changed, 63 insertions(+), 23 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index c98d591a56..ff8860fee8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -248,7 +248,7 @@ struct BlockDriver {
     int coroutine_fn (*bdrv_co_pwrite_zeroes)(BlockDriverState *bs,
         int64_t offset, int64_t bytes, BdrvRequestFlags flags);
     int coroutine_fn (*bdrv_co_pdiscard)(BlockDriverState *bs,
-        int64_t offset, int bytes);
+        int64_t offset, int64_t bytes);
 
     /* Map [offset, offset + nbytes) range onto a child of @bs to copy from,
      * and invoke bdrv_co_copy_range_from(child, ...), or invoke
diff --git a/block/backup-top.c b/block/backup-top.c
index 2484293fb0..8c02fa3a58 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -65,7 +65,7 @@ static coroutine_fn int backup_top_cbw(BlockDriverState *bs, uint64_t offset,
 }
 
 static int coroutine_fn backup_top_co_pdiscard(BlockDriverState *bs,
-                                               int64_t offset, int bytes)
+                                               int64_t offset, int64_t bytes)
 {
     int ret = backup_top_cbw(bs, offset, bytes, 0);
     if (ret < 0) {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index d593d6c85d..e1d91cf707 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -705,7 +705,7 @@ static int coroutine_fn blkdebug_co_pwrite_zeroes(BlockDriverState *bs,
 }
 
 static int coroutine_fn blkdebug_co_pdiscard(BlockDriverState *bs,
-                                             int64_t offset, int bytes)
+                                             int64_t offset, int64_t bytes)
 {
     uint32_t align = bs->bl.pdiscard_alignment;
     int err;
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index 8ca41d09cd..f488864106 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -490,9 +490,9 @@ static int coroutine_fn blk_log_writes_co_flush_to_disk(BlockDriverState *bs)
 }
 
 static int coroutine_fn
-blk_log_writes_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
+blk_log_writes_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
 {
-    return blk_log_writes_co_log(bs, offset, count, NULL, 0,
+    return blk_log_writes_co_log(bs, offset, bytes, NULL, 0,
                                  blk_log_writes_co_do_file_pdiscard,
                                  LOG_DISCARD_FLAG, false);
 }
diff --git a/block/blkreplay.c b/block/blkreplay.c
index 186d28cc6a..34a12ad5ac 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -106,7 +106,7 @@ static int coroutine_fn blkreplay_co_pwrite_zeroes(BlockDriverState *bs,
 }
 
 static int coroutine_fn blkreplay_co_pdiscard(BlockDriverState *bs,
-                                              int64_t offset, int bytes)
+                                              int64_t offset, int64_t bytes)
 {
     uint64_t reqid = blkreplay_next_id();
     int ret = bdrv_co_pdiscard(bs->file, offset, bytes);
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index d99e07e99f..5bf1fabee6 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -102,7 +102,7 @@ static int coroutine_fn cor_co_pwrite_zeroes(BlockDriverState *bs,
 
 
 static int coroutine_fn cor_co_pdiscard(BlockDriverState *bs,
-                                        int64_t offset, int bytes)
+                                        int64_t offset, int64_t bytes)
 {
     return bdrv_co_pdiscard(bs->file, offset, bytes);
 }
diff --git a/block/file-posix.c b/block/file-posix.c
index fa9ac5b13e..132001dba8 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2791,11 +2791,18 @@ raw_do_pdiscard(BlockDriverState *bs, int64_t offset, int bytes, bool blkdev)
 }
 
 static coroutine_fn int
-raw_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
+raw_co_pdiscard_old(BlockDriverState *bs, int64_t offset, int bytes)
 {
     return raw_do_pdiscard(bs, offset, bytes, false);
 }
 
+static coroutine_fn int
+raw_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
+{
+    assert(bytes <= INT_MAX);
+    return raw_co_pdiscard_old(bs, offset, bytes);
+}
+
 static int coroutine_fn
 raw_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes,
                      BdrvRequestFlags flags, bool blkdev)
@@ -3475,7 +3482,7 @@ static int fd_open(BlockDriverState *bs)
 }
 
 static coroutine_fn int
-hdev_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
+hdev_co_pdiscard_old(BlockDriverState *bs, int64_t offset, int bytes)
 {
     BDRVRawState *s = bs->opaque;
     int ret;
@@ -3488,6 +3495,13 @@ hdev_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
     return raw_do_pdiscard(bs, offset, bytes, true);
 }
 
+static coroutine_fn int
+hdev_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
+{
+    assert(bytes <= INT_MAX);
+    return hdev_co_pdiscard_old(bs, offset, bytes);
+}
+
 static coroutine_fn int hdev_co_pwrite_zeroes(BlockDriverState *bs,
     int64_t offset, int64_t bytes, BdrvRequestFlags flags)
 {
diff --git a/block/filter-compress.c b/block/filter-compress.c
index 7cf47608b5..477dfaf959 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -93,7 +93,7 @@ static int coroutine_fn compress_co_pwrite_zeroes(BlockDriverState *bs,
 
 
 static int coroutine_fn compress_co_pdiscard(BlockDriverState *bs,
-                                             int64_t offset, int bytes)
+                                             int64_t offset, int64_t bytes)
 {
     return bdrv_co_pdiscard(bs->file, offset, bytes);
 }
diff --git a/block/gluster.c b/block/gluster.c
index 88130c3d2d..87cf69199d 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1312,18 +1312,20 @@ error:
 
 #ifdef CONFIG_GLUSTERFS_DISCARD
 static coroutine_fn int qemu_gluster_co_pdiscard(BlockDriverState *bs,
-                                                 int64_t offset, int size)
+                                                 int64_t offset, int64_t bytes)
 {
     int ret;
     GlusterAIOCB acb;
     BDRVGlusterState *s = bs->opaque;
 
+    assert(bytes <= INT_MAX);
+
     acb.size = 0;
     acb.ret = 0;
     acb.coroutine = qemu_coroutine_self();
     acb.aio_context = bdrv_get_aio_context(bs);
 
-    ret = glfs_discard_async(s->fd, offset, size, gluster_finish_aiocb, &acb);
+    ret = glfs_discard_async(s->fd, offset, bytes, gluster_finish_aiocb, &acb);
     if (ret < 0) {
         return -errno;
     }
diff --git a/block/iscsi.c b/block/iscsi.c
index c4183ef12f..c06521b74f 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1141,7 +1141,8 @@ iscsi_getlength(BlockDriverState *bs)
 }
 
 static int
-coroutine_fn iscsi_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
+coroutine_fn iscsi_co_pdiscard_old(BlockDriverState *bs, int64_t offset,
+                                   int bytes)
 {
     IscsiLun *iscsilun = bs->opaque;
     struct IscsiTask iTask;
@@ -1203,6 +1204,13 @@ out_unlock:
     return r;
 }
 
+static int
+coroutine_fn iscsi_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
+{
+    assert(bytes <= INT_MAX);
+    return iscsi_co_pdiscard_old(bs, offset, bytes);
+}
+
 static int
 coroutine_fn iscsi_co_pwrite_zeroes_old(BlockDriverState *bs, int64_t offset,
                                         int bytes, BdrvRequestFlags flags)
diff --git a/block/mirror.c b/block/mirror.c
index 7f4fdfb470..bd01c032fa 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1471,7 +1471,7 @@ static int coroutine_fn bdrv_mirror_top_pwrite_zeroes(BlockDriverState *bs,
 }
 
 static int coroutine_fn bdrv_mirror_top_pdiscard(BlockDriverState *bs,
-    int64_t offset, int bytes)
+    int64_t offset, int64_t bytes)
 {
     return bdrv_mirror_top_do_write(bs, MIRROR_METHOD_DISCARD, offset, bytes,
                                     NULL, 0);
diff --git a/block/nbd.c b/block/nbd.c
index 0e52b76d72..93d0ced28c 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1295,7 +1295,7 @@ 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 = {
@@ -1304,6 +1304,8 @@ static int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset,
         .len = bytes,
     };
 
+    assert(bytes < INT_MAX);
+
     assert(!(s->info.flags & NBD_FLAG_READ_ONLY));
     if (!(s->info.flags & NBD_FLAG_SEND_TRIM) || !bytes) {
         return 0;
diff --git a/block/nvme.c b/block/nvme.c
index 724f894b00..adc19ee9e6 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1166,9 +1166,9 @@ static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
     return nvme_co_pwrite_zeroes_old(bs, offset, bytes, flags);
 }
 
-static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
-                                         int64_t offset,
-                                         int bytes)
+static int coroutine_fn nvme_co_pdiscard_old(BlockDriverState *bs,
+                                             int64_t offset,
+                                             int bytes)
 {
     BDRVNVMeState *s = bs->opaque;
     NVMeQueuePair *ioq = s->queues[1];
@@ -1245,6 +1245,13 @@ out:
 
 }
 
+static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
+                                         int64_t offset,
+                                         int64_t bytes)
+{
+    assert(bytes <= INT_MAX);
+    return nvme_co_pdiscard_old(bs, offset, bytes);
+}
 
 static int nvme_reopen_prepare(BDRVReopenState *reopen_state,
                                BlockReopenQueue *queue, Error **errp)
diff --git a/block/qcow2.c b/block/qcow2.c
index 0800d0378a..c85f5a8331 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3787,7 +3787,7 @@ static coroutine_fn int qcow2_co_pwrite_zeroes(BlockDriverState *bs,
 }
 
 static coroutine_fn int qcow2_co_pdiscard(BlockDriverState *bs,
-                                          int64_t offset, int bytes)
+                                          int64_t offset, int64_t bytes)
 {
     int ret;
     BDRVQcow2State *s = bs->opaque;
diff --git a/block/raw-format.c b/block/raw-format.c
index 0996741056..f31bc4338f 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -292,7 +292,7 @@ static int coroutine_fn raw_co_pwrite_zeroes(BlockDriverState *bs,
 }
 
 static int coroutine_fn raw_co_pdiscard(BlockDriverState *bs,
-                                        int64_t offset, int bytes)
+                                        int64_t offset, int64_t bytes)
 {
     int ret;
 
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 59f7ebb171..750bfae016 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -3091,8 +3091,8 @@ static int sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
 }
 
 
-static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
-                                      int bytes)
+static coroutine_fn int sd_co_pdiscard_old(BlockDriverState *bs, int64_t offset,
+                                           int bytes)
 {
     SheepdogAIOCB acb;
     BDRVSheepdogState *s = bs->opaque;
@@ -3121,6 +3121,13 @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
     return acb.ret;
 }
 
+static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
+                                       int64_t bytes)
+{
+    assert(bytes <= INT_MAX);
+    return sd_co_pdiscard_old(bs, offset, bytes);
+}
+
 static coroutine_fn int
 sd_co_block_status(BlockDriverState *bs, bool want_zero, int64_t offset,
                    int64_t bytes, int64_t *pnum, int64_t *map,
diff --git a/block/throttle.c b/block/throttle.c
index c97da1d6a7..61f6c291e7 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -147,7 +147,7 @@ static int coroutine_fn throttle_co_pwrite_zeroes(BlockDriverState *bs,
 }
 
 static int coroutine_fn throttle_co_pdiscard(BlockDriverState *bs,
-                                             int64_t offset, int bytes)
+                                             int64_t offset, int64_t bytes)
 {
     ThrottleGroupMember *tgm = bs->opaque;
     throttle_group_co_io_limits_intercept(tgm, bytes, true);
-- 
2.21.0

Re: [PATCH v3 17/17] block: use int64_t instead of int in driver discard handlers

[Vladimir Sementsov-Ogievskiy]

30.04.2020 14:10, 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.
> 
> This patch just converts handlers where it is obvious that bytes
> parameter is passed further to 64bit interfaces, and add simple
> wrappers where it is not obvious.
> 
> Series: 64bit-block-status
> Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>


squash in:

--- a/tests/test-block-iothread.c
+++ b/tests/test-block-iothread.c
@@ -48,7 +48,7 @@ static int coroutine_fn bdrv_test_co_pwritev(BlockDriverState *bs,
  }
  
  static int coroutine_fn bdrv_test_co_pdiscard(BlockDriverState *bs,
-                                              int64_t offset, int bytes)
+                                              int64_t offset, int64_t bytes)
  {
      return 0;
  }

Re: [PATCH v3 00/17] 64bit block-layer

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200430111033.29980-1-vsementsov@virtuozzo.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      tests/test-timed-average.o
  CC      tests/test-util-filemonitor.o
  CC      tests/test-util-sockets.o
/tmp/qemu-test/src/tests/test-block-iothread.c:79:5: error: initialization from incompatible pointer type [-Werror]
     .bdrv_co_pdiscard       = bdrv_test_co_pdiscard,
     ^
/tmp/qemu-test/src/tests/test-block-iothread.c:79:5: error: (near initialization for 'bdrv_test.bdrv_co_pdiscard') [-Werror]
cc1: all warnings being treated as errors
  CC      tests/test-authz-simple.o
make: *** [tests/test-block-iothread.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=42df3799b5074b82a8a4b696cb76547f', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-t9q8cenf/src/docker-src.2020-04-30-16.47.42.6034:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=42df3799b5074b82a8a4b696cb76547f
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-t9q8cenf/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    3m38.963s
user    0m9.441s


The full log is available at
http://patchew.org/logs/20200430111033.29980-1-vsementsov@virtuozzo.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v3 00/17] 64bit block-layer

[Vladimir Sementsov-Ogievskiy]

30.04.2020 23:51, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200430111033.29980-1-vsementsov@virtuozzo.com/
> 
> 
> 
> Hi,
> 
> This series failed the docker-quick@centos7 build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> make docker-image-centos7 V=1 NETWORK=1
> time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
> === TEST SCRIPT END ===
> 
>    CC      tests/test-timed-average.o
>    CC      tests/test-util-filemonitor.o
>    CC      tests/test-util-sockets.o
> /tmp/qemu-test/src/tests/test-block-iothread.c:79:5: error: initialization from incompatible pointer type [-Werror]
>       .bdrv_co_pdiscard       = bdrv_test_co_pdiscard,
>       ^

Oh yes. Shows me didn't run make check :(

> /tmp/qemu-test/src/tests/test-block-iothread.c:79:5: error: (near initialization for 'bdrv_test.bdrv_co_pdiscard') [-Werror]
> cc1: all warnings being treated as errors
>    CC      tests/test-authz-simple.o
> make: *** [tests/test-block-iothread.o] Error 1
> make: *** Waiting for unfinished jobs....
> Traceback (most recent call last):
>    File "./tests/docker/docker.py", line 664, in <module>
> ---
>      raise CalledProcessError(retcode, cmd)
> subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=42df3799b5074b82a8a4b696cb76547f', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-t9q8cenf/src/docker-src.2020-04-30-16.47.42.6034:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
> filter=--filter=label=com.qemu.instance.uuid=42df3799b5074b82a8a4b696cb76547f
> make[1]: *** [docker-run] Error 1
> make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-t9q8cenf/src'
> make: *** [docker-run-test-quick@centos7] Error 2
> 
> real    3m38.963s
> user    0m9.441s
> 
> 
> The full log is available at
> http://patchew.org/logs/20200430111033.29980-1-vsementsov@virtuozzo.com/testing.docker-quick@centos7/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
> 


-- 
Best regards,
Vladimir

Re: [PATCH v3 00/17] 64bit block-layer

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200430111033.29980-1-vsementsov@virtuozzo.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

clang -iquote /tmp/qemu-test/build/tests -iquote tests -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/usr/include/pixman-1   -Werror -fsanitize=undefined -fsanitize=address  -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong   -I/usr/include/p11-kit-1   -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED  -I/usr/include/libpng16  -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests -I/tmp/qemu-test/src/tests/qtest -MMD -MP -MT tests/test-crypto-secret.o -MF tests/test-crypto-secret.d -g   -c -o tests/test-crypto-secret.o /tmp/qemu-test/src/tests/test-crypto-secret.c
clang -iquote /tmp/qemu-test/build/tests -iquote tests -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/usr/include/pixman-1   -Werror -fsanitize=undefined -fsanitize=address  -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong   -I/usr/include/p11-kit-1   -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED  -I/usr/include/libpng16  -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests -I/tmp/qemu-test/src/tests/qtest -MMD -MP -MT tests/test-crypto-tlscredsx509.o -MF tests/test-crypto-tlscredsx509.d -g   -c -o tests/test-crypto-tlscredsx509.o /tmp/qemu-test/src/tests/test-crypto-tlscredsx509.c
clang -iquote /tmp/qemu-test/build/tests -iquote tests -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/usr/include/pixman-1   -Werror -fsanitize=undefined -fsanitize=address  -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong   -I/usr/include/p11-kit-1   -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED  -I/usr/include/libpng16  -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests -I/tmp/qemu-test/src/tests/qtest -MMD -MP -MT tests/crypto-tls-x509-helpers.o -MF tests/crypto-tls-x509-helpers.d -g   -c -o tests/crypto-tls-x509-helpers.o /tmp/qemu-test/src/tests/crypto-tls-x509-helpers.c
/tmp/qemu-test/src/tests/test-block-iothread.c:79:31: error: incompatible pointer types initializing 'int (*)(BlockDriverState *, int64_t, int64_t)' (aka 'int (*)(struct BlockDriverState *, long, long)') with an expression of type 'int (BlockDriverState *, int64_t, int)' (aka 'int (struct BlockDriverState *, long, int)') [-Werror,-Wincompatible-pointer-types]
    .bdrv_co_pdiscard       = bdrv_test_co_pdiscard,
                              ^~~~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: tests/test-block-iothread.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=e0743bd266804107bb9089aec26150f7', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-39rsbv95/src/docker-src.2020-04-30-16.52.24.18470:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=e0743bd266804107bb9089aec26150f7
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-39rsbv95/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    4m51.462s
user    0m8.644s


The full log is available at
http://patchew.org/logs/20200430111033.29980-1-vsementsov@virtuozzo.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v3 00/17] 64bit block-layer

[Vladimir Sementsov-Ogievskiy]

Hi!

I'm sorry, I should have pinged it, or resend, or suggest to pull at least a half long ago :(

I've rebased it on master and make some fixes.

What to do next? I can just resend. But I'm afraid that Eric's careful audits may be out of date: time passed, there is no guarantee that callers are not changed. Really sorry :(
So r-b marks are not applicable as well, yes?

But if I just resend it with no r-bs, is it feasible to review/merge it in a finite time? So that audits of patches will not become outdated?

Any ideas?

I have an idea: instead of auditing each function callers, can we just make some good assumptions (like that the whole offset/bytes request being aligned to bs->request_alignement doesn't lay inside [0..INT64_MAX] region), check it once in bdrv_check_bytes_request() and assert in each function we convert to int64_t.

Then, if somewhere our assumption is wrong, we'll have a crash and fix the bug.

30.04.2020 14:10, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> We want 64bit write-zeroes, and for this, convert all io functions to
> 64bit.
> 
> 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).
> 
> Please refer to initial cover-letter
>   https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg08723.html
> for more info.
> 
> v3 is available at
>   https://src.openvz.org/scm/~vsementsov/qemu.git #tag up-64bit-block-layer-v3
> 
> v3: Based on "[PATCH v2 0/9] block/io: safer inc/dec in_flight sections"
> Add Eric's r-bs, improve commit message with short reasoning of the whole
> thing, and Eric's audits (if you don't like something, I'll change or drop for
> next series).
> 
> Add "Series:" tag to each patch. Just an idea, if it's inappropriate thing,
> I'll drop it.
> 
> 01: add assertion that bytes > 0
> 02: fix indentation
> 06: refactor calculations in bdrv_co_write_req_prepare
> 09,10: simple rebase conflicts solved
> 
> Also, cover more drivers by driver-updating patches and fix int flags
> to be BdrvRequestFlags flags.
> 
> Based-on: <20200427143907.5710-1-vsementsov@virtuozzo.com>
> Series: 64bit-block-status
> 
> Vladimir Sementsov-Ogievskiy (17):
>    block/throttle-groups: throttle_group_co_io_limits_intercept(): 64bit
>      bytes
>    block: use int64_t as bytes type in tracked requests
>    block/io: use int64_t bytes parameter in bdrv_check_byte_request()
>    block/io: use int64_t bytes in driver wrappers
>    block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes()
>    block/io: support int64_t bytes in bdrv_aligned_pwritev()
>    block/io: support int64_t bytes in bdrv_co_do_copy_on_readv()
>    block/io: support int64_t bytes in bdrv_aligned_preadv()
>    block/io: support int64_t bytes in bdrv_co_p{read,write}v_part()
>    block/io: support int64_t bytes in read/write wrappers
>    block/io: use int64_t bytes in copy_range
>    block/block-backend: convert blk io path to use int64_t parameters
>    block: use int64_t instead of uint64_t in driver read handlers
>    block: use int64_t instead of uint64_t in driver write handlers
>    block: use int64_t instead of uint64_t in copy_range driver handlers
>    block: use int64_t instead of int in driver write_zeroes handlers
>    block: use int64_t instead of int in driver discard handlers
> 
>   include/block/block.h           |  17 +++--
>   include/block/block_int.h       |  67 ++++++++---------
>   include/block/throttle-groups.h |   2 +-
>   include/sysemu/block-backend.h  |  26 +++----
>   block/backup-top.c              |  14 ++--
>   block/blkdebug.c                |  12 +--
>   block/blklogwrites.c            |  16 ++--
>   block/blkreplay.c               |   8 +-
>   block/blkverify.c               |  10 +--
>   block/block-backend.c           |  60 +++++++--------
>   block/bochs.c                   |   4 +-
>   block/cloop.c                   |   4 +-
>   block/commit.c                  |   2 +-
>   block/copy-on-read.c            |  14 ++--
>   block/crypto.c                  |   8 +-
>   block/curl.c                    |   3 +-
>   block/dmg.c                     |   4 +-
>   block/file-posix.c              |  46 ++++++++----
>   block/file-win32.c              |   8 +-
>   block/filter-compress.c         |  15 ++--
>   block/gluster.c                 |  14 ++--
>   block/io.c                      | 126 +++++++++++++++++---------------
>   block/iscsi.c                   |  34 ++++++---
>   block/mirror.c                  |   8 +-
>   block/nbd.c                     |  18 +++--
>   block/nfs.c                     |  12 +--
>   block/null.c                    |  18 +++--
>   block/nvme.c                    |  38 +++++++---
>   block/qcow.c                    |  16 ++--
>   block/qcow2.c                   |  34 +++++----
>   block/qed.c                     |  17 ++++-
>   block/quorum.c                  |   9 ++-
>   block/raw-format.c              |  36 ++++-----
>   block/rbd.c                     |  10 ++-
>   block/sheepdog.c                |  11 ++-
>   block/throttle-groups.c         |   5 +-
>   block/throttle.c                |  14 ++--
>   block/vdi.c                     |   8 +-
>   block/vmdk.c                    |  14 ++--
>   block/vpc.c                     |   8 +-
>   block/vvfat.c                   |  12 +--
>   block/vxhs.c                    |   8 +-
>   tests/test-bdrv-drain.c         |  16 ++--
>   tests/test-block-iothread.c     |  19 +++--
>   block/trace-events              |  14 ++--
>   45 files changed, 488 insertions(+), 371 deletions(-)
> 


-- 
Best regards,
Vladimir

Re: [PATCH v3 00/17] 64bit block-layer

[Vladimir Sementsov-Ogievskiy]

01.12.2020 19:07, Vladimir Sementsov-Ogievskiy wrote:
> 
> I have an idea: instead of auditing each function callers, can we just make some good assumptions (like that the whole offset/bytes request being aligned to bs->request_alignement doesn't lay inside [0..INT64_MAX] region), check it once in bdrv_check_bytes_request() and assert in each function we convert to int64_t.


s/doesn't//

-- 
Best regards,
Vladimir

Re: [PATCH v3 00/17] 64bit block-layer

[Eric Blake]

On 12/1/20 10:07 AM, Vladimir Sementsov-Ogievskiy wrote:
> Hi!
> 
> I'm sorry, I should have pinged it, or resend, or suggest to pull at
> least a half long ago :(
> 
> I've rebased it on master and make some fixes.
> 
> What to do next? I can just resend. But I'm afraid that Eric's careful
> audits may be out of date: time passed, there is no guarantee that
> callers are not changed. Really sorry :(
> So r-b marks are not applicable as well, yes?

If you think the rebase has fundamentally changed things, then dropping
the r-b is safest.  I will probably spend a good time on the audit
again, but this time, I want to see the project through to completion,
and am willing to take patches through my NBD tree if Kevin or other
block maintainers do not have enough time to take it through a broader
block tree.  I can justify it because I have a specific patch in NBD
that will benefit from this audit - I want to rever 890cbccb08 in favor
of using saner 64-bit APIs throughout the block layer.  But I am also
aware that your patches touch more than NBD, so even if Kevin can't
commit to a full review, I will at least try to get his Acked-by.

> 
> But if I just resend it with no r-bs, is it feasible to review/merge it
> in a finite time? So that audits of patches will not become outdated?

Yes, let's agree to put a lot more effort into getting this series in
for 6.0.

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