All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/3] 64bit block-layer part I
@ 2020-03-30 14:18 Vladimir Sementsov-Ogievskiy
  2020-03-30 14:18 ` [PATCH 1/3] block: use int64_t as bytes type in tracked requests Vladimir Sementsov-Ogievskiy
                   ` (9 more replies)
  0 siblings, 10 replies; 17+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-03-30 14:18 UTC (permalink / raw)
  To: qemu-block
  Cc: fam, kwolf, vsementsov, berto, stefanha, pavel.dovgaluk, sw, pl,
	qemu-devel, mreitz, jsnow, ronniesahlberg, den, pbonzini,
	dillaman, ari

Hi all!

There is an idea to make NBD protocol extension to support 64bit
write-zero/discard/block-status commands (commands, which doesn't
transfer user data). It's needed to increase performance of zeroing
large ranges (up to the whole image). Zeroing of the whole image is used
as first step of mirror job, qemu-img convert, it should be also used at
start of backup actually..

We need to support it in block-layer, so we want 64bit write_zeros.
Currently driver handler now have int bytes parameter.

write_zeros path goes through normal pwritev, so we need 64bit write,
and then we need 64bit read for symmetry, and better, let's make all io
path work with 64bit bytes parameter.

Actually most of block-layer already have 64bit parameters: offset is
sometimes int64_t and sometimes uint64_t. bytes parameter is one of
int64_t, uint64_t, int, unsigned int...

I think we need one type for all of this, and this one type is int64_t.
Signed int64_t is a bit better than uint64_t: you can use same variable
to get some result (including error < 0) and than reuse it as an
argument without any type conversion.

So, I propose, as a first step, convert all uint64_t parameters to
int64_t.

Still, I don't have good idea of how to split this into more than 3
patches, so, this is an RFC.

What's next?

Converting write_zero and discard is not as simple: we can't just
s/int/uint64_t/, as some functions may use some int variables for
calculations and this will be broken by something larger than int.

So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and
.bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all
drivers updated - drop unused 32bit functions, and then drop "64" suffix
from API. If not - we'll live with both APIs.

Another thing to do is updating default limiting of request (currently
they are limited to INT_MAX).

Then we may move some drivers to 64bit discard/write_zero: I think about
qcow2, file-posix and nbd (as a proof-of-concept for already proposed
NBD extension).

Any ideas?

Vladimir Sementsov-Ogievskiy (3):
  block: use int64_t as bytes type in tracked requests
  block/io: convert generic io path to use int64_t parameters
  block: use int64_t instead of uint64_t in driver handlers

 include/block/block.h     |  8 ++--
 include/block/block_int.h | 52 ++++++++++-----------
 block/backup-top.c        |  5 +-
 block/blkdebug.c          |  4 +-
 block/blklogwrites.c      |  4 +-
 block/blkreplay.c         |  4 +-
 block/blkverify.c         |  6 +--
 block/bochs.c             |  2 +-
 block/cloop.c             |  2 +-
 block/commit.c            |  2 +-
 block/copy-on-read.c      |  4 +-
 block/crypto.c            |  4 +-
 block/curl.c              |  2 +-
 block/dmg.c               |  2 +-
 block/file-posix.c        | 18 ++++----
 block/filter-compress.c   |  6 +--
 block/io.c                | 97 ++++++++++++++++++++-------------------
 block/iscsi.c             | 12 ++---
 block/mirror.c            |  4 +-
 block/nbd.c               |  8 ++--
 block/nfs.c               |  8 ++--
 block/null.c              |  8 ++--
 block/nvme.c              |  4 +-
 block/qcow.c              | 12 ++---
 block/qcow2.c             | 18 ++++----
 block/quorum.c            |  8 ++--
 block/raw-format.c        | 28 +++++------
 block/rbd.c               |  4 +-
 block/throttle.c          |  4 +-
 block/vdi.c               |  4 +-
 block/vmdk.c              |  8 ++--
 block/vpc.c               |  4 +-
 block/vvfat.c             |  6 +--
 tests/test-bdrv-drain.c   |  8 ++--
 block/trace-events        | 14 +++---
 35 files changed, 192 insertions(+), 192 deletions(-)

-- 
2.21.0



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/3] block: use int64_t as bytes type in tracked requests
  2020-03-30 14:18 [RFC 0/3] 64bit block-layer part I Vladimir Sementsov-Ogievskiy
@ 2020-03-30 14:18 ` Vladimir Sementsov-Ogievskiy
  2020-04-22 15:37   ` Stefan Hajnoczi
  2020-04-23 15:25   ` Kevin Wolf
  2020-03-30 14:18 ` [PATCH 2/3] block/io: convert generic io path to use int64_t parameters Vladimir Sementsov-Ogievskiy
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 17+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-03-30 14:18 UTC (permalink / raw)
  To: qemu-block
  Cc: fam, kwolf, vsementsov, berto, stefanha, pavel.dovgaluk, sw, pl,
	qemu-devel, mreitz, jsnow, ronniesahlberg, den, pbonzini,
	dillaman, ari

We are generally moving to int64_t for both offset and bytes paramaters
on all io paths. Convert tracked requests now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/3] block/io: convert generic io path to use int64_t parameters
  2020-03-30 14:18 [RFC 0/3] 64bit block-layer part I Vladimir Sementsov-Ogievskiy
  2020-03-30 14:18 ` [PATCH 1/3] block: use int64_t as bytes type in tracked requests Vladimir Sementsov-Ogievskiy
@ 2020-03-30 14:18 ` Vladimir Sementsov-Ogievskiy
  2020-04-22 15:50   ` Stefan Hajnoczi
  2020-03-30 14:18 ` [PATCH 3/3] block: use int64_t instead of uint64_t in driver handlers Vladimir Sementsov-Ogievskiy
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-03-30 14:18 UTC (permalink / raw)
  To: qemu-block
  Cc: fam, kwolf, vsementsov, berto, stefanha, pavel.dovgaluk, sw, pl,
	qemu-devel, mreitz, jsnow, ronniesahlberg, den, pbonzini,
	dillaman, ari

