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

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

[Vladimir Sementsov-Ogievskiy]

Hi all!

v1 was "[RFC 0/3] 64bit block-layer part I", please refer to initial
cover-letter 
 https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg08723.html
for motivation.

v2:
patch 02 is unchanged, add Stefan's r-b. Everything other is changed a
lot. What's new:

- conversion of block/io.c is now done step-by-step, to make careful
  review possible as well as future bisecting

- converting of driver handlers split by io type

- convert write_zeroes and discard (so the series is not called "part I"
  any more). I decided to convert most of things alltogether, leaving
  simple wrappers only in unobvious places. Still, if you consider it
  risky, I can refactor it to use only wrappers as a first patch and
  then update driver-by-driver, but it would be lot more patches, I'm
  not sure it worth doing.

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           |  16 ++---
 include/block/block_int.h       |  56 ++++++++--------
 include/block/throttle-groups.h |   2 +-
 include/sysemu/block-backend.h  |  26 ++++----
 block/backup-top.c              |   9 ++-
 block/blkdebug.c                |   8 +--
 block/blklogwrites.c            |  12 ++--
 block/blkreplay.c               |   8 +--
 block/blkverify.c               |   6 +-
 block/block-backend.c           |  60 +++++++++---------
 block/bochs.c                   |   2 +-
 block/cloop.c                   |   2 +-
 block/commit.c                  |   2 +-
 block/copy-on-read.c            |   8 +--
 block/crypto.c                  |   4 +-
 block/curl.c                    |   2 +-
 block/dmg.c                     |   2 +-
 block/file-posix.c              |  42 ++++++++----
 block/filter-compress.c         |  10 +--
 block/gluster.c                 |  14 ++--
 block/io.c                      | 109 +++++++++++++++++---------------
 block/iscsi.c                   |  34 +++++++---
 block/mirror.c                  |   8 +--
 block/nbd.c                     |  16 +++--
 block/nfs.c                     |   8 +--
 block/null.c                    |   8 +--
 block/nvme.c                    |  33 +++++++---
 block/qcow.c                    |  12 ++--
 block/qcow2.c                   |  29 +++++----
 block/qed.c                     |  17 +++--
 block/quorum.c                  |   8 +--
 block/raw-format.c              |  32 +++++-----
 block/rbd.c                     |   4 +-
 block/sheepdog.c                |  11 +++-
 block/throttle-groups.c         |   2 +-
 block/throttle.c                |   8 +--
 block/vdi.c                     |   4 +-
 block/vmdk.c                    |  10 +--
 block/vpc.c                     |   4 +-
 block/vvfat.c                   |   6 +-
 tests/test-bdrv-drain.c         |   8 +--
 block/trace-events              |  14 ++--
 42 files changed, 379 insertions(+), 297 deletions(-)

-- 
2.21.0

[PATCH v2 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.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/throttle-groups.h | 2 +-
 block/throttle-groups.c         | 2 +-
 2 files changed, 2 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..37d1b7a8b8 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -358,7 +358,7 @@ 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;
-- 
2.21.0

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

[Philippe Mathieu-Daudé]

On 4/27/20 10:23 AM, 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.

What is the meaning of negative bytes in this function?

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   include/block/throttle-groups.h | 2 +-
>   block/throttle-groups.c         | 2 +-
>   2 files changed, 2 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..37d1b7a8b8 100644
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -358,7 +358,7 @@ 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;
> 

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

[Eric Blake]

On 4/27/20 5:05 AM, Philippe Mathieu-Daudé wrote:
> On 4/27/20 10:23 AM, 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.
> 
> What is the meaning of negative bytes in this function?

An error.  It is more for consistency, in that we really cannot access 
more than 63 bits of size information (off_t is signed, and network 
protocols don't magically add a 64th bit to that underlying inherent 
limitation of files and block devices).

It may be worth adding assert(bytes >= 0), if that makes it easier to 
prove we are only dealing with positive lengths.

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

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

[Eric Blake]

On 4/27/20 3:23 AM, 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.

My audit for this patch:

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.

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

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

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

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

[Vladimir Sementsov-Ogievskiy]

29.04.2020 1:09, Eric Blake wrote:
> On 4/27/20 3:23 AM, 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.
> 
> My audit for this patch:
> 
> 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.

Should I add this all to commit message?

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

Thanks for careful review!

-- 
Best regards,
Vladimir

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

[Eric Blake]

On 4/29/20 12:05 AM, Vladimir Sementsov-Ogievskiy wrote:
> 29.04.2020 1:09, Eric Blake wrote:
>> On 4/27/20 3:23 AM, 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.
>>
>> My audit for this patch:
>>
>> 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.
> 
> Should I add this all to commit message?

It never hurts to provide such rationale ;)

> 
>>
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   include/block/throttle-groups.h | 2 +-
>>>   block/throttle-groups.c         | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>
> 
> Thanks for careful review!
> 

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

[PATCH v2 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. Convert tracked requests now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@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 aba67f66b9..7cbb80bd24 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 v2 02/17] block: use int64_t as bytes type in tracked requests

[Philippe Mathieu-Daudé]

On 4/27/20 10:23 AM, Vladimir Sementsov-Ogievskiy wrote:
> We are generally moving to int64_t for both offset and bytes parameters
> on all io paths. Convert tracked requests now.

This doesn't seem a strong justification... If I understand correctly 
this patch, it is safer to use positive signed type rather than unsigned 
type. OK it might make sense to better catch overflow, but it should be 
explained in the function prototypes, else commit message, else the 
series cover IMHO.

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@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 aba67f66b9..7cbb80bd24 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);
> 

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

[Vladimir Sementsov-Ogievskiy]

27.04.2020 13:11, Philippe Mathieu-Daudé wrote:
> On 4/27/20 10:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>> We are generally moving to int64_t for both offset and bytes parameters
>> on all io paths. Convert tracked requests now.
> 
> This doesn't seem a strong justification... If I understand correctly this patch, it is safer to use positive signed type rather than unsigned type. OK it might make sense to better catch overflow, but it should be explained in the function prototypes, else commit message, else the series cover IMHO.

First time I decided to follow the tendency not to copy the whole cover-letter from previous series, but just give a link to it :) It's chosen not for safety..

My reason is the fact that some functions may return int64_t offset/bytes, and negative values are used to indicate an error. It seems good to use same type always, making it simple to reuse local variables for storing return value and as arguments (if it is appropriate in the context).

Eric also added (in v1 thread), that off_t is signed too.

So the aim of the series is not signed type, the aim is 64bit. And for consistency, we should use same type for all io functions. And my proposal is int64_t, for these two reasons above. May be good to add them to the first commit message.

-- 
Best regards,
Vladimir

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

[Philippe Mathieu-Daudé]

On 4/27/20 1:26 PM, Vladimir Sementsov-Ogievskiy wrote:
> 27.04.2020 13:11, Philippe Mathieu-Daudé wrote:
>> On 4/27/20 10:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> We are generally moving to int64_t for both offset and bytes parameters
>>> on all io paths. Convert tracked requests now.
>>
>> This doesn't seem a strong justification... If I understand correctly 
>> this patch, it is safer to use positive signed type rather than 
>> unsigned type. OK it might make sense to better catch overflow, but it 
>> should be explained in the function prototypes, else commit message, 
>> else the series cover IMHO.
> 
> First time I decided to follow the tendency not to copy the whole 
> cover-letter from previous series, but just give a link to it :) It's 
> chosen not for safety..
> 
> My reason is the fact that some functions may return int64_t 
> offset/bytes, and negative values are used to indicate an error. It 
> seems good to use same type always, making it simple to reuse local 
> variables for storing return value and as arguments (if it is 
> appropriate in the context).

I agree we want errors returned, so negative values for that.

I'm not sure it is a good practice to pass unsigned values via signed 
type simply to 'reuse' local variables. I'm worried we might hide new 
bugs and it might become harder to find others bugs.

Anyway I'll follow Eric and Stefan choice here, as they are more 
experienced.

> 
> Eric also added (in v1 thread), that off_t is signed too.
> 
> So the aim of the series is not signed type, the aim is 64bit. And for 
> consistency, we should use same type for all io functions. And my 
> proposal is int64_t, for these two reasons above. May be good to add 
> them to the first commit message.
> 

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

[Vladimir Sementsov-Ogievskiy]

27.04.2020 11:23, Vladimir Sementsov-Ogievskiy wrote:
> We are generally moving to int64_t for both offset and bytes parameters
> on all io paths. Convert tracked requests now.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@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 aba67f66b9..7cbb80bd24 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;

sorry, I forget to fix indentation

>       bool waited;
>   
>       qemu_co_mutex_lock(&bs->reqs_lock);
> 


-- 
Best regards,
Vladimir

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

[Eric Blake]