We are generally moving to int64_t for both offset and bytes paramaters
on all io paths. Convert generic io path now.

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

diff --git a/include/block/block.h b/include/block/block.h
index b05995fe9c..fa5c7285b6 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -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);
@@ -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 c8daba608b..28aea2bcfd 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -972,16 +972,16 @@ 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, 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,
+    int64_t offset, int64_t 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,
@@ -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/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 7cbb80bd24..79e3014489 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)
@@ -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 (offset < 0 || bytes < 0 || bytes > BDRV_REQUEST_MAX_BYTES) {
         return -EIO;
     }
 
@@ -885,10 +885,6 @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
         return -ENOMEDIUM;
     }
 
-    if (offset < 0) {
-        return -EIO;
-    }
-
     return 0;
 }
 
@@ -1086,7 +1082,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 +1151,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 +1231,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;
@@ -1268,7 +1264,7 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs, uint64_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;
@@ -1430,16 +1426,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);
@@ -1590,13 +1587,13 @@ static int bdrv_padding_rmw_read(BdrvChild *child,
 {
     QEMUIOVector local_qiov;
     BlockDriverState *bs = child->bs;
-    uint64_t align = bs->bl.request_alignment;
+    int64_t align = bs->bl.request_alignment;
     int ret;
 
     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);
 
@@ -1667,7 +1664,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)) {
@@ -1686,14 +1683,14 @@ 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);
 }
 
 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)
 {
@@ -1702,7 +1699,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) {
@@ -1743,7 +1740,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 +1770,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
@@ -1799,6 +1796,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
         ret = -ENOTSUP;
         /* First try the efficient write zeroes operation */
         if (drv->bdrv_co_pwrite_zeroes) {
+            assert(num <= INT_MAX);
             ret = drv->bdrv_co_pwrite_zeroes(bs, offset, num,
                                              flags & bs->supported_zero_flags);
             if (ret != -ENOTSUP && (flags & BDRV_REQ_FUA) &&
@@ -1854,7 +1852,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 +1904,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 +1946,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 +1965,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);
@@ -2030,7 +2030,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)
 {
@@ -2067,7 +2067,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) {
@@ -2097,14 +2097,14 @@ 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);
 }
 
 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;
@@ -2113,7 +2113,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;
@@ -2174,7 +2174,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);
 
@@ -3205,8 +3205,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)
 {
@@ -3284,9 +3284,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)
 {
@@ -3300,9 +3300,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)
 {
@@ -3312,9 +3312,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 29dff8881c..e4d68438af 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -11,12 +11,12 @@ 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_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_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_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, 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, 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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/3] block: use int64_t instead of uint64_t in driver handlers
  2020-03-30 14:18 [RFC 0/3] 64bit block-layer part I Vladimir Sementsov-Ogievskiy
  2020-03-30 14:18 ` [PATCH 1/3] block: use int64_t as bytes type in tracked requests Vladimir Sementsov-Ogievskiy
  2020-03-30 14:18 ` [PATCH 2/3] block/io: convert generic io path to use int64_t parameters Vladimir Sementsov-Ogievskiy
@ 2020-03-30 14:18 ` Vladimir Sementsov-Ogievskiy
  2020-03-30 17:43 ` [RFC 0/3] 64bit block-layer part I no-reply
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-03-30 14:18 UTC (permalink / raw)
  To: qemu-block
  Cc: fam, kwolf, vsementsov, berto, stefanha, pavel.dovgaluk, sw, pl,
	qemu-devel, mreitz, jsnow, ronniesahlberg, den, pbonzini,
	dillaman, ari

We are generally moving to int64_t for both offset and bytes parameters
on all io paths. Convert driver handlers parameters which are already
64bit to signed type.

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

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 28aea2bcfd..ea8fca5e28 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -180,10 +180,10 @@ 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,
+        int64_t offset, int64_t bytes, QEMUIOVector *qiov, int flags,
         BlockCompletionFunc *cb, void *opaque);
     BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
         BlockCompletionFunc *cb, void *opaque);