On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
> We are generally moving to int64_t for both offset and bytes parameters
> on all io paths. Convert tracked requests now.

As mentioned elsewhere in the thread, this states 'what' but not 'why'; 
adding a bit more of the 'why' can be useful when revisiting this commit 
in the future.

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@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;

unsigned values have defined wraparound semantics, signed values have a 
lower maximum and require careful handling to avoid undefined overflow. 
So we have to check all clients for any surprises.

block/file-posix.c:raw_do_pwrite_zeroes() -
         assert(req->offset + req->bytes >= offset + bytes);
pre-patch: assert(int64_t + uint64_t >= int64_t + int)
            assert(uint64_t >= int64_t) - unsigned compare
post-patch: assert(int64_t >= int64_t) - signed compare
Risky if adding req->bytes could ever overflow 63 bits but still fit in 
64 bits, but I couldn't find any way to trigger that.  I think we're 
safe because the block layer never calls a driver's .pwrite_zeroes with 
a bytes that would overflow 63 bits.

block/write-threshold.c:bdrv_write_threshold_exceeded() -
         if ((req->offset + req->bytes) > bs->write_threshold_offset) {
pre-patch: ((int64_t + uint64_t) > uint64_t) - unsigned compare
post-patch: (int64_t > uint64_t) - still unsigned compare

Perhaps that function should be changed to return int64_t, but probably 
as a different patch in the series (I didn't read ahead yet to see if 
you already did).

>   
>       QLIST_ENTRY(BdrvTrackedRequest) list;
>       Coroutine *co; /* owner, used for deadlock detection */
> diff --git a/block/io.c b/block/io.c
> index aba67f66b9..7cbb80bd24 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);

This part is nice - it makes it very easy to prove all other uses of 
BdrvTrackedRequest.bytes were already in range (both pre- and post-patch).

>   
>       *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);

While here, should we use QEMU_ALIGN_DOWN instead of open-coding?

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

Looking through uses of BdrvTrackedRequest in io.c, I couldn't find any 
other surprises (it seems everything starts with tracked_request_begin, 
and while you did switch between unsigned and signed, you did not switch 
width, so it's easier to reason about once we know things don't overflow).

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 v2 02/17] block: use int64_t as bytes type in tracked requests

[Vladimir Sementsov-Ogievskiy]

29.04.2020 18:50, Eric Blake wrote:
> On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>> We are generally moving to int64_t for both offset and bytes parameters
>> on all io paths. Convert tracked requests now.
> 
> As mentioned elsewhere in the thread, this states 'what' but not 'why'; adding a bit more of the 'why' can be useful when revisiting this commit in the future.
> 
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Reviewed-by: Stefan Hajnoczi <stefanha@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;
> 
> unsigned values have defined wraparound semantics, signed values have a lower maximum and require careful handling to avoid undefined overflow. So we have to check all clients for any surprises.
> 
> block/file-posix.c:raw_do_pwrite_zeroes() -
>          assert(req->offset + req->bytes >= offset + bytes);
> pre-patch: assert(int64_t + uint64_t >= int64_t + int)
>             assert(uint64_t >= int64_t) - unsigned compare
> post-patch: assert(int64_t >= int64_t) - signed compare
> Risky if adding req->bytes could ever overflow 63 bits but still fit in 64 bits, but I couldn't find any way to trigger that.  I think we're safe because the block layer never calls a driver's .pwrite_zeroes with a bytes that would overflow 63 bits.
> 
> block/write-threshold.c:bdrv_write_threshold_exceeded() -
>          if ((req->offset + req->bytes) > bs->write_threshold_offset) {
> pre-patch: ((int64_t + uint64_t) > uint64_t) - unsigned compare
> post-patch: (int64_t > uint64_t) - still unsigned compare
> 
> Perhaps that function should be changed to return int64_t, but probably as a different patch in the series (I didn't read ahead yet to see if you already did).
> 
>>       QLIST_ENTRY(BdrvTrackedRequest) list;
>>       Coroutine *co; /* owner, used for deadlock detection */
>> diff --git a/block/io.c b/block/io.c
>> index aba67f66b9..7cbb80bd24 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);
> 
> This part is nice - it makes it very easy to prove all other uses of BdrvTrackedRequest.bytes were already in range (both pre- and post-patch).
> 
>>       *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);
> 
> While here, should we use QEMU_ALIGN_DOWN instead of open-coding?

But then, also s/ROUND_UP/QEMU_ALIGN_UP/ for consistency? But ROUND_UP is faster. Still, we tend to generally use QEMU_ALIGN_UP everywhere.. So, may be better to refactor these thing alltogether in some good way. Maybe, add ROUND_DOWN macro for symmetry?

> 
>> -    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);
>>
> 
> Looking through uses of BdrvTrackedRequest in io.c, I couldn't find any other surprises (it seems everything starts with tracked_request_begin, and while you did switch between unsigned and signed, you did not switch width, so it's easier to reason about once we know things don't overflow).
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> 


-- 
Best regards,
Vladimir

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

[Vladimir Sementsov-Ogievskiy]

29.04.2020 18:50, Eric Blake wrote:
> On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>> We are generally moving to int64_t for both offset and bytes parameters
>> on all io paths. Convert tracked requests now.
> 
> As mentioned elsewhere in the thread, this states 'what' but not 'why'; adding a bit more of the 'why' can be useful when revisiting this commit in the future.
> 
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Reviewed-by: Stefan Hajnoczi <stefanha@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;
> 
> unsigned values have defined wraparound semantics, signed values have a lower maximum and require careful handling to avoid undefined overflow. So we have to check all clients for any surprises.
> 
> block/file-posix.c:raw_do_pwrite_zeroes() -
>          assert(req->offset + req->bytes >= offset + bytes);
> pre-patch: assert(int64_t + uint64_t >= int64_t + int)
>             assert(uint64_t >= int64_t) - unsigned compare
> post-patch: assert(int64_t >= int64_t) - signed compare
> Risky if adding req->bytes could ever overflow 63 bits but still fit in 64 bits, but I couldn't find any way to trigger that.  I think we're safe because the block layer never calls a driver's .pwrite_zeroes with a bytes that would overflow 63 bits.
> 
> block/write-threshold.c:bdrv_write_threshold_exceeded() -
>          if ((req->offset + req->bytes) > bs->write_threshold_offset) {
> pre-patch: ((int64_t + uint64_t) > uint64_t) - unsigned compare
> post-patch: (int64_t > uint64_t) - still unsigned compare
> 
> Perhaps that function should be changed to return int64_t, but probably as a different patch in the series (I didn't read ahead yet to see if you already did).

No, it's not here... But of course converting write-threshold.c should be separate patch.

I think that if we have BdrvTrackedRequest object, it must be valid, and offset + bytes must fit into INT64_MAX. And object-creator is responsible for this, and tracked_request_begin assert this thing.


-- 
Best regards,
Vladimir

[PATCH v2 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. Convert bdrv_check_byte_request() now.

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

diff --git a/block/io.c b/block/io.c
index 7cbb80bd24..1267918def 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 v2 03/17] block/io: use int64_t bytes parameter in bdrv_check_byte_request()

[Eric Blake]

On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
> We are generally moving to int64_t for both offset and bytes parameters
> on all io paths. Convert bdrv_check_byte_request() now.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/io.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 7cbb80bd24..1267918def 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)
>   {

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, but the commit message should 
definitely call out the fix for an unreachable latent bug.

I will also point out that on Linux, copy_file_range() is capped by a 
size_t len parameter, so even if we DO have a 32-bit caller passing > 
2G, it will encounter further truncation bugs if the block layer does 
not fragment the request internally.  I don't see any fragmentation 
logic in the public bdrv_co_copy_range() or in 
bdrv_co_copy_range_internal() - I suspect that needs to be added 
(probably as a separate patch); maybe by moving the fragmentation loop 
currently in block-copy.c to instead live in io.c?

> -    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;
>       }

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

I don't know if we have any iotest coverage of copy_range with a 5G 
image to prove that it doesn't misbehave on a 32-bit machine.  That 
seems like it will eventually be useful, but doesn't necessarily have to 
be at the same time as this patch.

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

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

[Vladimir Sementsov-Ogievskiy]

29.04.2020 22:27, Eric Blake wrote:
> On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>> We are generally moving to int64_t for both offset and bytes parameters
>> on all io paths. Convert bdrv_check_byte_request() now.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block/io.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/io.c b/block/io.c
>> index 7cbb80bd24..1267918def 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)
>>   {
> 
> 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, but the commit message should definitely call out the fix for an unreachable latent bug.
> 
> I will also point out that on Linux, copy_file_range() is capped by a size_t len parameter, so even if we DO have a 32-bit caller passing > 2G, it will encounter further truncation bugs if the block layer does not fragment the request internally.  I don't see any fragmentation logic in the public bdrv_co_copy_range() or in bdrv_co_copy_range_internal() - I suspect that needs to be added (probably as a separate patch); maybe by moving the fragmentation loop currently in block-copy.c to instead live in io.c?

fragmentation loop in io.c should have larger granularity than in block-copy.c, isn't it? Fragmentation loop in block-copy will be async for performance reasons, based on aio-task-pool. And it should cover both copy_range and simple copy and write-zeroes cases.. So, I think it's simpler to keep separate fragmentation loop in io.c. Still it's not really needed until block-copy is the only user of the interface.

> 
>> -    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;
>>       }
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> I don't know if we have any iotest coverage of copy_range with a 5G image to prove that it doesn't misbehave on a 32-bit machine.  That seems like it will eventually be useful, but doesn't necessarily have to be at the same time as this patch.
> 