@@ -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);
@@ -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);
 
     /*
@@ -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);
 
@@ -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 3b50c06e2c..4190d465d6 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);
@@ -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 af44aa973f..b4d0966982 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;
@@ -640,7 +640,7 @@ blkdebug_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_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 04d8b33607..890a61dfba 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);
@@ -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 c96ac8f4bc..d93383a88f 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);
@@ -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 667e60d832..a3c447fc68 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;
@@ -253,7 +253,7 @@ blkverify_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_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/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..fc3186bacb 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,
@@ -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 d577f89659..ecb5697a4b 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;
@@ -398,7 +398,7 @@ block_crypto_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_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/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..76da620135 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1941,15 +1941,15 @@ 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);
 }
 
-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);
@@ -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/filter-compress.c b/block/filter-compress.c
index 82c315b298..f2400fea37 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)
@@ -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/iscsi.c b/block/iscsi.c
index 4e216bd8aa..b403e67be7 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2181,10 +2181,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)
 {
@@ -2322,10 +2322,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/mirror.c b/block/mirror.c
index c26fd9260d..687a91e654 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);
 }
@@ -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 2160859f64..54bce3911e 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;
@@ -1218,8 +1218,8 @@ static int nbd_client_co_preadv(BlockDriverState *bs, uint64_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 cc2413d5ab..d59d8e7d32 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;
@@ -298,8 +298,8 @@ static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, uint64_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 15e1d56746..cac37e7ba6 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;
@@ -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);
@@ -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)
@@ -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 7b7c0cc5d6..db7fffe94f 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1062,14 +1062,14 @@ 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);
 }
 
 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 8973e4e565..9583e8e8b2 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;
@@ -725,8 +725,8 @@ static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, uint64_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 2bb536b014..4d4f972187 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)
 {
@@ -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;
@@ -3803,9 +3803,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;
@@ -3884,9 +3884,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;
@@ -4322,7 +4322,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 6d7a56bd93..4164c3fbf8 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);
@@ -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 93b25e1b6b..2537755f84 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;
@@ -206,8 +206,8 @@ static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_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;
@@ -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, &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;
     }
@@ -507,10 +507,10 @@ 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)
 {
@@ -526,10 +526,10 @@ 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)
 {
diff --git a/block/rbd.c b/block/rbd.c
index e637639a07..412df8cdb7 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)
@@ -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 71f4bb0ad1..28e73073e7 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)
 {
 
@@ -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 e1a11f2aa0..07ad195239 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;
@@ -603,7 +603,7 @@ vdi_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_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 218d9c9800..a88e9c9ba4 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;
@@ -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 6df75e22dc..960c198afc 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;
@@ -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 ab800c4887..97e6b0e2a5 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;
@@ -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/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)
 {
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



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [RFC 0/3] 64bit block-layer part I
  2020-03-30 14:18 [RFC 0/3] 64bit block-layer part I Vladimir Sementsov-Ogievskiy
                   ` (2 preceding siblings ...)
  2020-03-30 14:18 ` [PATCH 3/3] block: use int64_t instead of uint64_t in driver handlers Vladimir Sementsov-Ogievskiy
@ 2020-03-30 17:43 ` no-reply
  2020-03-30 17:48 ` no-reply
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: no-reply @ 2020-03-30 17:43 UTC (permalink / raw)
  To: vsementsov
  Cc: fam, kwolf, vsementsov, berto, pavel.dovgaluk, qemu-block,
	dillaman, sw, pl, qemu-devel, mreitz, ari, stefanha, pbonzini,
	den, jsnow, ronniesahlberg

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



Hi,

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

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

clang -iquote /tmp/qemu-test/build/tests -iquote tests -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/usr/include/pixman-1   -Werror -fsanitize=undefined -fsanitize=address  -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong   -I/usr/include/p11-kit-1   -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED  -I/usr/include/libpng16  -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests -I/tmp/qemu-test/src/tests/qtest -MMD -MP -MT tests/test-crypto-hash.o -MF tests/test-crypto-hash.d -g   -c -o tests/test-crypto-hash.o /tmp/qemu-test/src/tests/test-crypto-hash.c
clang -iquote /tmp/qemu-test/build/tests -iquote tests -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/usr/include/pixman-1   -Werror -fsanitize=undefined -fsanitize=address  -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong   -I/usr/include/p11-kit-1   -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED  -I/usr/include/libpng16  -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests -I/tmp/qemu-test/src/tests/qtest -MMD -MP -MT tests/test-crypto-hmac.o -MF tests/test-crypto-hmac.d -g   -c -o tests/test-crypto-hmac.o /tmp/qemu-test/src/tests/test-crypto-hmac.c
clang -iquote /tmp/qemu-test/build/tests -iquote tests -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/usr/include/pixman-1   -Werror -fsanitize=undefined -fsanitize=address  -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong   -I/usr/include/p11-kit-1   -DLEGACY_RDMA_REG_MR -DSTRUCT_IOVEC_DEFINED  -I/usr/include/libpng16  -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests -I/tmp/qemu-test/src/tests/qtest -MMD -MP -MT tests/test-crypto-cipher.o -MF tests/test-crypto-cipher.d -g   -c -o tests/test-crypto-cipher.o /tmp/qemu-test/src/tests/test-crypto-cipher.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,
                              ^~~~~~~~~~~~~~~~~
2 errors generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: tests/test-block-iothread.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=733d780c712b4d27a92e0973628b05b2', '-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-c0tiulxy/src/docker-src.2020-03-30-13.37.39.18228:/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=733d780c712b4d27a92e0973628b05b2
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-c0tiulxy/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    5m21.587s
user    0m8.451s


The full log is available at
http://patchew.org/logs/20200330141818.31294-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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 0/3] 64bit block-layer part I
  2020-03-30 14:18 [RFC 0/3] 64bit block-layer part I Vladimir Sementsov-Ogievskiy
                   ` (3 preceding siblings ...)
  2020-03-30 17:43 ` [RFC 0/3] 64bit block-layer part I no-reply
@ 2020-03-30 17:48 ` no-reply
  2020-03-30 17:50 ` no-reply
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: no-reply @ 2020-03-30 17:48 UTC (permalink / raw)
  To: vsementsov
  Cc: fam, kwolf, vsementsov, berto, pavel.dovgaluk, qemu-block,
	dillaman, sw, pl, qemu-devel, mreitz, ari, stefanha, pbonzini,
	den, jsnow, ronniesahlberg

Patchew URL: https://patchew.org/QEMU/20200330141818.31294-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-int128.o
  CC      tests/rcutorture.o
  CC      tests/test-rcu-list.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]
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=ac3e5859c1b54c68988c07cda02958cd', '-u', '1003', '--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/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-4cxd8d3k/src/docker-src.2020-03-30-13.44.40.29863:/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=ac3e5859c1b54c68988c07cda02958cd
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-4cxd8d3k/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    3m30.965s
user    0m8.371s


The full log is available at
http://patchew.org/logs/20200330141818.31294-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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 0/3] 64bit block-layer part I
  2020-03-30 14:18 [RFC 0/3] 64bit block-layer part I Vladimir Sementsov-Ogievskiy
                   ` (4 preceding siblings ...)
  2020-03-30 17:48 ` no-reply
@ 2020-03-30 17:50 ` no-reply
  2020-04-22 14:29 ` Vladimir Sementsov-Ogievskiy
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: no-reply @ 2020-03-30 17:50 UTC (permalink / raw)
  To: vsementsov
  Cc: fam, kwolf, vsementsov, berto, pavel.dovgaluk, qemu-block,
	dillaman, sw, pl, qemu-devel, mreitz, ari, stefanha, pbonzini,
	den, jsnow, ronniesahlberg

Patchew URL: https://patchew.org/QEMU/20200330141818.31294-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/nbd.o
  CC      block/sheepdog.o
  CC      block/accounting.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=46f0957c6a3a47e5971905cb30141048', '-u', '1003', '--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/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-0szyhgs2/src/docker-src.2020-03-30-13.48.43.9903:/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=46f0957c6a3a47e5971905cb30141048
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-0szyhgs2/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    2m13.293s
user    0m8.052s


The full log is available at
http://patchew.org/logs/20200330141818.31294-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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 0/3] 64bit block-layer part I
  2020-03-30 14:18 [RFC 0/3] 64bit block-layer part I Vladimir Sementsov-Ogievskiy
                   ` (5 preceding siblings ...)
  2020-03-30 17:50 ` no-reply
@ 2020-04-22 14:29 ` Vladimir Sementsov-Ogievskiy
  2020-04-22 14:52   ` Eric Blake
  2020-04-22 15:53 ` Stefan Hajnoczi
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-22 14:29 UTC (permalink / raw)
  To: qemu-block
  Cc: fam, kwolf, berto, stefanha, pavel.dovgaluk, sw, pl, qemu-devel,
	mreitz, jsnow, ronniesahlberg, den, pbonzini, dillaman, ari

Any thoughts here? I need to resend to update some more functions as patchew said.

Is it OK in general? Or should we instead convert everything to uint64_t ?

30.03.2020 17:18, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> There is an idea to make NBD protocol extension to support 64bit
> write-zero/discard/block-status commands (commands, which doesn't
> transfer user data). It's needed to increase performance of zeroing
> large ranges (up to the whole image). Zeroing of the whole image is used
> as first step of mirror job, qemu-img convert, it should be also used at
> start of backup actually..
> 
> We need to support it in block-layer, so we want 64bit write_zeros.
> Currently driver handler now have int bytes parameter.
> 
> write_zeros path goes through normal pwritev, so we need 64bit write,
> and then we need 64bit read for symmetry, and better, let's make all io
> path work with 64bit bytes parameter.
> 
> Actually most of block-layer already have 64bit parameters: offset is
> sometimes int64_t and sometimes uint64_t. bytes parameter is one of
> int64_t, uint64_t, int, unsigned int...
> 
> I think we need one type for all of this, and this one type is int64_t.
> Signed int64_t is a bit better than uint64_t: you can use same variable
> to get some result (including error < 0) and than reuse it as an
> argument without any type conversion.
> 
> So, I propose, as a first step, convert all uint64_t parameters to
> int64_t.
> 
> Still, I don't have good idea of how to split this into more than 3
> patches, so, this is an RFC.
> 
> What's next?
> 
> Converting write_zero and discard is not as simple: we can't just
> s/int/uint64_t/, as some functions may use some int variables for
> calculations and this will be broken by something larger than int.
> 
> So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and
> .bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all
> drivers updated - drop unused 32bit functions, and then drop "64" suffix
> from API. If not - we'll live with both APIs.
> 
> Another thing to do is updating default limiting of request (currently
> they are limited to INT_MAX).
> 
> Then we may move some drivers to 64bit discard/write_zero: I think about
> qcow2, file-posix and nbd (as a proof-of-concept for already proposed
> NBD extension).
> 
> Any ideas?
> 
> Vladimir Sementsov-Ogievskiy (3):
>    block: use int64_t as bytes type in tracked requests
>    block/io: convert generic io path to use int64_t parameters
>    block: use int64_t instead of uint64_t in driver handlers
> 
>   include/block/block.h     |  8 ++--
>   include/block/block_int.h | 52 ++++++++++-----------
>   block/backup-top.c        |  5 +-
>   block/blkdebug.c          |  4 +-
>   block/blklogwrites.c      |  4 +-
>   block/blkreplay.c         |  4 +-
>   block/blkverify.c         |  6 +--
>   block/bochs.c             |  2 +-
>   block/cloop.c             |  2 +-
>   block/commit.c            |  2 +-
>   block/copy-on-read.c      |  4 +-
>   block/crypto.c            |  4 +-
>   block/curl.c              |  2 +-
>   block/dmg.c               |  2 +-
>   block/file-posix.c        | 18 ++++----
>   block/filter-compress.c   |  6 +--
>   block/io.c                | 97 ++++++++++++++++++++-------------------
>   block/iscsi.c             | 12 ++---
>   block/mirror.c            |  4 +-
>   block/nbd.c               |  8 ++--
>   block/nfs.c               |  8 ++--
>   block/null.c              |  8 ++--
>   block/nvme.c              |  4 +-
>   block/qcow.c              | 12 ++---
>   block/qcow2.c             | 18 ++++----
>   block/quorum.c            |  8 ++--
>   block/raw-format.c        | 28 +++++------
>   block/rbd.c               |  4 +-
>   block/throttle.c          |  4 +-
>   block/vdi.c               |  4 +-
>   block/vmdk.c              |  8 ++--
>   block/vpc.c               |  4 +-
>   block/vvfat.c             |  6 +--
>   tests/test-bdrv-drain.c   |  8 ++--
>   block/trace-events        | 14 +++---
>   35 files changed, 192 insertions(+), 192 deletions(-)
> 


-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 0/3] 64bit block-layer part I
  2020-04-22 14:29 ` Vladimir Sementsov-Ogievskiy
@ 2020-04-22 14:52   ` Eric Blake
  0 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2020-04-22 14:52 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: fam, kwolf, berto, stefanha, pavel.dovgaluk, sw, pl, qemu-devel,
	mreitz, jsnow, ronniesahlberg, den, pbonzini, dillaman, ari

On 4/22/20 9:29 AM, Vladimir Sementsov-Ogievskiy wrote:
> Any thoughts here? I need to resend to update some more functions as 
> patchew said.
> 
> Is it OK in general? Or should we instead convert everything to uint64_t ?

I definitely prefer int64_t as our base (off_t is signed as well, making 
63 bits an effective limit everywhere).

> 
> 30.03.2020 17:18, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> There is an idea to make NBD protocol extension to support 64bit
>> write-zero/discard/block-status commands (commands, which doesn't
>> transfer user data). It's needed to increase performance of zeroing
>> large ranges (up to the whole image). Zeroing of the whole image is used
>> as first step of mirror job, qemu-img convert, it should be also used at
>> start of backup actually..
>>
>> We need to support it in block-layer, so we want 64bit write_zeros.
>> Currently driver handler now have int bytes parameter.
>>
>> write_zeros path goes through normal pwritev, so we need 64bit write,
>> and then we need 64bit read for symmetry, and better, let's make all io
>> path work with 64bit bytes parameter.
>>
>> Actually most of block-layer already have 64bit parameters: offset is
>> sometimes int64_t and sometimes uint64_t. bytes parameter is one of
>> int64_t, uint64_t, int, unsigned int...
>>
>> I think we need one type for all of this, and this one type is int64_t.
>> Signed int64_t is a bit better than uint64_t: you can use same variable
>> to get some result (including error < 0) and than reuse it as an
>> argument without any type conversion.
>>
>> So, I propose, as a first step, convert all uint64_t parameters to
>> int64_t.

Makes sense.  I haven't looked at the series closely in part because it 
was 5.1 material while we were still focused on getting 5.0 out the 
door, but it is now raising in my review queue again.

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



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] block: use int64_t as bytes type in tracked requests
  2020-03-30 14:18 ` [PATCH 1/3] block: use int64_t as bytes type in tracked requests Vladimir Sementsov-Ogievskiy
@ 2020-04-22 15:37   ` Stefan Hajnoczi
  2020-04-23 15:25   ` Kevin Wolf
  1 sibling, 0 replies; 17+ messages in thread
From: Stefan Hajnoczi @ 2020-04-22 15:37 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: fam, kwolf, pavel.dovgaluk, qemu-block, sw, pl, qemu-devel,
	mreitz, stefanha, pbonzini, den, ronniesahlberg

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

On Mon, Mar 30, 2020 at 05:18:16PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We are generally moving to int64_t for both offset and bytes paramaters
> on all io paths. Convert tracked requests now.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/block/block_int.h |  4 ++--
>  block/io.c                | 11 ++++++-----
>  2 files changed, 8 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] block/io: convert generic io path to use int64_t parameters
  2020-03-30 14:18 ` [PATCH 2/3] block/io: convert generic io path to use int64_t parameters Vladimir Sementsov-Ogievskiy
@ 2020-04-22 15:50   ` Stefan Hajnoczi
  2020-04-22 17:45     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Hajnoczi @ 2020-04-22 15:50 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: fam, kwolf, berto, pavel.dovgaluk, qemu-block, dillaman, sw, pl,
	qemu-devel, mreitz, ari, stefanha, pbonzini, den, jsnow,
	ronniesahlberg

[-- Attachment #1: Type: text/plain, Size: 2375 bytes --]

On Mon, Mar 30, 2020 at 05:18:17PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> @@ -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 (offset < 0 || bytes < 0 || bytes > BDRV_REQUEST_MAX_BYTES) {
>          return -EIO;
>      }
>  
> @@ -885,10 +885,6 @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
>          return -ENOMEDIUM;
>      }
>  
> -    if (offset < 0) {
> -        return -EIO;
> -    }
> -
>      return 0;
>  }

Moving this if statement was unnecessary.  I'm not sure if there are
cases where callers will now see EIO instead of ENOMEDIUM and change
their behavior :(.

More conservative code changes are easier to review and merge because
they are less risky.

> @@ -1743,7 +1740,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 +1770,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
> @@ -1799,6 +1796,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
>          ret = -ENOTSUP;
>          /* First try the efficient write zeroes operation */
>          if (drv->bdrv_co_pwrite_zeroes) {
> +            assert(num <= INT_MAX);

max_write_zeroes already enforces this, so the assertion is always
false:

  int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX);
  ...
        /* limit request size */
        if (num > max_write_zeroes) {
            num = max_write_zeroes;
        }

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 0/3] 64bit block-layer part I
  2020-03-30 14:18 [RFC 0/3] 64bit block-layer part I Vladimir Sementsov-Ogievskiy
                   ` (6 preceding siblings ...)
  2020-04-22 14:29 ` Vladimir Sementsov-Ogievskiy