-- 
Best regards,
Vladimir

[PATCH v2 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. Convert driver wrappers parameters which are
already 64bit to signed type.

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

diff --git a/block/io.c b/block/io.c
index 1267918def..4796476835 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1086,7 +1086,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)
 {
@@ -1155,7 +1155,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)
 {
@@ -1235,8 +1235,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 v2 04/17] block/io: use int64_t bytes in driver wrappers

[Eric Blake]

On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
> We are generally moving to int64_t for both offset and bytes parameters
> on all io paths. Convert driver wrappers parameters which are
> already 64bit to signed type.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/io.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 1267918def..4796476835 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1086,7 +1086,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,

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, 
if you don't fix qemu_iovec
drv->bdrv_co_preadv(uint64_t) - safe
drv->bdrv_aio_preadv(uint64_t) - safe
drv->bdrv_co_readv(int) after assertion of <2G - safe

>                                              QEMUIOVector *qiov,
>                                              size_t qiov_offset, int flags)
>   {
> @@ -1155,7 +1155,7 @@ out:
>   }
>   
>   static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
> -                                            uint64_t offset, uint64_t bytes,
> +                                            int64_t offset, int64_t bytes,

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, 
if you don't fix qemu_iovec
drv->bdrv_co_pwritev(uint64_t) - safe
drv->bdrv_aio_pwritev(uint64_t) - safe
drv->bdrv_co_writev(int) after assertion of <2G - safe

>                                               QEMUIOVector *qiov,
>                                               size_t qiov_offset, int flags)
>   {
> @@ -1235,8 +1235,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)

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

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, 
if you don't fix qemu_iovec

Because our input is bounded, all of the changes made here have no 
semantic impact, and I argue this patch is safe.  However, I also 
pointed out the spots where we have latent bugs (on 32-bit machines 
only) if the callers are relaxed to pass larger than 2G or 4G.  As long 
as you are auditing things, it would be nice to either fix that in this 
patch, or document in the commit message how a future patch will address 
that latent problem (whether by enforcing max_transfer to be capped at 
2G on 32-bit platforms, or else fixing qemu_iovec to use int64_t instead 
of size_t bounds).

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 v2 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. Prepare bdrv_co_do_pwrite_zeroes() now.

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

diff --git a/block/io.c b/block/io.c
index 4796476835..c8c30e3699 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)
@@ -1743,7 +1743,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;
@@ -1773,7 +1773,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 v2 05/17] block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes()

[Eric Blake]

On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
> We are generally moving to int64_t for both offset and bytes parameters
> on all io paths. Prepare bdrv_co_do_pwrite_zeroes() now.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/io.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 4796476835..c8c30e3699 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)
> @@ -1743,7 +1743,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)

Widens from 32- to 64-bit.  Callers (I'm looking at pre-series code, the 
further I get into your series, the more likely that intermediate 
changes may alter the analysis...):

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.  Still, the latent 
fix would be nice to mention in the commit message.

>   {
>       BlockDriver *drv = bs->drv;
>       QEMUIOVector qiov;
> @@ -1773,7 +1773,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;

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.

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

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

diff --git a/block/io.c b/block/io.c
index c8c30e3699..fe19e09034 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1854,7 +1854,7 @@ 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;
@@ -1906,7 +1906,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);
@@ -1948,14 +1948,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) {
@@ -1967,6 +1967,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);
@@ -2067,7 +2069,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 v2 06/17] block/io: support int64_t bytes in bdrv_aligned_pwritev()

[Eric Blake]