@ 2020-04-22 15:53 ` Stefan Hajnoczi
  2020-04-22 18:24 ` Vladimir Sementsov-Ogievskiy
  2020-04-23 15:43 ` Kevin Wolf
  9 siblings, 0 replies; 17+ messages in thread
From: Stefan Hajnoczi @ 2020-04-22 15:53 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: fam, kwolf, pavel.dovgaluk, qemu-block, sw, pl, qemu-devel,
	mreitz, stefanha, pbonzini, den, ronniesahlberg

[-- Attachment #1: Type: text/plain, Size: 2396 bytes --]

On Mon, Mar 30, 2020 at 05:18:15PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> There is an idea to make NBD protocol extension to support 64bit
> write-zero/discard/block-status commands (commands, which doesn't
> transfer user data). It's needed to increase performance of zeroing
> large ranges (up to the whole image). Zeroing of the whole image is used
> as first step of mirror job, qemu-img convert, it should be also used at
> start of backup actually..
> 
> We need to support it in block-layer, so we want 64bit write_zeros.
> Currently driver handler now have int bytes parameter.
> 
> write_zeros path goes through normal pwritev, so we need 64bit write,
> and then we need 64bit read for symmetry, and better, let's make all io
> path work with 64bit bytes parameter.
> 
> Actually most of block-layer already have 64bit parameters: offset is
> sometimes int64_t and sometimes uint64_t. bytes parameter is one of
> int64_t, uint64_t, int, unsigned int...
> 
> I think we need one type for all of this, and this one type is int64_t.
> Signed int64_t is a bit better than uint64_t: you can use same variable
> to get some result (including error < 0) and than reuse it as an
> argument without any type conversion.
> 
> So, I propose, as a first step, convert all uint64_t parameters to
> int64_t.
> 
> Still, I don't have good idea of how to split this into more than 3
> patches, so, this is an RFC.
> 
> What's next?
> 
> Converting write_zero and discard is not as simple: we can't just
> s/int/uint64_t/, as some functions may use some int variables for
> calculations and this will be broken by something larger than int.
> 
> So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and
> .bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all
> drivers updated - drop unused 32bit functions, and then drop "64" suffix
> from API. If not - we'll live with both APIs.
> 
> Another thing to do is updating default limiting of request (currently
> they are limited to INT_MAX).
> 
> Then we may move some drivers to 64bit discard/write_zero: I think about
> qcow2, file-posix and nbd (as a proof-of-concept for already proposed
> NBD extension).

Sounds good to me.  This is a good series to merge early in the QEMU 5.1
release cycle.  That way we'll have time to catch any issues.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] block/io: convert generic io path to use int64_t parameters
  2020-04-22 15:50   ` Stefan Hajnoczi