On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
> We are generally moving to int64_t for both offset and bytes parameters
> on all io paths. Prepare bdrv_aligned_pwritev() now (and convert the
> dependencies: bdrv_co_write_req_prepare() and
> bdrv_co_write_req_finish() to signed type bytes)
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/io.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index c8c30e3699..fe19e09034 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1854,7 +1854,7 @@ 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)
>   {

No change in size.  First, check usage within function:
     int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
Changes computation from uint64_t to int64_t.  This causes a borderline 
bug on images between INT64_MAX-511 and INT64_MAX (nbdkit can produce 
such images over NBD, although they are atypical on disk), where 
DIV_ROUND_UP() would give the right answer as uint64_t but a negative 
answer with int64_t.  As those images are not sector-aligned, maybe we 
don't need to care?
all other uses appear to be within asserts related to offset+bytes being 
positive, so that's what we should check for.

Callers:
bdrv_aligned_pwritev() - changed in this patch to 'int64_t', analyzed 
below [1]
bdrv_co_pdiscard() - already passes 'int64_t', also checks for 
offset+bytes overflow - safe
bdrv_co_copy_range_internal() - 'uint64_t', but already analyzed for 
3/17 how it was capped < 2M - safe
bdrv_co_truncate() - already passes 'int64_t', passes new_bytes computed 
by subtracting from a positive 'int64_t offset' - safe


[1] except I hit the end of my work day, so my analysis will have to 
continue tomorrow...


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

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

[Vladimir Sementsov-Ogievskiy]

30.04.2020 1:04, Eric Blake wrote:
> On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>> We are generally moving to int64_t for both offset and bytes parameters
>> on all io paths. Prepare bdrv_aligned_pwritev() now (and convert the
>> dependencies: bdrv_co_write_req_prepare() and
>> bdrv_co_write_req_finish() to signed type bytes)
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block/io.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/block/io.c b/block/io.c
>> index c8c30e3699..fe19e09034 100644
>> --- a/block/io.c
>> +++ b/block/io.c
>> @@ -1854,7 +1854,7 @@ 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)
>>   {
> 
> No change in size.  First, check usage within function:
>      int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
> Changes computation from uint64_t to int64_t.  This causes a borderline bug on images between INT64_MAX-511 and INT64_MAX (nbdkit can produce such images over NBD, although they are atypical on disk), where DIV_ROUND_UP() would give the right answer as uint64_t but a negative answer with int64_t.  As those images are not sector-aligned, maybe we don't need to care?
> all other uses appear to be within asserts related to offset+bytes being positive, so that's what we should check for.
> 
> Callers:
> bdrv_aligned_pwritev() - changed in this patch to 'int64_t', analyzed below [1]
> bdrv_co_pdiscard() - already passes 'int64_t', also checks for offset+bytes overflow - safe
> bdrv_co_copy_range_internal() - 'uint64_t', but already analyzed for 3/17 how it was capped < 2M - safe
> bdrv_co_truncate() - already passes 'int64_t', passes new_bytes computed by subtracting from a positive 'int64_t offset' - safe
> 
> 
> [1] except I hit the end of my work day, so my analysis will have to continue tomorrow...
> 

Thanks for reviewing!

I'm very sorry, I just need to say once again: the series should be rebased on "[PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections", as it is already mostly reviewed by Stefan. Seems, that your analysis will be still valid after it, although patches will change. I'll do it now to see, can I keep your r-b's.


-- 
Best regards,
Vladimir

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

[Vladimir Sementsov-Ogievskiy]

30.04.2020 8:25, Vladimir Sementsov-Ogievskiy wrote:
> 30.04.2020 1:04, Eric Blake wrote:
>> On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> We are generally moving to int64_t for both offset and bytes parameters
>>> on all io paths. Prepare bdrv_aligned_pwritev() now (and convert the
>>> dependencies: bdrv_co_write_req_prepare() and
>>> bdrv_co_write_req_finish() to signed type bytes)
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   block/io.c | 12 +++++++-----
>>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/block/io.c b/block/io.c
>>> index c8c30e3699..fe19e09034 100644
>>> --- a/block/io.c
>>> +++ b/block/io.c
>>> @@ -1854,7 +1854,7 @@ 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)
>>>   {
>>
>> No change in size.  First, check usage within function:
>>      int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
>> Changes computation from uint64_t to int64_t.  This causes a borderline bug on images between INT64_MAX-511 and INT64_MAX (nbdkit can produce such images over NBD, although they are atypical on disk), where DIV_ROUND_UP() would give the right answer as uint64_t but a negative answer with int64_t.  As those images are not sector-aligned, maybe we don't need to care?
>> all other uses appear to be within asserts related to offset+bytes being positive, so that's what we should check for.
>>
>> Callers:
>> bdrv_aligned_pwritev() - changed in this patch to 'int64_t', analyzed below [1]
>> bdrv_co_pdiscard() - already passes 'int64_t', also checks for offset+bytes overflow - safe
>> bdrv_co_copy_range_internal() - 'uint64_t', but already analyzed for 3/17 how it was capped < 2M - safe
>> bdrv_co_truncate() - already passes 'int64_t', passes new_bytes computed by subtracting from a positive 'int64_t offset' - safe
>>
>>
>> [1] except I hit the end of my work day, so my analysis will have to continue tomorrow...
>>
> 
> Thanks for reviewing!
> 
> I'm very sorry, I just need to say once again: the series should be rebased on "[PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections", as it is already mostly reviewed by Stefan. Seems, that your analysis will be still valid after it, although patches will change. I'll do it now to see, can I keep your r-b's.
> 

I mean "[PATCH v2 0/9] block/io: safer inc/dec in_flight sections" of course
https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg04559.html


-- 
Best regards,
Vladimir

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

[Vladimir Sementsov-Ogievskiy]

30.04.2020 8:30, Vladimir Sementsov-Ogievskiy wrote:
> 30.04.2020 8:25, Vladimir Sementsov-Ogievskiy wrote:
>> 30.04.2020 1:04, Eric Blake wrote:
>>> On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>>>> We are generally moving to int64_t for both offset and bytes parameters
>>>> on all io paths. Prepare bdrv_aligned_pwritev() now (and convert the
>>>> dependencies: bdrv_co_write_req_prepare() and
>>>> bdrv_co_write_req_finish() to signed type bytes)
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>> ---
>>>>   block/io.c | 12 +++++++-----
>>>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/block/io.c b/block/io.c
>>>> index c8c30e3699..fe19e09034 100644
>>>> --- a/block/io.c
>>>> +++ b/block/io.c
>>>> @@ -1854,7 +1854,7 @@ 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)
>>>>   {
>>>
>>> No change in size.  First, check usage within function:
>>>      int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
>>> Changes computation from uint64_t to int64_t.  This causes a borderline bug on images between INT64_MAX-511 and INT64_MAX (nbdkit can produce such images over NBD, although they are atypical on disk), where DIV_ROUND_UP() would give the right answer as uint64_t but a negative answer with int64_t.  As those images are not sector-aligned, maybe we don't need to care?
>>> all other uses appear to be within asserts related to offset+bytes being positive, so that's what we should check for.
>>>
>>> Callers:
>>> bdrv_aligned_pwritev() - changed in this patch to 'int64_t', analyzed below [1]
>>> bdrv_co_pdiscard() - already passes 'int64_t', also checks for offset+bytes overflow - safe
>>> bdrv_co_copy_range_internal() - 'uint64_t', but already analyzed for 3/17 how it was capped < 2M - safe
>>> bdrv_co_truncate() - already passes 'int64_t', passes new_bytes computed by subtracting from a positive 'int64_t offset' - safe
>>>
>>>
>>> [1] except I hit the end of my work day, so my analysis will have to continue tomorrow...
>>>
>>
>> Thanks for reviewing!
>>
>> I'm very sorry, I just need to say once again: the series should be rebased on "[PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections", as it is already mostly reviewed by Stefan. Seems, that your analysis will be still valid after it, although patches will change. I'll do it now to see, can I keep your r-b's.
>>
> 
> I mean "[PATCH v2 0/9] block/io: safer inc/dec in_flight sections" of course
> https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg04559.html
> 
> 

Cool! Exactly up to this patch (inclusive) it rebases without conflicts. And the next patch (and may be further) are conflicting. I'll finish rebasing and resend.

-- 
Best regards,
Vladimir

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

[Vladimir Sementsov-Ogievskiy]

30.04.2020 1:04, Eric Blake wrote:
> On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>> We are generally moving to int64_t for both offset and bytes parameters
>> on all io paths. Prepare bdrv_aligned_pwritev() now (and convert the
>> dependencies: bdrv_co_write_req_prepare() and
>> bdrv_co_write_req_finish() to signed type bytes)
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block/io.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/block/io.c b/block/io.c
>> index c8c30e3699..fe19e09034 100644
>> --- a/block/io.c
>> +++ b/block/io.c
>> @@ -1854,7 +1854,7 @@ 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)
>>   {
> 
> No change in size.  First, check usage within function:
>      int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);

this variable used only for assertion, it's simple to refactor to avoid the problem.

> Changes computation from uint64_t to int64_t.  This causes a borderline bug on images between INT64_MAX-511 and INT64_MAX (nbdkit can produce such images over NBD, although they are atypical on disk), where DIV_ROUND_UP() would give the right answer as uint64_t but a negative answer with int64_t.  As those images are not sector-aligned, maybe we don't need to care?
> all other uses appear to be within asserts related to offset+bytes being positive, so that's what we should check for.
> 
> Callers:
> bdrv_aligned_pwritev() - changed in this patch to 'int64_t', analyzed below [1]
> bdrv_co_pdiscard() - already passes 'int64_t', also checks for offset+bytes overflow - safe
> bdrv_co_copy_range_internal() - 'uint64_t', but already analyzed for 3/17 how it was capped < 2M - safe
> bdrv_co_truncate() - already passes 'int64_t', passes new_bytes computed by subtracting from a positive 'int64_t offset' - safe
> 
> 
> [1] except I hit the end of my work day, so my analysis will have to continue tomorrow...
> 
> 


-- 
Best regards,
Vladimir

[PATCH v2 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. Prepare bdrv_co_do_copy_on_readv() now.

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 fe19e09034..ee38c9afb4 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1268,7 +1268,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;
@@ -1283,11 +1283,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

[PATCH v2 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. 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().

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 ee38c9afb4..70b51dbd0f 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1430,16 +1430,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);
@@ -1495,7 +1496,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));
@@ -1596,7 +1597,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

[PATCH v2 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. Prepare bdrv_co_preadv_part() and
bdrv_co_pwritev_part() and their remaining dependencies now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block_int.h |  4 ++--
 block/io.c                | 12 ++++++------
 block/trace-events        |  4 ++--
 3 files changed, 10 insertions(+), 10 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 70b51dbd0f..784eaf02f2 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1668,7 +1668,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)) {
@@ -1694,7 +1694,7 @@ int coroutine_fn bdrv_co_preadv(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)
 {
@@ -1703,7 +1703,7 @@ int coroutine_fn bdrv_co_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) {
@@ -2033,7 +2033,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)
 {
@@ -2107,7 +2107,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
 }
 
 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)
 {
     BlockDriverState *bs = child->bs;
@@ -2116,7 +2116,7 @@ int coroutine_fn bdrv_co_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;
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

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

[Vladimir Sementsov-Ogievskiy]

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

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

diff --git a/include/block/block.h b/include/block/block.h
index b05995fe9c..03abf30d1d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -318,14 +318,14 @@ 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 +333,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 784eaf02f2..02632eaf59 100644
--- a/block/io.c
+++ b/block/io.c
@@ -946,7 +946,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);
 
@@ -1008,7 +1008,7 @@ int bdrv_preadv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov)
 }
 
 /* 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);
 
@@ -1037,7 +1037,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);
 
@@ -1055,11 +1056,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;
     }
@@ -1687,7 +1688,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);
@@ -2100,7 +2101,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);
@@ -2177,7 +2178,7 @@ out:
 }
 
 int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset,
-                                       int bytes, BdrvRequestFlags flags)
+                                       int64_t bytes, BdrvRequestFlags flags)
 {
     trace_bdrv_co_pwrite_zeroes(child->bs, offset, bytes, flags);
 
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 v2 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. Convert copy_range parameters which are already 64bit
to signed type.

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 03abf30d1d..2eea27bc99 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -731,8 +731,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 02632eaf59..7c979eacd8 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3209,8 +3209,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)
 {
@@ -3288,9 +3288,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)
 {
@@ -3304,9 +3304,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)
 {
@@ -3316,9 +3316,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 v2 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. Now bdrv layer is converted, convert blk layer too.

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

[PATCH v2 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. Convert driver read handlers parameters which are
already 64bit to signed type.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block_int.h |  6 +++---
 block/backup-top.c        |  2 +-
 block/blkdebug.c          |  2 +-
 block/blklogwrites.c      |  2 +-
 block/blkreplay.c         |  2 +-
 block/blkverify.c         |  2 +-
 block/bochs.c             |  2 +-
 block/cloop.c             |  2 +-
 block/commit.c            |  2 +-
 block/copy-on-read.c      |  2 +-
 block/crypto.c            |  2 +-
 block/curl.c              |  2 +-
 block/dmg.c               |  2 +-
 block/file-posix.c        |  4 ++--
 block/filter-compress.c   |  2 +-
 block/mirror.c            |  2 +-
 block/nbd.c               |  4 ++--
 block/nfs.c               |  4 ++--
 block/null.c              |  4 ++--
 block/nvme.c              |  2 +-
 block/qcow.c              |  4 ++--
 block/qcow2.c             |  2 +-
 block/quorum.c            |  4 ++--
 block/raw-format.c        | 18 +++++++++---------
 block/rbd.c               |  2 +-
 block/throttle.c          |  2 +-
 block/vdi.c               |  2 +-
 block/vmdk.c              |  2 +-
 block/vpc.c               |  2 +-
 block/vvfat.c             |  2 +-
 tests/test-bdrv-drain.c   |  8 ++++----
 31 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 28aea2bcfd..1588df02c2 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -180,7 +180,7 @@ struct BlockDriver {
 
     /* aio */
     BlockAIOCB *(*bdrv_aio_preadv)(BlockDriverState *bs,
-        uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov, int flags,
         BlockCompletionFunc *cb, void *opaque);
     BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs,
         uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
@@ -210,9 +210,9 @@ 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, int flags);
     int coroutine_fn (*bdrv_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);
     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..49fd8763cc 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -42,7 +42,7 @@ typedef struct BDRVBackupTopState {
 } BDRVBackupTopState;
 
 static coroutine_fn int backup_top_co_preadv(
-        BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+        BlockDriverState *bs, int64_t offset, int64_t bytes,
         QEMUIOVector *qiov, int flags)
 {
     return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
diff --git a/block/blkdebug.c b/block/blkdebug.c
index af44aa973f..b61275f229 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -619,7 +619,7 @@ 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,
+blkdebug_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
                    QEMUIOVector *qiov, int flags)
 {
     int err;
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index 04d8b33607..6e5bd09993 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -307,7 +307,7 @@ 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,
+blk_log_writes_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
                          QEMUIOVector *qiov, int flags)
 {
     return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
diff --git a/block/blkreplay.c b/block/blkreplay.c
index c96ac8f4bc..70bc1158e1 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, int 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..f456c99814 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -224,7 +224,7 @@ 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,
+blkverify_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
                     QEMUIOVector *qiov, int flags)
 {
     BlkverifyRequest r;
diff --git a/block/bochs.c b/block/bochs.c
index 32bb83b268..08ae3793e4 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -238,7 +238,7 @@ 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,
+bochs_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
                 QEMUIOVector *qiov, int flags)
 {
     BDRVBochsState *s = bs->opaque;
diff --git a/block/cloop.c b/block/cloop.c
index 4de94876d4..a5db202678 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -245,7 +245,7 @@ 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,
+cloop_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
                 QEMUIOVector *qiov, int flags)
 {
     BDRVCloopState *s = bs->opaque;
diff --git a/block/commit.c b/block/commit.c
index 8e672799af..951e4f623a 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, int 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..0f6b5398b1 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -74,7 +74,7 @@ static int64_t cor_getlength(BlockDriverState *bs)
 
 
 static int coroutine_fn cor_co_preadv(BlockDriverState *bs,
-                                      uint64_t offset, uint64_t bytes,
+                                      int64_t offset, int64_t bytes,
                                       QEMUIOVector *qiov, int flags)
 {
     return bdrv_co_preadv(bs->file, offset, bytes, qiov,
diff --git a/block/crypto.c b/block/crypto.c
index d577f89659..1117b1fafe 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -335,7 +335,7 @@ 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,
+block_crypto_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
                        QEMUIOVector *qiov, int flags)
 {
     BlockCrypto *crypto = bs->opaque;
diff --git a/block/curl.c b/block/curl.c
index 6e325901dc..305a7a5e95 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -922,7 +922,7 @@ 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, int flags)
 {
     CURLAIOCB acb = {
         .co = qemu_coroutine_self(),
diff --git a/block/dmg.c b/block/dmg.c
index 4a045f2b3e..c378b4f555 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -689,7 +689,7 @@ 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,
+dmg_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
               QEMUIOVector *qiov, int flags)
 {
     BDRVDMGState *s = bs->opaque;
diff --git a/block/file-posix.c b/block/file-posix.c
index 7e19bbff5f..87415a0a3c 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1941,8 +1941,8 @@ 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,
+static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
+                                      int64_t bytes, QEMUIOVector *qiov,
                                       int flags)
 {
     return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_READ);
diff --git a/block/filter-compress.c b/block/filter-compress.c
index 82c315b298..496bbd9e4b 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -62,7 +62,7 @@ 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)
diff --git a/block/mirror.c b/block/mirror.c
index c26fd9260d..51ff4ffd43 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, int flags)
 {
     return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
 }
diff --git a/block/nbd.c b/block/nbd.c
index 2160859f64..d878caa7ad 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1160,8 +1160,8 @@ 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, int flags)
 {
     int ret, request_ret;
     Error *local_err = NULL;
diff --git a/block/nfs.c b/block/nfs.c
index cc2413d5ab..7bcb068715 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -263,8 +263,8 @@ 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,
+static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, int64_t offset,
+                                      int64_t bytes, QEMUIOVector *iov,
                                       int flags)
 {
     NFSClient *client = bs->opaque;
diff --git a/block/null.c b/block/null.c
index 15e1d56746..483e1eebb2 100644
--- a/block/null.c
+++ b/block/null.c
@@ -116,7 +116,7 @@ static coroutine_fn int null_co_common(BlockDriverState *bs)
 }
 
 static coroutine_fn int null_co_preadv(BlockDriverState *bs,
-                                       uint64_t offset, uint64_t bytes,
+                                       int64_t offset, int64_t bytes,
                                        QEMUIOVector *qiov, int flags)
 {
     BDRVNullState *s = bs->opaque;
@@ -187,7 +187,7 @@ static inline BlockAIOCB *null_aio_common(BlockDriverState *bs,
 }
 
 static BlockAIOCB *null_aio_preadv(BlockDriverState *bs,
-                                   uint64_t offset, uint64_t bytes,
+                                   int64_t offset, int64_t bytes,
                                    QEMUIOVector *qiov, int flags,
                                    BlockCompletionFunc *cb,
                                    void *opaque)
diff --git a/block/nvme.c b/block/nvme.c
index 7b7c0cc5d6..254943672a 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1062,7 +1062,7 @@ 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,
+                                       int64_t offset, int64_t bytes,
                                        QEMUIOVector *qiov, int flags)
 {
     return nvme_co_prw(bs, offset, bytes, qiov, false, flags);
diff --git a/block/qcow.c b/block/qcow.c
index 8973e4e565..b3376465f4 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -628,8 +628,8 @@ 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,
+static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, int64_t offset,
+                                       int64_t bytes, QEMUIOVector *qiov,
                                        int flags)
 {
     BDRVQcowState *s = bs->opaque;
diff --git a/block/qcow2.c b/block/qcow2.c
index b524b0c53f..45ed81304a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2202,7 +2202,7 @@ 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)
 {
diff --git a/block/quorum.c b/block/quorum.c
index 6d7a56bd93..b51d6bdc41 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, int 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..edce5f66c5 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,8 +191,8 @@ 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,
+static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
+                                      int64_t bytes, QEMUIOVector *qiov,
                                       int 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..bc96e69fe9 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1034,7 +1034,7 @@ failed:
 }
 
 static BlockAIOCB *qemu_rbd_aio_preadv(BlockDriverState *bs,
-                                       uint64_t offset, uint64_t bytes,
+                                       int64_t offset, int64_t bytes,
                                        QEMUIOVector *qiov, int flags,
                                        BlockCompletionFunc *cb,
                                        void *opaque)
diff --git a/block/throttle.c b/block/throttle.c
index 71f4bb0ad1..81ff98ac30 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -114,7 +114,7 @@ static int64_t throttle_getlength(BlockDriverState *bs)
 }
 
 static int coroutine_fn throttle_co_preadv(BlockDriverState *bs,
-                                           uint64_t offset, uint64_t bytes,
+                                           int64_t offset, int64_t bytes,
                                            QEMUIOVector *qiov, int flags)
 {
 
diff --git a/block/vdi.c b/block/vdi.c
index e1a11f2aa0..0b395dc3cc 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -547,7 +547,7 @@ 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,
+vdi_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
               QEMUIOVector *qiov, int flags)
 {
     BDRVVdiState *s = bs->opaque;
diff --git a/block/vmdk.c b/block/vmdk.c
index 218d9c9800..acad4118e4 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1868,7 +1868,7 @@ 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,
+vmdk_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
                QEMUIOVector *qiov, int flags)
 {
     BDRVVmdkState *s = bs->opaque;
diff --git a/block/vpc.c b/block/vpc.c
index d8141b52da..cc9c9b2297 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -611,7 +611,7 @@ static int vpc_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
 }
 
 static int coroutine_fn