@ 2020-04-22 17:45     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 17+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-22 17:45 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: fam, kwolf, berto, pavel.dovgaluk, qemu-block, dillaman, sw, pl,
	qemu-devel, mreitz, ari, stefanha, pbonzini, den, jsnow,
	ronniesahlberg

22.04.2020 18:50, Stefan Hajnoczi wrote:
> On Mon, Mar 30, 2020 at 05:18:17PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> @@ -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 (offset < 0 || bytes < 0 || bytes > BDRV_REQUEST_MAX_BYTES) {
>>           return -EIO;
>>       }
>>   
>> @@ -885,10 +885,6 @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
>>           return -ENOMEDIUM;
>>       }
>>   
>> -    if (offset < 0) {
>> -        return -EIO;
>> -    }
>> -
>>       return 0;
>>   }
> 
> Moving this if statement was unnecessary.  I'm not sure if there are
> cases where callers will now see EIO instead of ENOMEDIUM and change
> their behavior :(.
> 
> More conservative code changes are easier to review and merge because
> they are less risky.

Hmm, would be a bit strange to just add "bytes < 0" to the first "if" keeping "offset < 0" in the third.
And strange to keep "bytes > BDRV_REQUEST_MAX_BYTES" in the first, if add "bytes < 0" to the third..

> 
>> @@ -1743,7 +1740,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 +1770,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
>> @@ -1799,6 +1796,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
>>           ret = -ENOTSUP;
>>           /* First try the efficient write zeroes operation */
>>           if (drv->bdrv_co_pwrite_zeroes) {
>> +            assert(num <= INT_MAX);
> 
> max_write_zeroes already enforces this, so the assertion is always
> false:
> 
>    int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX);
>    ...
>          /* limit request size */
>          if (num > max_write_zeroes) {
>              num = max_write_zeroes;
>          }
> 

You are right, I'll drop it.

-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 0/3] 64bit block-layer part I
  2020-03-30 14:18 [RFC 0/3] 64bit block-layer part I Vladimir Sementsov-Ogievskiy
                   ` (7 preceding siblings ...)
  2020-04-22 15:53 ` Stefan Hajnoczi