-vpc_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+vpc_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
               QEMUIOVector *qiov, int flags)
 {
     BDRVVPCState *s = bs->opaque;
diff --git a/block/vvfat.c b/block/vvfat.c
index ab800c4887..b177ee7672 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1538,7 +1538,7 @@ 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,
+vvfat_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
                 QEMUIOVector *qiov, int flags)
 {
     int ret;
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
index fa0e6a648b..c03b021777 100644
--- a/tests/test-bdrv-drain.c
+++ b/tests/test-bdrv-drain.c
@@ -65,7 +65,7 @@ static void co_reenter_bh(void *opaque)
 }
 
 static int coroutine_fn bdrv_test_co_preadv(BlockDriverState *bs,
-                                            uint64_t offset, uint64_t bytes,
+                                            int64_t offset, int64_t bytes,
                                             QEMUIOVector *qiov, int flags)
 {
     BDRVTestState *s = bs->opaque;
@@ -1120,7 +1120,7 @@ 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,
+                                                int64_t offset, int64_t bytes,
                                                 QEMUIOVector *qiov, int flags)
 {
     BDRVTestTopState *tts = bs->opaque;
@@ -1862,8 +1862,8 @@ 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)
 {
-- 
2.21.0

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

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/block/block_int.h | 10 +++++-----
 block/backup-top.c        |  3 +--
 block/blkdebug.c          |  2 +-
 block/blklogwrites.c      |  2 +-
 block/blkreplay.c         |  2 +-
 block/blkverify.c         |  2 +-
 block/copy-on-read.c      |  2 +-
 block/crypto.c            |  2 +-
 block/file-posix.c        |  4 ++--
 block/filter-compress.c   |  4 ++--
 block/mirror.c            |  2 +-
 block/nbd.c               |  4 ++--
 block/nfs.c               |  4 ++--
 block/null.c              |  4 ++--
 block/nvme.c              |  2 +-
 block/qcow.c              |  8 ++++----
 block/qcow2.c             |  4 ++--
 block/quorum.c            |  4 ++--
 block/raw-format.c        |  8 ++++----
 block/rbd.c               |  2 +-
 block/throttle.c          |  2 +-
 block/vdi.c               |  2 +-
 block/vmdk.c              |  6 +++---
 block/vpc.c               |  2 +-
 block/vvfat.c             |  4 ++--
 block/trace-events        |  2 +-
 26 files changed, 46 insertions(+), 47 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1588df02c2..0da54cdf48 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -183,7 +183,7 @@ struct BlockDriver {
         int64_t offset, int64_t bytes, QEMUIOVector *qiov, int flags,
         BlockCompletionFunc *cb, void *opaque);
     BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs,
-        uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov, int flags,
         BlockCompletionFunc *cb, void *opaque);
     BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
         BlockCompletionFunc *cb, void *opaque);
@@ -232,9 +232,9 @@ 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, int flags);
     int coroutine_fn (*bdrv_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);
 
     /*
@@ -364,9 +364,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,
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov,
         size_t qiov_offset);
 
     int (*bdrv_snapshot_create)(BlockDriverState *bs,
diff --git a/block/backup-top.c b/block/backup-top.c
index 49fd8763cc..4190d465d6 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -87,8 +87,7 @@ 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,
+                                              int64_t offset, int64_t bytes,
                                               QEMUIOVector *qiov, int flags)
 {
     int ret = backup_top_cbw(bs, offset, bytes, flags);
diff --git a/block/blkdebug.c b/block/blkdebug.c
index b61275f229..b4d0966982 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -640,7 +640,7 @@ 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,
+blkdebug_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
                     QEMUIOVector *qiov, int flags)
 {
     int err;
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index 6e5bd09993..890a61dfba 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -466,7 +466,7 @@ 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,
+blk_log_writes_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
                           QEMUIOVector *qiov, int flags)
 {
     return blk_log_writes_co_log(bs, offset, bytes, qiov, flags,
diff --git a/block/blkreplay.c b/block/blkreplay.c
index 70bc1158e1..d93383a88f 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, int 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 f456c99814..a3c447fc68 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -253,7 +253,7 @@ 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,
+blkverify_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
                      QEMUIOVector *qiov, int flags)
 {
     BlkverifyRequest r;
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 0f6b5398b1..fc3186bacb 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -83,7 +83,7 @@ static int coroutine_fn cor_co_preadv(BlockDriverState *bs,
 
 
 static int coroutine_fn cor_co_pwritev(BlockDriverState *bs,
-                                       uint64_t offset, uint64_t bytes,
+                                       int64_t offset, int64_t bytes,
                                        QEMUIOVector *qiov, int flags)
 {
 
diff --git a/block/crypto.c b/block/crypto.c
index 1117b1fafe..ecb5697a4b 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -398,7 +398,7 @@ 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,
+block_crypto_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
                         QEMUIOVector *qiov, int flags)
 {
     BlockCrypto *crypto = bs->opaque;
diff --git a/block/file-posix.c b/block/file-posix.c
index 87415a0a3c..688f2fa54c 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1948,8 +1948,8 @@ 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,
+static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset,
+                                       int64_t bytes, QEMUIOVector *qiov,
                                        int flags)
 {
     assert(flags == 0);
diff --git a/block/filter-compress.c b/block/filter-compress.c
index 496bbd9e4b..f2400fea37 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -73,8 +73,8 @@ 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)
 {
diff --git a/block/mirror.c b/block/mirror.c
index 51ff4ffd43..687a91e654 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, int flags)
 {
     MirrorBDSOpaque *s = bs->opaque;
     QEMUIOVector bounce_qiov;
diff --git a/block/nbd.c b/block/nbd.c
index d878caa7ad..54bce3911e 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1218,8 +1218,8 @@ 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, int flags)
 {
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
     NBDRequest request = {
diff --git a/block/nfs.c b/block/nfs.c
index 7bcb068715..d59d8e7d32 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -298,8 +298,8 @@ 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,
+static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, int64_t offset,
+                                       int64_t bytes, QEMUIOVector *iov,
                                        int flags)
 {
     NFSClient *client = bs->opaque;
diff --git a/block/null.c b/block/null.c
index 483e1eebb2..cac37e7ba6 100644
--- a/block/null.c
+++ b/block/null.c
@@ -129,7 +129,7 @@ static coroutine_fn int null_co_preadv(BlockDriverState *bs,
 }
 
 static coroutine_fn int null_co_pwritev(BlockDriverState *bs,
-                                        uint64_t offset, uint64_t bytes,
+                                        int64_t offset, int64_t bytes,
                                         QEMUIOVector *qiov, int flags)
 {
     return null_co_common(bs);
@@ -202,7 +202,7 @@ static BlockAIOCB *null_aio_preadv(BlockDriverState *bs,
 }
 
 static BlockAIOCB *null_aio_pwritev(BlockDriverState *bs,
-                                    uint64_t offset, uint64_t bytes,
+                                    int64_t offset, int64_t bytes,
                                     QEMUIOVector *qiov, int flags,
                                     BlockCompletionFunc *cb,
                                     void *opaque)
diff --git a/block/nvme.c b/block/nvme.c
index 254943672a..db7fffe94f 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1069,7 +1069,7 @@ static coroutine_fn int nvme_co_preadv(BlockDriverState *bs,
 }
 
 static coroutine_fn int nvme_co_pwritev(BlockDriverState *bs,
-                                        uint64_t offset, uint64_t bytes,
+                                        int64_t offset, int64_t bytes,
                                         QEMUIOVector *qiov, int flags)
 {
     return nvme_co_prw(bs, offset, bytes, qiov, true, flags);
diff --git a/block/qcow.c b/block/qcow.c
index b3376465f4..9583e8e8b2 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -725,8 +725,8 @@ 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,
+static coroutine_fn int qcow_co_pwritev(BlockDriverState *bs, int64_t offset,
+                                        int64_t bytes, QEMUIOVector *qiov,
                                         int flags)
 {
     BDRVQcowState *s = bs->opaque;
@@ -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 45ed81304a..ed5e456b09 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2475,7 +2475,7 @@ 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,
+        BlockDriverState *bs, int64_t offset, int64_t bytes,
         QEMUIOVector *qiov, size_t qiov_offset, int flags)
 {
     BDRVQcow2State *s = bs->opaque;
@@ -4328,7 +4328,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 b51d6bdc41..4164c3fbf8 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -706,8 +706,8 @@ 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, int 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 edce5f66c5..d5ff7012b7 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -206,8 +206,8 @@ 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,
+static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset,
+                                       int64_t bytes, QEMUIOVector *qiov,
                                        int flags)
 {
     void *buf = NULL;
@@ -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 bc96e69fe9..412df8cdb7 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1044,7 +1044,7 @@ static BlockAIOCB *qemu_rbd_aio_preadv(BlockDriverState *bs,
 }
 
 static BlockAIOCB *qemu_rbd_aio_pwritev(BlockDriverState *bs,
-                                        uint64_t offset, uint64_t bytes,
+                                        int64_t offset, int64_t bytes,
                                         QEMUIOVector *qiov, int flags,
                                         BlockCompletionFunc *cb,
                                         void *opaque)
diff --git a/block/throttle.c b/block/throttle.c
index 81ff98ac30..28e73073e7 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -125,7 +125,7 @@ static int coroutine_fn throttle_co_preadv(BlockDriverState *bs,
 }
 
 static int coroutine_fn throttle_co_pwritev(BlockDriverState *bs,
-                                            uint64_t offset, uint64_t bytes,
+                                            int64_t offset, int64_t bytes,
                                             QEMUIOVector *qiov, int flags)
 {
     ThrottleGroupMember *tgm = bs->opaque;
diff --git a/block/vdi.c b/block/vdi.c
index 0b395dc3cc..07ad195239 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -603,7 +603,7 @@ 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,
+vdi_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
                QEMUIOVector *qiov, int flags)
 {
     BDRVVdiState *s = bs->opaque;
diff --git a/block/vmdk.c b/block/vmdk.c
index acad4118e4..a88e9c9ba4 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2048,7 +2048,7 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
 }
 
 static int coroutine_fn
-vmdk_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+vmdk_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
                 QEMUIOVector *qiov, int flags)
 {
     int ret;
@@ -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 cc9c9b2297..90a402eb41 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -662,7 +662,7 @@ fail:
 }
 
 static int coroutine_fn
-vpc_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+vpc_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
                QEMUIOVector *qiov, int flags)
 {
     BDRVVPCState *s = bs->opaque;
diff --git a/block/vvfat.c b/block/vvfat.c
index b177ee7672..97e6b0e2a5 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3073,7 +3073,7 @@ DLOG(checkpoint());
 }
 
 static int coroutine_fn
-vvfat_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+vvfat_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
                  QEMUIOVector *qiov, int flags)
 {
     int ret;
@@ -3111,7 +3111,7 @@ 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,
+write_target_commit(BlockDriverState *bs, int64_t offset, int64_t bytes,
                     QEMUIOVector *qiov, int flags)
 {
     int ret;
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 v2 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. Convert driver copy_range handlers parameters which
are already 64bit to signed type.

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 0da54cdf48..ea8fca5e28 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -257,10 +257,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);
 
@@ -274,10 +274,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 688f2fa54c..76da620135 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 ed5e456b09..945554b25c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3809,9 +3809,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;
@@ -3890,9 +3890,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 d5ff7012b7..2537755f84 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 v2 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. 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.

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 ea8fca5e28..fe446f11eb 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -244,7 +244,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 4190d465d6..20c943280f 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 b4d0966982..521b7b1fe2 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 890a61dfba..ccfb1100e4 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 d93383a88f..74ea935593 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 fc3186bacb..59272b5faa 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -92,7 +92,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 76da620135..bfddfbb9b3 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 f2400fea37..b22e57c5f2 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -84,7 +84,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 687a91e654..2d8fe6008a 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 54bce3911e..fc30514e10 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1243,7 +1243,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 = {
@@ -1252,6 +1252,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 db7fffe94f..ef27c7eb3c 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1103,10 +1103,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];
@@ -1156,6 +1156,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 945554b25c..905d22e2c8 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3726,7 +3726,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;
@@ -3778,6 +3778,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 2537755f84..de2fef9edc 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 28e73073e7..c1503f133f 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -135,7 +135,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 a88e9c9ba4..2898c10fa8 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 v2 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. 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.

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 fe446f11eb..814837603d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -246,7 +246,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 20c943280f..7ff0b0aa7e 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 521b7b1fe2..3c84cc3f84 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 ccfb1100e4..172afbf199 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 74ea935593..f34ea6b249 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 59272b5faa..c05553eba7 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -100,7 +100,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 bfddfbb9b3..b938e1ee83 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 b22e57c5f2..646abad5b0 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -92,7 +92,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 2d8fe6008a..d284b5296e 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 fc30514e10..50874351de 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1293,7 +1293,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 = {
@@ -1302,6 +1302,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 ef27c7eb3c..939d5d36dd 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1165,9 +1165,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];
@@ -1244,6 +1244,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 905d22e2c8..0f6458c1cf 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3786,7 +3786,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 de2fef9edc..2f25bed301 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 c1503f133f..c0a8521dca 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -145,7 +145,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 v2 00/17] 64bit block-layer

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200427082325.10414-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-authz-listfile.o -MF tests/test-authz-listfile.d -g   -c -o tests/test-authz-listfile.o /tmp/qemu-test/src/tests/test-authz-listfile.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-io-task.o -MF tests/test-io-task.d -g   -c -o tests/test-io-task.o /tmp/qemu-test/src/tests/test-io-task.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-io-channel-socket.o -MF tests/test-io-channel-socket.d -g   -c -o tests/test-io-channel-socket.o /tmp/qemu-test/src/tests/test-io-channel-socket.c
/tmp/qemu-test/src/tests/test-block-iothread.c:68:31: error: incompatible pointer types initializing 'int (*)(BlockDriverState *, int64_t, int64_t, QEMUIOVector *, int)' (aka 'int (*)(struct BlockDriverState *, long, long, struct QEMUIOVector *, int)') with an expression of type 'int (BlockDriverState *, uint64_t, uint64_t, QEMUIOVector *, int)' (aka 'int (struct BlockDriverState *, unsigned long, unsigned long, struct QEMUIOVector *, int)') [-Werror,-Wincompatible-pointer-types]
    .bdrv_co_preadv         = bdrv_test_co_prwv,
                              ^~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/tests/test-block-iothread.c:69:31: error: incompatible pointer types initializing 'int (*)(BlockDriverState *, int64_t, int64_t, QEMUIOVector *, int)' (aka 'int (*)(struct BlockDriverState *, long, long, struct QEMUIOVector *, int)') with an expression of type 'int (BlockDriverState *, uint64_t, uint64_t, QEMUIOVector *, int)' (aka 'int (struct BlockDriverState *, unsigned long, unsigned long, struct QEMUIOVector *, int)') [-Werror,-Wincompatible-pointer-types]
    .bdrv_co_pwritev        = bdrv_test_co_prwv,
                              ^~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/tests/test-block-iothread.c:70: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,
                              ^~~~~~~~~~~~~~~~~~~~~
3 errors generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: tests/test-block-iothread.o] Error 1
make: *** Waiting for unfinished jobs....
make[1]: Leaving directory '/tmp/qemu-test/build/x86_64-softmmu'
Traceback (most recent call last):
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=d2d235a1f4134804a67563d47cf752e4', '-u', '1003', '--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/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-m9k8_1ij/src/docker-src.2020-04-27-05.25.43.843:/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=d2d235a1f4134804a67563d47cf752e4
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-m9k8_1ij/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    4m17.946s
user    0m8.042s


The full log is available at
http://patchew.org/logs/20200427082325.10414-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 v2 00/17] 64bit block-layer

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200427082325.10414-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-qht.o
  CC      tests/test-qht-par.o
  CC      tests/qht-bench.o
/tmp/qemu-test/src/tests/test-block-iothread.c:68:5: error: initialization from incompatible pointer type [-Werror]
     .bdrv_co_preadv         = bdrv_test_co_prwv,
     ^
/tmp/qemu-test/src/tests/test-block-iothread.c:68:5: error: (near initialization for 'bdrv_test.bdrv_co_preadv') [-Werror]
/tmp/qemu-test/src/tests/test-block-iothread.c:69:5: error: initialization from incompatible pointer type [-Werror]
     .bdrv_co_pwritev        = bdrv_test_co_prwv,
     ^
/tmp/qemu-test/src/tests/test-block-iothread.c:69:5: error: (near initialization for 'bdrv_test.bdrv_co_pwritev') [-Werror]
/tmp/qemu-test/src/tests/test-block-iothread.c:70:5: error: initialization from incompatible pointer type [-Werror]
     .bdrv_co_pdiscard       = bdrv_test_co_pdiscard,
     ^
/tmp/qemu-test/src/tests/test-block-iothread.c:70:5: error: (near initialization for 'bdrv_test.bdrv_co_pdiscard') [-Werror]
cc1: all warnings being treated as errors
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=957479540fc74221b559a98e20734189', '-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-d_zj0hid/src/docker-src.2020-04-27-05.55.36.14128:/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=957479540fc74221b559a98e20734189
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-d_zj0hid/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    7m9.461s
user    0m9.365s


The full log is available at
http://patchew.org/logs/20200427082325.10414-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 v2 00/17] 64bit block-layer

[no-reply]

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



Hi,

This series failed the docker-mingw@fedora 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-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      block/sheepdog.o
  CC      block/accounting.o
  CC      block/dirty-bitmap.o
/tmp/qemu-test/src/block/file-win32.c:643:27: error: initialization of 'BlockAIOCB * (*)(BlockDriverState *, int64_t,  int64_t,  QEMUIOVector *, int,  void (*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct BlockDriverState *, long long int,  long long int,  struct QEMUIOVector *, int,  void (*)(void *, int), void *)'} from incompatible pointer type 'BlockAIOCB * (*)(BlockDriverState *, uint64_t,  uint64_t,  QEMUIOVector *, int,  void (*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct BlockDriverState *, long long unsigned int,  long long unsigned int,  struct QEMUIOVector *, int,  void (*)(void *, int), void *)'} [-Werror=incompatible-pointer-types]
     .bdrv_aio_preadv    = raw_aio_preadv,
                           ^~~~~~~~~~~~~~
/tmp/qemu-test/src/block/file-win32.c:643:27: note: (near initialization for 'bdrv_file.bdrv_aio_preadv')
/tmp/qemu-test/src/block/file-win32.c:644:27: error: initialization of 'BlockAIOCB * (*)(BlockDriverState *, int64_t,  int64_t,  QEMUIOVector *, int,  void (*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct BlockDriverState *, long long int,  long long int,  struct QEMUIOVector *, int,  void (*)(void *, int), void *)'} from incompatible pointer type 'BlockAIOCB * (*)(BlockDriverState *, uint64_t,  uint64_t,  QEMUIOVector *, int,  void (*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct BlockDriverState *, long long unsigned int,  long long unsigned int,  struct QEMUIOVector *, int,  void (*)(void *, int), void *)'} [-Werror=incompatible-pointer-types]
     .bdrv_aio_pwritev   = raw_aio_pwritev,
                           ^~~~~~~~~~~~~~~
/tmp/qemu-test/src/block/file-win32.c:644:27: note: (near initialization for 'bdrv_file.bdrv_aio_pwritev')
/tmp/qemu-test/src/block/file-win32.c:812:27: error: initialization of 'BlockAIOCB * (*)(BlockDriverState *, int64_t,  int64_t,  QEMUIOVector *, int,  void (*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct BlockDriverState *, long long int,  long long int,  struct QEMUIOVector *, int,  void (*)(void *, int), void *)'} from incompatible pointer type 'BlockAIOCB * (*)(BlockDriverState *, uint64_t,  uint64_t,  QEMUIOVector *, int,  void (*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct BlockDriverState *, long long unsigned int,  long long unsigned int,  struct QEMUIOVector *, int,  void (*)(void *, int), void *)'} [-Werror=incompatible-pointer-types]
     .bdrv_aio_preadv    = raw_aio_preadv,
                           ^~~~~~~~~~~~~~
/tmp/qemu-test/src/block/file-win32.c:812:27: note: (near initialization for 'bdrv_host_device.bdrv_aio_preadv')
/tmp/qemu-test/src/block/file-win32.c:813:27: error: initialization of 'BlockAIOCB * (*)(BlockDriverState *, int64_t,  int64_t,  QEMUIOVector *, int,  void (*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct BlockDriverState *, long long int,  long long int,  struct QEMUIOVector *, int,  void (*)(void *, int), void *)'} from incompatible pointer type 'BlockAIOCB * (*)(BlockDriverState *, uint64_t,  uint64_t,  QEMUIOVector *, int,  void (*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct BlockDriverState *, long long unsigned int,  long long unsigned int,  struct QEMUIOVector *, int,  void (*)(void *, int), void *)'} [-Werror=incompatible-pointer-types]
     .bdrv_aio_pwritev   = raw_aio_pwritev,
                           ^~~~~~~~~~~~~~~
/tmp/qemu-test/src/block/file-win32.c:813:27: note: (near initialization for 'bdrv_host_device.bdrv_aio_pwritev')
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: block/file-win32.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=e3fcc585a42b4bfa8acd11130f8d165e', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-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-4hyn5l20/src/docker-src.2020-04-27-06.04.28.26691:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=e3fcc585a42b4bfa8acd11130f8d165e
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-4hyn5l20/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    4m14.193s
user    0m9.376s


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

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

[Vladimir Sementsov-Ogievskiy]

Hmm. I definitely should rebase it onto "[PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections"...


27.04.2020 11:23, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> v1 was "[RFC 0/3] 64bit block-layer part I", please refer to initial
> cover-letter
>   https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg08723.html
> for motivation.
> 
> v2:
> patch 02 is unchanged, add Stefan's r-b. Everything other is changed a
> lot. What's new:
> 
> - conversion of block/io.c is now done step-by-step, to make careful
>    review possible as well as future bisecting
> 
> - converting of driver handlers split by io type
> 
> - convert write_zeroes and discard (so the series is not called "part I"
>    any more). I decided to convert most of things alltogether, leaving
>    simple wrappers only in unobvious places. Still, if you consider it
>    risky, I can refactor it to use only wrappers as a first patch and
>    then update driver-by-driver, but it would be lot more patches, I'm
>    not sure it worth doing.
> 
> 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           |  16 ++---
>   include/block/block_int.h       |  56 ++++++++--------
>   include/block/throttle-groups.h |   2 +-
>   include/sysemu/block-backend.h  |  26 ++++----
>   block/backup-top.c              |   9 ++-
>   block/blkdebug.c                |   8 +--
>   block/blklogwrites.c            |  12 ++--
>   block/blkreplay.c               |   8 +--
>   block/blkverify.c               |   6 +-
>   block/block-backend.c           |  60 +++++++++---------
>   block/bochs.c                   |   2 +-
>   block/cloop.c                   |   2 +-
>   block/commit.c                  |   2 +-
>   block/copy-on-read.c            |   8 +--
>   block/crypto.c                  |   4 +-
>   block/curl.c                    |   2 +-
>   block/dmg.c                     |   2 +-
>   block/file-posix.c              |  42 ++++++++----
>   block/filter-compress.c         |  10 +--
>   block/gluster.c                 |  14 ++--
>   block/io.c                      | 109 +++++++++++++++++---------------
>   block/iscsi.c                   |  34 +++++++---
>   block/mirror.c                  |   8 +--
>   block/nbd.c                     |  16 +++--
>   block/nfs.c                     |   8 +--
>   block/null.c                    |   8 +--
>   block/nvme.c                    |  33 +++++++---
>   block/qcow.c                    |  12 ++--
>   block/qcow2.c                   |  29 +++++----
>   block/qed.c                     |  17 +++--
>   block/quorum.c                  |   8 +--
>   block/raw-format.c              |  32 +++++-----
>   block/rbd.c                     |   4 +-
>   block/sheepdog.c                |  11 +++-
>   block/throttle-groups.c         |   2 +-
>   block/throttle.c                |   8 +--
>   block/vdi.c                     |   4 +-
>   block/vmdk.c                    |  10 +--
>   block/vpc.c                     |   4 +-
>   block/vvfat.c                   |   6 +-
>   tests/test-bdrv-drain.c         |   8 +--
>   block/trace-events              |  14 ++--
>   42 files changed, 379 insertions(+), 297 deletions(-)
> 


-- 
Best regards,
Vladimir

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

[Eric Blake]

On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> v1 was "[RFC 0/3] 64bit block-layer part I", please refer to initial
> cover-letter
>   https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg08723.html
> for motivation.
> 
> v2:
> patch 02 is unchanged, add Stefan's r-b. Everything other is changed a
> lot. What's new:
> 

You'll also want to check my (now-abandoned?) posting from a while back:
https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg02769.html

to see what (if anything) from that attempt can be salvaged.

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

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

[Vladimir Sementsov-Ogievskiy]

29.04.2020 0:33, Eric Blake wrote:
> On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> v1 was "[RFC 0/3] 64bit block-layer part I", please refer to initial
>> cover-letter
>>   https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg08723.html
>> for motivation.
>>
>> v2:
>> patch 02 is unchanged, add Stefan's r-b. Everything other is changed a
>> lot. What's new:
>>
> 
> You'll also want to check my (now-abandoned?) posting from a while back:
> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg02769.html
> 
> to see what (if anything) from that attempt can be salvaged.
> 

Hmm, looks close :) will keep it in mind, thanks. Or, may be you want to resend? First 4 patches are not needed now, as bdrv_read/bdrv_write already dropped.

-- 
Best regards,
Vladimir