@ 2020-04-22 18:24 ` Vladimir Sementsov-Ogievskiy
  2020-04-22 19:32   ` Eric Blake
  2020-04-23 15:43 ` Kevin Wolf
  9 siblings, 1 reply; 17+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-22 18:24 UTC (permalink / raw)
  To: qemu-block
  Cc: fam, kwolf, berto, stefanha, pavel.dovgaluk, sw, pl, qemu-devel,
	mreitz, jsnow, ronniesahlberg, den, pbonzini, dillaman, ari

30.03.2020 17:18, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> There is an idea to make NBD protocol extension to support 64bit
> write-zero/discard/block-status commands (commands, which doesn't
> transfer user data). It's needed to increase performance of zeroing
> large ranges (up to the whole image). Zeroing of the whole image is used
> as first step of mirror job, qemu-img convert, it should be also used at
> start of backup actually..
> 
> We need to support it in block-layer, so we want 64bit write_zeros.
> Currently driver handler now have int bytes parameter.
> 
> write_zeros path goes through normal pwritev, so we need 64bit write,
> and then we need 64bit read for symmetry, and better, let's make all io
> path work with 64bit bytes parameter.
> 
> Actually most of block-layer already have 64bit parameters: offset is
> sometimes int64_t and sometimes uint64_t. bytes parameter is one of
> int64_t, uint64_t, int, unsigned int...
> 
> I think we need one type for all of this, and this one type is int64_t.
> Signed int64_t is a bit better than uint64_t: you can use same variable
> to get some result (including error < 0) and than reuse it as an
> argument without any type conversion.
> 
> So, I propose, as a first step, convert all uint64_t parameters to
> int64_t.
> 
> Still, I don't have good idea of how to split this into more than 3
> patches, so, this is an RFC.
> 
> What's next?
> 
> Converting write_zero and discard is not as simple: we can't just
> s/int/uint64_t/, as some functions may use some int variables for
> calculations and this will be broken by something larger than int.
> 
> So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and
> .bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all
> drivers updated - drop unused 32bit functions, and then drop "64" suffix
> from API. If not - we'll live with both APIs.

Hmm. Or, maybe nothing dangerous if we convert it to 64bit, and add comment,
that function is not actually prepared for 64bit bytes and depends on
max_transfer/max_zeroes being <= INT_MAX ?

Or, maybe better, keep old functions as is and add 64bit wrappers, which
assert that bytes < INT_MAX, and call old function? This is clean, driver
maybe updated later on demand, and we don't need extra API. If no objections,
I'll try this way in the next version.

> 
> Another thing to do is updating default limiting of request (currently
> they are limited to INT_MAX).
> 
> Then we may move some drivers to 64bit discard/write_zero: I think about
> qcow2, file-posix and nbd (as a proof-of-concept for already proposed
> NBD extension).
> 
> Any ideas?
> 
> Vladimir Sementsov-Ogievskiy (3):
>    block: use int64_t as bytes type in tracked requests
>    block/io: convert generic io path to use int64_t parameters
>    block: use int64_t instead of uint64_t in driver handlers
> 
>   include/block/block.h     |  8 ++--
>   include/block/block_int.h | 52 ++++++++++-----------
>   block/backup-top.c        |  5 +-
>   block/blkdebug.c          |  4 +-
>   block/blklogwrites.c      |  4 +-
>   block/blkreplay.c         |  4 +-
>   block/blkverify.c         |  6 +--
>   block/bochs.c             |  2 +-
>   block/cloop.c             |  2 +-
>   block/commit.c            |  2 +-
>   block/copy-on-read.c      |  4 +-
>   block/crypto.c            |  4 +-
>   block/curl.c              |  2 +-
>   block/dmg.c               |  2 +-
>   block/file-posix.c        | 18 ++++----
>   block/filter-compress.c   |  6 +--
>   block/io.c                | 97 ++++++++++++++++++++-------------------
>   block/iscsi.c             | 12 ++---
>   block/mirror.c            |  4 +-
>   block/nbd.c               |  8 ++--
>   block/nfs.c               |  8 ++--
>   block/null.c              |  8 ++--
>   block/nvme.c              |  4 +-
>   block/qcow.c              | 12 ++---
>   block/qcow2.c             | 18 ++++----
>   block/quorum.c            |  8 ++--
>   block/raw-format.c        | 28 +++++------
>   block/rbd.c               |  4 +-
>   block/throttle.c          |  4 +-
>   block/vdi.c               |  4 +-
>   block/vmdk.c              |  8 ++--
>   block/vpc.c               |  4 +-
>   block/vvfat.c             |  6 +--
>   tests/test-bdrv-drain.c   |  8 ++--
>   block/trace-events        | 14 +++---
>   35 files changed, 192 insertions(+), 192 deletions(-)
> 


-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 0/3] 64bit block-layer part I
  2020-04-22 18:24 ` Vladimir Sementsov-Ogievskiy
@ 2020-04-22 19:32   ` Eric Blake
  0 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2020-04-22 19:32 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block
  Cc: fam, kwolf, berto, stefanha, pavel.dovgaluk, sw, pl, qemu-devel,
	mreitz, jsnow, ronniesahlberg, den, pbonzini, dillaman, ari

On 4/22/20 1:24 PM, Vladimir Sementsov-Ogievskiy wrote:

>> So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and
>> .bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all
>> drivers updated - drop unused 32bit functions, and then drop "64" suffix
>> from API. If not - we'll live with both APIs.
> 
> Hmm. Or, maybe nothing dangerous if we convert it to 64bit, and add 
> comment,
> that function is not actually prepared for 64bit bytes and depends on
> max_transfer/max_zeroes being <= INT_MAX ?
> 
> Or, maybe better, keep old functions as is and add 64bit wrappers, which
> assert that bytes < INT_MAX, and call old function? This is clean, driver
> maybe updated later on demand, and we don't need extra API. If no 
> objections,
> I'll try this way in the next version.

That approach sounds good; it matches how we added flags and iov 
support, as well as how we transitioned from sector interfaces over to 
byte interfaces: we added a new .bdrv_ callback for the drivers that 
could take advantage of the increased interface, without having to touch 
the older drivers using the old callbacks, then only later finally got 
rid of the old interfaces as we modernized other drivers.  There's still 
the issue of how much we convert in the initial series - even if adding 
a new wrapper makes it easier to patch only one driver at a time, it's 
not nice to leave the job half-done by not visiting all of the drivers 
eventually.

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



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] block: use int64_t as bytes type in tracked requests
  2020-03-30 14:18 ` [PATCH 1/3] block: use int64_t as bytes type in tracked requests Vladimir Sementsov-Ogievskiy
  2020-04-22 15:37   ` Stefan Hajnoczi
@ 2020-04-23 15:25   ` Kevin Wolf
  1 sibling, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2020-04-23 15:25 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: fam, berto, stefanha, qemu-block, dillaman, pavel.dovgaluk, sw,
	pl, qemu-devel, mreitz, jsnow, ronniesahlberg, den, pbonzini,
	ari

Am 30.03.2020 um 16:18 hat Vladimir Sementsov-Ogievskiy geschrieben:
> We are generally moving to int64_t for both offset and bytes paramaters
> on all io paths. Convert tracked requests now.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.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;

An indentation with 8 spaces is unconventional in QEMU.

Kevin



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC 0/3] 64bit block-layer part I
  2020-03-30 14:18 [RFC 0/3] 64bit block-layer part I Vladimir Sementsov-Ogievskiy
                   ` (8 preceding siblings ...)
  2020-04-22 18:24 ` Vladimir Sementsov-Ogievskiy
@ 2020-04-23 15:43 ` Kevin Wolf
  9 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2020-04-23 15:43 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: fam, berto, stefanha, qemu-block, dillaman, pavel.dovgaluk, sw,
	pl, qemu-devel, mreitz, jsnow, ronniesahlberg, den, pbonzini,
	ari

Am 30.03.2020 um 16:18 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Hi all!
> 
> There is an idea to make NBD protocol extension to support 64bit
> write-zero/discard/block-status commands (commands, which doesn't
> transfer user data). It's needed to increase performance of zeroing
> large ranges (up to the whole image). Zeroing of the whole image is used
> as first step of mirror job, qemu-img convert, it should be also used at
> start of backup actually..
> 
> We need to support it in block-layer, so we want 64bit write_zeros.
> Currently driver handler now have int bytes parameter.
> 
> write_zeros path goes through normal pwritev, so we need 64bit write,
> and then we need 64bit read for symmetry, and better, let's make all io
> path work with 64bit bytes parameter.
> 
> Actually most of block-layer already have 64bit parameters: offset is
> sometimes int64_t and sometimes uint64_t. bytes parameter is one of
> int64_t, uint64_t, int, unsigned int...
> 
> I think we need one type for all of this, and this one type is int64_t.
> Signed int64_t is a bit better than uint64_t: you can use same variable
> to get some result (including error < 0) and than reuse it as an
> argument without any type conversion.
> 
> So, I propose, as a first step, convert all uint64_t parameters to
> int64_t.
> 
> Still, I don't have good idea of how to split this into more than 3
> patches, so, this is an RFC.

I think the split in three patches isn't too bad because it's not a
whole lot of code. But of course, it is little code that has lots of
implications which does make it hard to review. If we think that we
might bisect a bug in the series later, maybe it would be better to
split it into more patches.

write/write_zeroes has to be a single thing, I'm afraid. But I guess
read could be a separate patch, as could be copy_range. Not sure about
discard.

> What's next?
> 
> Converting write_zero and discard is not as simple: we can't just
> s/int/uint64_t/, as some functions may use some int variables for
> calculations and this will be broken by something larger than int.
> 
> So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and
> .bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all
> drivers updated - drop unused 32bit functions, and then drop "64" suffix
> from API. If not - we'll live with both APIs.

We already have too many unfinished conversions in QEMU, let's not add
one more.

Fortunately, we already have a tool that could help us here: Things like
bs->bl.max_pwrite_zeroes. We could make BDRV_REQUEST_MAX_BYTES the
default value and only drivers that override it can get bigger requests.

> Another thing to do is updating default limiting of request (currently
> they are limited to INT_MAX).

As above, I wouldn't update the default, but rather enable drivers to
overload the default with a larger value. This will involve changing
some places where we use MIN() between INT_MAX and the driver's value.

> Then we may move some drivers to 64bit discard/write_zero: I think about
> qcow2, file-posix and nbd (as a proof-of-concept for already proposed
> NBD extension).

Makes sense to me.

Kevin



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-04-23 16:58 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-30 14:18 [RFC 0/3] 64bit block-layer part I Vladimir Sementsov-Ogievskiy
2020-03-30 14:18 ` [PATCH 1/3] block: use int64_t as bytes type in tracked requests Vladimir Sementsov-Ogievskiy
2020-04-22 15:37   ` Stefan Hajnoczi
2020-04-23 15:25   ` Kevin Wolf
2020-03-30 14:18 ` [PATCH 2/3] block/io: convert generic io path to use int64_t parameters Vladimir Sementsov-Ogievskiy
2020-04-22 15:50   ` Stefan Hajnoczi
2020-04-22 17:45     ` Vladimir Sementsov-Ogievskiy
2020-03-30 14:18 ` [PATCH 3/3] block: use int64_t instead of uint64_t in driver handlers Vladimir Sementsov-Ogievskiy
2020-03-30 17:43 ` [RFC 0/3] 64bit block-layer part I no-reply
2020-03-30 17:48 ` no-reply
2020-03-30 17:50 ` no-reply
2020-04-22 14:29 ` Vladimir Sementsov-Ogievskiy
2020-04-22 14:52   ` Eric Blake
2020-04-22 15:53 ` Stefan Hajnoczi
2020-04-22 18:24 ` Vladimir Sementsov-Ogievskiy
2020-04-22 19:32   ` Eric Blake
2020-04-23 15:43 ` Kevin Wolf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.