* [PATCH v4 00/11] 64bit block-layer: part II
@ 2021-03-24 20:51 Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 01/11] block/io: bring request check to bdrv_co_{read, write}v_vmstate Vladimir Sementsov-Ogievskiy via
` (11 more replies)
0 siblings, 12 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-24 20:51 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, vsementsov, jsnow
Hi all!
Here is part II of 64bit block-layer, when part I already landed
([PATCH v4 00/16] 64bit block-layer: part I
<20201211183934.169161-1-vsementsov@virtuozzo.com>)
This is called v4 too, because it follows old
([PATCH v3 00/17] 64bit block-layer
<20200430111033.29980-1-vsementsov@virtuozzo.com>)
Make patchew consider the old series obsolete:
Supersedes: <20200430111033.29980-1-vsementsov@virtuozzo.com>
(still actually, it is split into parts. And one more part that will
update blk layer is still needed.)
The old series is so old that I don't want to compare :) I've changed a
lot of things and new patches added.
part II aims to update block drivers to int64_t.
I remind that main aim of this update of the whole block-layer to 64bit
is to implement 64bit write-zeroes NBD request.
Vladimir Sementsov-Ogievskiy (11):
block/io: bring request check to bdrv_co_{read,write}v_vmstate
qcow2: check request on vmstate save/load path
block: use int64_t instead of uint64_t in driver read handlers
block: use int64_t instead of uint64_t in driver write handlers
block: use int64_t instead of uint64_t in copy_range driver handlers
block: make BlockLimits::max_pwrite_zeroes 64bit
block: use int64_t instead of int in driver write_zeroes handlers
block/io: allow 64bit write-zeroes requests
block: make BlockLimits::max_pdiscard 64bit
block: use int64_t instead of int in driver discard handlers
block/io: allow 64bit discard requests
include/block/block_int.h | 62 ++++++++++++++--------------
block/backup-top.c | 14 +++----
block/blkdebug.c | 12 +++---
block/blklogwrites.c | 16 ++++----
block/blkreplay.c | 8 ++--
block/blkverify.c | 8 ++--
block/bochs.c | 4 +-
block/cloop.c | 4 +-
block/commit.c | 2 +-
block/copy-on-read.c | 19 +++++----
block/crypto.c | 8 ++--
block/curl.c | 3 +-
block/dmg.c | 4 +-
block/file-posix.c | 35 ++++++++--------
block/file-win32.c | 8 ++--
block/filter-compress.c | 15 +++----
block/gluster.c | 13 +++---
block/io.c | 44 +++++++++++++++-----
block/iscsi.c | 53 ++++++++++++++----------
block/mirror.c | 8 ++--
block/nbd.c | 22 ++++++----
block/nfs.c | 12 +++---
block/null.c | 18 ++++----
block/nvme.c | 47 +++++++++++++++++----
block/preallocate.c | 14 +++----
block/qcow.c | 16 ++++----
block/qcow2-cluster.c | 14 ++++++-
block/qcow2.c | 70 +++++++++++++++++++++++---------
block/qed.c | 9 +++-
block/quorum.c | 11 ++---
block/raw-format.c | 36 ++++++++--------
block/rbd.c | 10 +++--
block/sheepdog.c | 15 ++++++-
block/throttle.c | 18 ++++----
block/vdi.c | 8 ++--
block/vmdk.c | 14 +++----
block/vpc.c | 8 ++--
block/vvfat.c | 12 +++---
tests/unit/test-bdrv-drain.c | 16 ++++----
tests/unit/test-block-iothread.c | 21 +++++++---
block/trace-events | 10 ++---
41 files changed, 451 insertions(+), 290 deletions(-)
--
2.29.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 01/11] block/io: bring request check to bdrv_co_{read, write}v_vmstate
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
@ 2021-03-24 20:51 ` Vladimir Sementsov-Ogievskiy via
2021-05-11 17:54 ` [PATCH v4 01/11] block/io: bring request check to bdrv_co_{read,write}v_vmstate Eric Blake
2021-03-24 20:51 ` [PATCH v4 02/11] qcow2: check request on vmstate save/load path Vladimir Sementsov-Ogievskiy
` (10 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy via @ 2021-03-24 20:51 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, vsementsov, jsnow
There are only two drivers supporting vmstate: qcow2 and sheepdog.
Sheepdog is deprecated. In qcow2 these requests go through
.bdrv_co_p{read,write}v_part handlers.
So, let's do our basic check for the request on vmstate generic
handlers.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/block/io.c b/block/io.c
index ca2dca3007..3bbb852da6 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2718,7 +2718,12 @@ bdrv_co_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
{
BlockDriver *drv = bs->drv;
BlockDriverState *child_bs = bdrv_primary_bs(bs);
- int ret = -ENOTSUP;
+ int ret;
+
+ ret = bdrv_check_qiov_request(pos, qiov->size, qiov, 0, NULL);
+ if (ret < 0) {
+ return ret;
+ }
if (!drv) {
return -ENOMEDIUM;
@@ -2730,6 +2735,8 @@ bdrv_co_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
ret = drv->bdrv_load_vmstate(bs, qiov, pos);
} else if (child_bs) {
ret = bdrv_co_readv_vmstate(child_bs, qiov, pos);
+ } else {
+ ret = -ENOTSUP;
}
bdrv_dec_in_flight(bs);
@@ -2742,7 +2749,12 @@ bdrv_co_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
{
BlockDriver *drv = bs->drv;
BlockDriverState *child_bs = bdrv_primary_bs(bs);
- int ret = -ENOTSUP;
+ int ret;
+
+ ret = bdrv_check_qiov_request(pos, qiov->size, qiov, 0, NULL);
+ if (ret < 0) {
+ return ret;
+ }
if (!drv) {
return -ENOMEDIUM;
@@ -2754,6 +2766,8 @@ bdrv_co_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
ret = drv->bdrv_save_vmstate(bs, qiov, pos);
} else if (child_bs) {
ret = bdrv_co_writev_vmstate(child_bs, qiov, pos);
+ } else {
+ ret = -ENOTSUP;
}
bdrv_dec_in_flight(bs);
--
2.29.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 02/11] qcow2: check request on vmstate save/load path
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 01/11] block/io: bring request check to bdrv_co_{read, write}v_vmstate Vladimir Sementsov-Ogievskiy via
@ 2021-03-24 20:51 ` Vladimir Sementsov-Ogievskiy
2021-05-11 17:57 ` Eric Blake
2021-03-24 20:51 ` [PATCH v4 03/11] block: use int64_t instead of uint64_t in driver read handlers Vladimir Sementsov-Ogievskiy
` (9 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-24 20:51 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, vsementsov, jsnow
We modify the request by adding an offset to vmstate. Let's check the
modified request. It will help us to safely move .bdrv_co_preadv_part
and .bdrv_co_pwritev_part to int64_t type of offset and bytes.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block_int.h | 3 +++
block/io.c | 6 +++---
block/qcow2.c | 43 +++++++++++++++++++++++++++++++++------
3 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 88e4111939..db7a909ea9 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -93,6 +93,9 @@ typedef struct BdrvTrackedRequest {
struct BdrvTrackedRequest *waiting_for;
} BdrvTrackedRequest;
+int bdrv_check_qiov_request(int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, size_t qiov_offset,
+ Error **errp);
int bdrv_check_request(int64_t offset, int64_t bytes, Error **errp);
struct BlockDriver {
diff --git a/block/io.c b/block/io.c
index 3bbb852da6..59924867c5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -924,9 +924,9 @@ bool coroutine_fn bdrv_make_request_serialising(BdrvTrackedRequest *req,
return waited;
}
-static int bdrv_check_qiov_request(int64_t offset, int64_t bytes,
- QEMUIOVector *qiov, size_t qiov_offset,
- Error **errp)
+int bdrv_check_qiov_request(int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, size_t qiov_offset,
+ Error **errp)
{
/*
* Check generic offset/bytes correctness
diff --git a/block/qcow2.c b/block/qcow2.c
index 0db1227ac9..b57acda010 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5166,24 +5166,55 @@ static int qcow2_has_zero_init(BlockDriverState *bs)
}
}
+/*
+ * Check the request to vmstate. On success return
+ * qcow2_vm_state_offset(bs) + @pos
+ */
+static int64_t qcow2_check_vmstate_request(BlockDriverState *bs,
+ QEMUIOVector *qiov, int64_t pos)
+{
+ BDRVQcow2State *s = bs->opaque;
+ int64_t vmstate_offset = qcow2_vm_state_offset(s);
+ int ret;
+
+ /* Incoming requests must be OK */
+ bdrv_check_qiov_request(pos, qiov->size, qiov, 0, &error_abort);
+
+ if (INT64_MAX - pos < vmstate_offset) {
+ return -EIO;
+ }
+
+ pos += vmstate_offset;
+ ret = bdrv_check_qiov_request(pos, qiov->size, qiov, 0, NULL);
+ if (ret < 0) {
+ return ret;
+ }
+
+ return pos;
+}
+
static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
int64_t pos)
{
- BDRVQcow2State *s = bs->opaque;
+ int64_t offset = qcow2_check_vmstate_request(bs, qiov, pos);
+ if (offset < 0) {
+ return offset;
+ }
BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_SAVE);
- return bs->drv->bdrv_co_pwritev_part(bs, qcow2_vm_state_offset(s) + pos,
- qiov->size, qiov, 0, 0);
+ return bs->drv->bdrv_co_pwritev_part(bs, offset, qiov->size, qiov, 0, 0);
}
static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
int64_t pos)
{
- BDRVQcow2State *s = bs->opaque;
+ int64_t offset = qcow2_check_vmstate_request(bs, qiov, pos);
+ if (offset < 0) {
+ return offset;
+ }
BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_LOAD);
- return bs->drv->bdrv_co_preadv_part(bs, qcow2_vm_state_offset(s) + pos,
- qiov->size, qiov, 0, 0);
+ return bs->drv->bdrv_co_preadv_part(bs, offset, qiov->size, qiov, 0, 0);
}
/*
--
2.29.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 03/11] block: use int64_t instead of uint64_t in driver read handlers
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 01/11] block/io: bring request check to bdrv_co_{read, write}v_vmstate Vladimir Sementsov-Ogievskiy via
2021-03-24 20:51 ` [PATCH v4 02/11] qcow2: check request on vmstate save/load path Vladimir Sementsov-Ogievskiy
@ 2021-03-24 20:51 ` Vladimir Sementsov-Ogievskiy
2021-05-11 19:22 ` Eric Blake
2021-03-24 20:51 ` [PATCH v4 04/11] block: use int64_t instead of uint64_t in driver write handlers Vladimir Sementsov-Ogievskiy
` (8 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-24 20:51 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, vsementsov, jsnow
We are going to convert .bdrv_co_preadv_part and .bdrv_co_pwritev_part
to int64_t type for offset and bytes parameters (as it's already done
for generic block/io.c layer).
In qcow2 .bdrv_co_preadv_part is used in some places, so let's add
corresponding checks and assertions.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
block: use int64_t instead of uint64_t in driver read handlers
We are generally moving to int64_t for both offset and bytes parameters
on all io paths.
Main motivation is realization of 64-bit write_zeroes operation for
fast zeroing large disk chunks, up to the whole disk.
We chose signed type, to be consistent with off_t (which is signed) and
with possibility for signed return type (where negative value means
error).
So, convert driver read handlers parameters which are already 64bit to
signed type.
While being here, convert also flags parameter to be BdrvRequestFlags.
Now let's consider all callers. Simple
git grep '\->bdrv_\(aio\|co\)_preadv\(_part\)\?'
shows that's there three callers of driver function:
bdrv_driver_preadv() in block/io.c, passes int64_t, checked by
bdrv_check_qiov_request() to be non-negative.
qcow2_load_vmstate() does bdrv_check_qiov_request().
do_perform_cow_read() has uint64_t argument. And a lot of things in
qcow2 driver are uint64_t, so converting it is big job. But we must
not work with requests that doesn't satisfy bdrv_check_qiov_request(),
so let's just assert it here.
Still, the functions may be called directly, not only by drv->...
Let's check:
git grep '\.bdrv_\(aio\|co\)_preadv\(_part\)\?\s*=' | \
awk '{print $4}' | sed 's/,//' | sed 's/&//' | sort | uniq | \
while read func; do git grep "$func(" | \
grep -v "$func(BlockDriverState"; done
The only one such caller:
QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, &data, 1);
...
ret = bdrv_replace_test_co_preadv(bs, 0, 1, &qiov, 0);
in tesTS/unit/test-bdrv-drain.c, and it's OK obviously.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block_int.h | 11 ++++++-----
block/backup-top.c | 4 ++--
block/blkdebug.c | 4 ++--
block/blklogwrites.c | 4 ++--
block/blkreplay.c | 2 +-
block/blkverify.c | 4 ++--
block/bochs.c | 4 ++--
block/cloop.c | 4 ++--
block/commit.c | 2 +-
block/copy-on-read.c | 4 ++--
block/crypto.c | 4 ++--
block/curl.c | 3 ++-
block/dmg.c | 4 ++--
block/file-posix.c | 6 +++---
block/file-win32.c | 4 ++--
block/filter-compress.c | 4 ++--
block/mirror.c | 2 +-
block/nbd.c | 5 +++--
block/nfs.c | 6 +++---
block/null.c | 9 +++++----
block/nvme.c | 5 +++--
block/preallocate.c | 4 ++--
block/qcow.c | 6 +++---
block/qcow2-cluster.c | 14 +++++++++++++-
block/qcow2.c | 5 +++--
block/quorum.c | 4 ++--
block/raw-format.c | 20 ++++++++++----------
block/rbd.c | 5 +++--
block/throttle.c | 5 +++--
block/vdi.c | 4 ++--
block/vmdk.c | 4 ++--
block/vpc.c | 4 ++--
block/vvfat.c | 4 ++--
tests/unit/test-bdrv-drain.c | 16 +++++++++-------
tests/unit/test-block-iothread.c | 19 ++++++++++++++-----
35 files changed, 120 insertions(+), 89 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index db7a909ea9..e6bcf74e46 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -234,8 +234,8 @@ struct BlockDriver {
/* aio */
BlockAIOCB *(*bdrv_aio_preadv)(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
- BlockCompletionFunc *cb, void *opaque);
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs,
uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
BlockCompletionFunc *cb, void *opaque);
@@ -264,10 +264,11 @@ struct BlockDriver {
* The buffer in @qiov may point directly to guest memory.
*/
int coroutine_fn (*bdrv_co_preadv)(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags);
int coroutine_fn (*bdrv_co_preadv_part)(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, size_t qiov_offset, int flags);
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int flags);
/**
diff --git a/block/backup-top.c b/block/backup-top.c
index 589e8b651d..9779493a4b 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -42,8 +42,8 @@ typedef struct BDRVBackupTopState {
} BDRVBackupTopState;
static coroutine_fn int backup_top_co_preadv(
- BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+ BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BDRVBackupTopState *s = bs->opaque;
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 2c0b9b0ee8..b26fecf222 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -614,8 +614,8 @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
}
static int coroutine_fn
-blkdebug_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+blkdebug_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
int err;
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index b7579370a3..de3d4ba2b6 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -301,8 +301,8 @@ static void blk_log_writes_refresh_limits(BlockDriverState *bs, Error **errp)
}
static int coroutine_fn
-blk_log_writes_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+blk_log_writes_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
}
diff --git a/block/blkreplay.c b/block/blkreplay.c
index 4a247752fd..13ea2166bb 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -72,7 +72,7 @@ static void block_request_create(uint64_t reqid, BlockDriverState *bs,
}
static int coroutine_fn blkreplay_co_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags)
{
uint64_t reqid = blkreplay_next_id();
int ret = bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
diff --git a/block/blkverify.c b/block/blkverify.c
index 188d7632fa..5e35744b8a 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -221,8 +221,8 @@ blkverify_co_prwv(BlockDriverState *bs, BlkverifyRequest *r, uint64_t offset,
}
static int coroutine_fn
-blkverify_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+blkverify_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BlkverifyRequest r;
QEMUIOVector raw_qiov;
diff --git a/block/bochs.c b/block/bochs.c
index 2f010ab40a..4d68658087 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -238,8 +238,8 @@ static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
}
static int coroutine_fn
-bochs_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+bochs_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BDRVBochsState *s = bs->opaque;
uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
diff --git a/block/cloop.c b/block/cloop.c
index c99192a57f..b8c6d0eccd 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -245,8 +245,8 @@ static inline int cloop_read_block(BlockDriverState *bs, int block_num)
}
static int coroutine_fn
-cloop_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+cloop_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BDRVCloopState *s = bs->opaque;
uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
diff --git a/block/commit.c b/block/commit.c
index dd9ba87349..04a1bb43c1 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -212,7 +212,7 @@ static const BlockJobDriver commit_job_driver = {
};
static int coroutine_fn bdrv_commit_top_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags)
{
return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
}
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 9cad9e1b8c..afb2a77b08 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -141,10 +141,10 @@ static int64_t cor_getlength(BlockDriverState *bs)
static int coroutine_fn cor_co_preadv_part(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
+ int64_t offset, int64_t bytes,
QEMUIOVector *qiov,
size_t qiov_offset,
- int flags)
+ BdrvRequestFlags flags)
{
int64_t n;
int local_flags;
diff --git a/block/crypto.c b/block/crypto.c
index 1d30fde38e..a732a36d10 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -397,8 +397,8 @@ static int block_crypto_reopen_prepare(BDRVReopenState *state,
#define BLOCK_CRYPTO_MAX_IO_SIZE (1024 * 1024)
static coroutine_fn int
-block_crypto_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+block_crypto_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BlockCrypto *crypto = bs->opaque;
uint64_t cur_bytes; /* number of bytes in current iteration */
diff --git a/block/curl.c b/block/curl.c
index 50e741a0d7..4a8ae2b269 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -896,7 +896,8 @@ out:
}
static int coroutine_fn curl_co_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
CURLAIOCB acb = {
.co = qemu_coroutine_self(),
diff --git a/block/dmg.c b/block/dmg.c
index ef35a505f2..447901fbb8 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -689,8 +689,8 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
}
static int coroutine_fn
-dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+dmg_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BDRVDMGState *s = bs->opaque;
uint64_t sector_num = offset >> BDRV_SECTOR_BITS;
diff --git a/block/file-posix.c b/block/file-posix.c
index 20e14f8e96..25da79cdd5 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2033,9 +2033,9 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
return raw_thread_pool_submit(bs, handle_aiocb_rw, &acb);
}
-static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov,
- int flags)
+static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_READ);
}
diff --git a/block/file-win32.c b/block/file-win32.c
index 2642088bd6..15076350f2 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -436,8 +436,8 @@ fail:
}
static BlockAIOCB *raw_aio_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags,
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque)
{
BDRVRawState *s = bs->opaque;
diff --git a/block/filter-compress.c b/block/filter-compress.c
index 5136371bf8..54a16c6c64 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -63,10 +63,10 @@ static int64_t compress_getlength(BlockDriverState *bs)
static int coroutine_fn compress_co_preadv_part(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
+ int64_t offset, int64_t bytes,
QEMUIOVector *qiov,
size_t qiov_offset,
- int flags)
+ BdrvRequestFlags flags)
{
return bdrv_co_preadv_part(bs->file, offset, bytes, qiov, qiov_offset,
flags);
diff --git a/block/mirror.c b/block/mirror.c
index 6af02a57c4..1587cda13c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1381,7 +1381,7 @@ static void coroutine_fn active_write_settle(MirrorOp *op)
}
static int coroutine_fn bdrv_mirror_top_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags)
{
return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
}
diff --git a/block/nbd.c b/block/nbd.c
index c26dc5a54f..a2b6aa1bca 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1534,8 +1534,9 @@ static int nbd_co_request(BlockDriverState *bs, NBDRequest *request,
return ret ? ret : request_ret;
}
-static int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov, int flags)
+static int nbd_client_co_preadv(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
int ret, request_ret;
Error *local_err = NULL;
diff --git a/block/nfs.c b/block/nfs.c
index 8c1968bb41..eee8f706ba 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -265,9 +265,9 @@ nfs_co_generic_cb(int ret, struct nfs_context *nfs, void *data,
nfs_co_generic_bh_cb, task);
}
-static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *iov,
- int flags)
+static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *iov,
+ BdrvRequestFlags flags)
{
NFSClient *client = bs->opaque;
NFSRPC task;
diff --git a/block/null.c b/block/null.c
index cc9b1d4ea7..343dbb580d 100644
--- a/block/null.c
+++ b/block/null.c
@@ -116,8 +116,9 @@ static coroutine_fn int null_co_common(BlockDriverState *bs)
}
static coroutine_fn int null_co_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
BDRVNullState *s = bs->opaque;
@@ -187,8 +188,8 @@ static inline BlockAIOCB *null_aio_common(BlockDriverState *bs,
}
static BlockAIOCB *null_aio_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags,
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags,
BlockCompletionFunc *cb,
void *opaque)
{
diff --git a/block/nvme.c b/block/nvme.c
index 2b5421e7aa..58342c6409 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1221,8 +1221,9 @@ static int nvme_co_prw(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
}
static coroutine_fn int nvme_co_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
return nvme_co_prw(bs, offset, bytes, qiov, false, flags);
}
diff --git a/block/preallocate.c b/block/preallocate.c
index b619206304..5709443612 100644
--- a/block/preallocate.c
+++ b/block/preallocate.c
@@ -227,8 +227,8 @@ static void preallocate_reopen_abort(BDRVReopenState *state)
}
static coroutine_fn int preallocate_co_preadv_part(
- BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, size_t qiov_offset, int flags)
+ BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags)
{
return bdrv_co_preadv_part(bs->file, offset, bytes, qiov, qiov_offset,
flags);
diff --git a/block/qcow.c b/block/qcow.c
index f8919a44d1..1151b8d79b 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -617,9 +617,9 @@ static void qcow_refresh_limits(BlockDriverState *bs, Error **errp)
bs->bl.request_alignment = BDRV_SECTOR_SIZE;
}
-static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov,
- int flags)
+static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
BDRVQcowState *s = bs->opaque;
int offset_in_cluster;
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index bd0597842f..70adea9f59 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -505,7 +505,19 @@ static int coroutine_fn do_perform_cow_read(BlockDriverState *bs,
return -ENOMEDIUM;
}
- /* Call .bdrv_co_readv() directly instead of using the public block-layer
+ /*
+ * We never deal with requests that doesn't satisfy
+ * bdrv_check_qiov_request(), and aligning requests to clusters never break
+ * this condition. So, do some assertions before calling
+ * bs->drv->bdrv_co_preadv_part() which has int64_t arguments.
+ */
+ assert(src_cluster_offset <= INT64_MAX);
+ assert(src_cluster_offset + offset_in_cluster <= INT64_MAX);
+ assert(qiov->size <= INT64_MAX);
+ bdrv_check_qiov_request(src_cluster_offset + offset_in_cluster, qiov->size,
+ qiov, 0, &error_abort);
+ /*
+ * Call .bdrv_co_readv() directly instead of using the public block-layer
* interface. This avoids double I/O throttling and request tracking,
* which can lead to deadlock when block layer copy-on-read is enabled.
*/
diff --git a/block/qcow2.c b/block/qcow2.c
index b57acda010..34c5e1c073 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2281,9 +2281,10 @@ static coroutine_fn int qcow2_co_preadv_task_entry(AioTask *task)
}
static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
+ int64_t offset, int64_t bytes,
QEMUIOVector *qiov,
- size_t qiov_offset, int flags)
+ size_t qiov_offset,
+ BdrvRequestFlags flags)
{
BDRVQcow2State *s = bs->opaque;
int ret = 0;
diff --git a/block/quorum.c b/block/quorum.c
index cfc1436abb..1eba2ce7ff 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -663,8 +663,8 @@ static int read_fifo_child(QuorumAIOCB *acb)
return ret;
}
-static int quorum_co_preadv(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov, int flags)
+static int quorum_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BDRVQuorumState *s = bs->opaque;
QuorumAIOCB *acb = quorum_aio_get(bs, qiov, offset, bytes, flags);
diff --git a/block/raw-format.c b/block/raw-format.c
index 7717578ed6..e3f459b2cb 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -181,8 +181,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;
@@ -201,9 +201,9 @@ static inline int raw_adjust_offset(BlockDriverState *bs, uint64_t *offset,
return 0;
}
-static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov,
- int flags)
+static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
int ret;
@@ -259,7 +259,7 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
qiov = &local_qiov;
}
- ret = raw_adjust_offset(bs, &offset, bytes, true);
+ ret = raw_adjust_offset(bs, (int64_t *)&offset, bytes, true);
if (ret) {
goto fail;
}
@@ -294,7 +294,7 @@ static int coroutine_fn raw_co_pwrite_zeroes(BlockDriverState *bs,
{
int ret;
- ret = raw_adjust_offset(bs, (uint64_t *)&offset, bytes, true);
+ ret = raw_adjust_offset(bs, (int64_t *)&offset, bytes, true);
if (ret) {
return ret;
}
@@ -306,7 +306,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;
}
@@ -541,7 +541,7 @@ static int coroutine_fn raw_co_copy_range_from(BlockDriverState *bs,
{
int ret;
- ret = raw_adjust_offset(bs, &src_offset, bytes, false);
+ ret = raw_adjust_offset(bs, (int64_t *)&src_offset, bytes, false);
if (ret) {
return ret;
}
@@ -560,7 +560,7 @@ static int coroutine_fn raw_co_copy_range_to(BlockDriverState *bs,
{
int ret;
- ret = raw_adjust_offset(bs, &dst_offset, bytes, true);
+ ret = raw_adjust_offset(bs, (int64_t *)&dst_offset, bytes, true);
if (ret) {
return ret;
}
diff --git a/block/rbd.c b/block/rbd.c
index 9071a00e3f..69299e5f6f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -992,8 +992,9 @@ failed:
}
static BlockAIOCB *qemu_rbd_aio_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags,
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov,
+ BdrvRequestFlags flags,
BlockCompletionFunc *cb,
void *opaque)
{
diff --git a/block/throttle.c b/block/throttle.c
index b685166ad4..20362b5fe5 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -112,8 +112,9 @@ static int64_t throttle_getlength(BlockDriverState *bs)
}
static int coroutine_fn throttle_co_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
ThrottleGroupMember *tgm = bs->opaque;
diff --git a/block/vdi.c b/block/vdi.c
index 5627e7d764..c0e71bfa53 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -544,8 +544,8 @@ static int coroutine_fn vdi_co_block_status(BlockDriverState *bs,
}
static int coroutine_fn
-vdi_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+vdi_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BDRVVdiState *s = bs->opaque;
QEMUIOVector local_qiov;
diff --git a/block/vmdk.c b/block/vmdk.c
index 4499f136bd..78592160d0 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1888,8 +1888,8 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
}
static int coroutine_fn
-vmdk_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+vmdk_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BDRVVmdkState *s = bs->opaque;
int ret;
diff --git a/block/vpc.c b/block/vpc.c
index 17a705b482..29c8517ff8 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -608,8 +608,8 @@ static int vpc_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
}
static int coroutine_fn
-vpc_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+vpc_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BDRVVPCState *s = bs->opaque;
int ret;
diff --git a/block/vvfat.c b/block/vvfat.c
index 54807f82ca..8ac90166c5 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1522,8 +1522,8 @@ static int vvfat_read(BlockDriverState *bs, int64_t sector_num,
}
static int coroutine_fn
-vvfat_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+vvfat_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
int ret;
BDRVVVFATState *s = bs->opaque;
diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index 8a29e33e00..2627d9781e 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -65,8 +65,9 @@ static void co_reenter_bh(void *opaque)
}
static int coroutine_fn bdrv_test_co_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
BDRVTestState *s = bs->opaque;
@@ -1105,8 +1106,9 @@ static void bdrv_test_top_close(BlockDriverState *bs)
}
static int coroutine_fn bdrv_test_top_co_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
BDRVTestTopState *tts = bs->opaque;
return bdrv_co_preadv(tts->wait_child, offset, bytes, qiov, flags);
@@ -1854,10 +1856,10 @@ static void bdrv_replace_test_close(BlockDriverState *bs)
* Set .has_read to true and return success.
*/
static int coroutine_fn bdrv_replace_test_co_preadv(BlockDriverState *bs,
- uint64_t offset,
- uint64_t bytes,
+ int64_t offset,
+ int64_t bytes,
QEMUIOVector *qiov,
- int flags)
+ BdrvRequestFlags flags)
{
BDRVReplaceTestState *s = bs->opaque;
diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
index 8cf172cb7a..b3f8f3ff8e 100644
--- a/tests/unit/test-block-iothread.c
+++ b/tests/unit/test-block-iothread.c
@@ -31,9 +31,18 @@
#include "qemu/main-loop.h"
#include "iothread.h"
-static int coroutine_fn bdrv_test_co_prwv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+static int coroutine_fn bdrv_test_co_preadv(BlockDriverState *bs,
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
+{
+ return 0;
+}
+
+static int coroutine_fn bdrv_test_co_pwritev(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov,
+ int flags)
{
return 0;
}
@@ -66,8 +75,8 @@ static BlockDriver bdrv_test = {
.format_name = "test",
.instance_size = 1,
- .bdrv_co_preadv = bdrv_test_co_prwv,
- .bdrv_co_pwritev = bdrv_test_co_prwv,
+ .bdrv_co_preadv = bdrv_test_co_preadv,
+ .bdrv_co_pwritev = bdrv_test_co_pwritev,
.bdrv_co_pdiscard = bdrv_test_co_pdiscard,
.bdrv_co_truncate = bdrv_test_co_truncate,
.bdrv_co_block_status = bdrv_test_co_block_status,
--
2.29.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 04/11] block: use int64_t instead of uint64_t in driver write handlers
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2021-03-24 20:51 ` [PATCH v4 03/11] block: use int64_t instead of uint64_t in driver read handlers Vladimir Sementsov-Ogievskiy
@ 2021-03-24 20:51 ` Vladimir Sementsov-Ogievskiy
2021-05-11 21:00 ` Eric Blake
2021-03-24 20:51 ` [PATCH v4 05/11] block: use int64_t instead of uint64_t in copy_range driver handlers Vladimir Sementsov-Ogievskiy
` (7 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-24 20:51 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, vsementsov, jsnow
We are generally moving to int64_t for both offset and bytes parameters
on all io paths.
Main motivation is realization of 64-bit write_zeroes operation for
fast zeroing large disk chunks, up to the whole disk.
We chose signed type, to be consistent with off_t (which is signed) and
with possibility for signed return type (where negative value means
error).
So, convert driver write handlers parameters which are already 64bit to
signed type.
While being here, convert also flags parameter to be BdrvRequestFlags.
Now let's consider all callers. Simple
git grep '\->bdrv_\(aio\|co\)_pwritev\(_part\)\?'
shows that's there two callers of driver function:
bdrv_driver_pwritev() in block/io.c, passes int64_t, checked by
bdrv_check_qiov_request() to be non-negative.
qcow2_save_vmstate() does bdrv_check_qiov_request().
Still, the functions may be called directly, not only by drv->...
Let's check:
git grep '\.bdrv_\(aio\|co\)_pwritev\(_part\)\?\s*=' | \
awk '{print $4}' | sed 's/,//' | sed 's/&//' | sort | uniq | \
while read func; do git grep "$func(" | \
grep -v "$func(BlockDriverState"; done
shows several callers:
qcow2:
qcow2_co_truncate() write at most up to @offset, which is checked in
generic qcow2_co_truncate() by bdrv_check_request().
qcow2_co_pwritev_compressed_task() pass the request (or part of the
request) that already went through normal write path, so it should
be OK
qcow:
qcow_co_pwritev_compressed() pass int64_t, it's updated by this patch
quorum:
quorum_co_pwrite_zeroes() pass int64_t and int - OK
throttle:
throttle_co_pwritev_compressed() pass int64_t, it's updated by this
patch
vmdk:
vmdk_co_pwritev_compressed() pass int64_t, it's updated by this
patch
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block_int.h | 16 ++++++++--------
block/backup-top.c | 6 +++---
block/blkdebug.c | 4 ++--
block/blklogwrites.c | 4 ++--
block/blkreplay.c | 2 +-
block/blkverify.c | 4 ++--
block/copy-on-read.c | 11 ++++++-----
block/crypto.c | 4 ++--
block/file-posix.c | 6 +++---
block/file-win32.c | 4 ++--
block/filter-compress.c | 7 ++++---
block/io.c | 6 ++++--
block/mirror.c | 2 +-
block/nbd.c | 5 +++--
block/nfs.c | 6 +++---
block/null.c | 9 +++++----
block/nvme.c | 4 ++--
block/preallocate.c | 6 +++---
block/qcow.c | 10 +++++-----
block/qcow2.c | 6 +++---
block/quorum.c | 5 +++--
block/raw-format.c | 10 +++++-----
block/rbd.c | 5 +++--
block/throttle.c | 9 +++++----
block/vdi.c | 4 ++--
block/vmdk.c | 8 ++++----
block/vpc.c | 4 ++--
block/vvfat.c | 8 ++++----
tests/unit/test-block-iothread.c | 4 ++--
block/trace-events | 2 +-
30 files changed, 95 insertions(+), 86 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index e6bcf74e46..928369e0bc 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -237,8 +237,8 @@ struct BlockDriver {
int64_t offset, int64_t bytes, QEMUIOVector *qiov,
BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
- BlockCompletionFunc *cb, void *opaque);
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
@@ -287,10 +287,11 @@ struct BlockDriver {
* The buffer in @qiov may point directly to guest memory.
*/
int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags);
int coroutine_fn (*bdrv_co_pwritev_part)(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, size_t qiov_offset, int flags);
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset,
+ BdrvRequestFlags flags);
/*
* Efficiently zero a region of the disk image. Typically an image format
@@ -428,10 +429,9 @@ struct BlockDriver {
Error **errp);
int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov);
int coroutine_fn (*bdrv_co_pwritev_compressed_part)(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov,
- size_t qiov_offset);
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset);
int (*bdrv_snapshot_create)(BlockDriverState *bs,
QEMUSnapshotInfo *sn_info);
diff --git a/block/backup-top.c b/block/backup-top.c
index 9779493a4b..aa27afbb1b 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -97,9 +97,9 @@ static int coroutine_fn backup_top_co_pwrite_zeroes(BlockDriverState *bs,
}
static coroutine_fn int backup_top_co_pwritev(BlockDriverState *bs,
- uint64_t offset,
- uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
int ret = backup_top_cbw(bs, offset, bytes, flags);
if (ret < 0) {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index b26fecf222..a7fedd80fd 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -635,8 +635,8 @@ blkdebug_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
}
static int coroutine_fn
-blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+blkdebug_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
int err;
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index de3d4ba2b6..ca174ab135 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -460,8 +460,8 @@ blk_log_writes_co_do_file_pdiscard(BlkLogWritesFileReq *fr)
}
static int coroutine_fn
-blk_log_writes_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+blk_log_writes_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
return blk_log_writes_co_log(bs, offset, bytes, qiov, flags,
blk_log_writes_co_do_file_pwritev, 0, false);
diff --git a/block/blkreplay.c b/block/blkreplay.c
index 13ea2166bb..7ba62dcac1 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -83,7 +83,7 @@ static int coroutine_fn blkreplay_co_preadv(BlockDriverState *bs,
}
static int coroutine_fn blkreplay_co_pwritev(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags)
{
uint64_t reqid = blkreplay_next_id();
int ret = bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
diff --git a/block/blkverify.c b/block/blkverify.c
index 5e35744b8a..d1facf5ba9 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -250,8 +250,8 @@ blkverify_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
}
static int coroutine_fn
-blkverify_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+blkverify_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BlkverifyRequest r;
return blkverify_co_prwv(bs, &r, offset, bytes, qiov, qiov, flags, true);
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index afb2a77b08..7675565080 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -194,10 +194,11 @@ static int coroutine_fn cor_co_preadv_part(BlockDriverState *bs,
static int coroutine_fn cor_co_pwritev_part(BlockDriverState *bs,
- uint64_t offset,
- uint64_t bytes,
+ int64_t offset,
+ int64_t bytes,
QEMUIOVector *qiov,
- size_t qiov_offset, int flags)
+ size_t qiov_offset,
+ BdrvRequestFlags flags)
{
return bdrv_co_pwritev_part(bs->file, offset, bytes, qiov, qiov_offset,
flags);
@@ -220,8 +221,8 @@ static int coroutine_fn cor_co_pdiscard(BlockDriverState *bs,
static int coroutine_fn cor_co_pwritev_compressed(BlockDriverState *bs,
- uint64_t offset,
- uint64_t bytes,
+ int64_t offset,
+ int64_t bytes,
QEMUIOVector *qiov)
{
return bdrv_co_pwritev(bs->file, offset, bytes, qiov,
diff --git a/block/crypto.c b/block/crypto.c
index a732a36d10..c8ba4681e2 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -460,8 +460,8 @@ block_crypto_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
static coroutine_fn int
-block_crypto_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+block_crypto_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BlockCrypto *crypto = bs->opaque;
uint64_t cur_bytes; /* number of bytes in current iteration */
diff --git a/block/file-posix.c b/block/file-posix.c
index 25da79cdd5..add56cab00 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2040,9 +2040,9 @@ static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_READ);
}
-static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov,
- int flags)
+static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
assert(flags == 0);
return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_WRITE);
diff --git a/block/file-win32.c b/block/file-win32.c
index 15076350f2..7c2706c329 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -451,8 +451,8 @@ static BlockAIOCB *raw_aio_preadv(BlockDriverState *bs,
}
static BlockAIOCB *raw_aio_pwritev(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags,
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags,
BlockCompletionFunc *cb, void *opaque)
{
BDRVRawState *s = bs->opaque;
diff --git a/block/filter-compress.c b/block/filter-compress.c
index 54a16c6c64..505822a44f 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -74,10 +74,11 @@ static int coroutine_fn compress_co_preadv_part(BlockDriverState *bs,
static int coroutine_fn compress_co_pwritev_part(BlockDriverState *bs,
- uint64_t offset,
- uint64_t bytes,
+ int64_t offset,
+ int64_t bytes,
QEMUIOVector *qiov,
- size_t qiov_offset, int flags)
+ size_t qiov_offset,
+ BdrvRequestFlags flags)
{
return bdrv_co_pwritev_part(bs->file, offset, bytes, qiov, qiov_offset,
flags | BDRV_REQ_WRITE_COMPRESSED);
diff --git a/block/io.c b/block/io.c
index 59924867c5..55095dd08e 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1198,7 +1198,8 @@ out:
static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
int64_t offset, int64_t bytes,
QEMUIOVector *qiov,
- size_t qiov_offset, int flags)
+ size_t qiov_offset,
+ BdrvRequestFlags flags)
{
BlockDriver *drv = bs->drv;
int64_t sector_num;
@@ -2037,7 +2038,8 @@ bdrv_co_write_req_finish(BdrvChild *child, int64_t offset, int64_t bytes,
*/
static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
BdrvTrackedRequest *req, int64_t offset, int64_t bytes,
- int64_t align, QEMUIOVector *qiov, size_t qiov_offset, int flags)
+ int64_t align, QEMUIOVector *qiov, size_t qiov_offset,
+ BdrvRequestFlags flags)
{
BlockDriverState *bs = child->bs;
BlockDriver *drv = bs->drv;
diff --git a/block/mirror.c b/block/mirror.c
index 1587cda13c..f7b7f4d566 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1435,7 +1435,7 @@ out:
}
static int coroutine_fn bdrv_mirror_top_pwritev(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags)
{
MirrorBDSOpaque *s = bs->opaque;
QEMUIOVector bounce_qiov;
diff --git a/block/nbd.c b/block/nbd.c
index a2b6aa1bca..34cb28927b 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1593,8 +1593,9 @@ static int nbd_client_co_preadv(BlockDriverState *bs, int64_t offset,
return ret ? ret : request_ret;
}
-static int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov, int flags)
+static int nbd_client_co_pwritev(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
NBDRequest request = {
diff --git a/block/nfs.c b/block/nfs.c
index eee8f706ba..c6563930cf 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -299,9 +299,9 @@ static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, int64_t offset,
return 0;
}
-static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *iov,
- int flags)
+static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *iov,
+ BdrvRequestFlags flags)
{
NFSClient *client = bs->opaque;
NFSRPC task;
diff --git a/block/null.c b/block/null.c
index 343dbb580d..75f7d0db40 100644
--- a/block/null.c
+++ b/block/null.c
@@ -130,8 +130,9 @@ static coroutine_fn int null_co_preadv(BlockDriverState *bs,
}
static coroutine_fn int null_co_pwritev(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
return null_co_common(bs);
}
@@ -203,8 +204,8 @@ static BlockAIOCB *null_aio_preadv(BlockDriverState *bs,
}
static BlockAIOCB *null_aio_pwritev(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags,
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags,
BlockCompletionFunc *cb,
void *opaque)
{
diff --git a/block/nvme.c b/block/nvme.c
index 58342c6409..1a128ef95e 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1229,8 +1229,8 @@ static coroutine_fn int nvme_co_preadv(BlockDriverState *bs,
}
static coroutine_fn int nvme_co_pwritev(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
return nvme_co_prw(bs, offset, bytes, qiov, true, flags);
}
diff --git a/block/preallocate.c b/block/preallocate.c
index 5709443612..c19885af17 100644
--- a/block/preallocate.c
+++ b/block/preallocate.c
@@ -349,11 +349,11 @@ static int coroutine_fn preallocate_co_pwrite_zeroes(BlockDriverState *bs,
}
static coroutine_fn int preallocate_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)
+ BdrvRequestFlags flags)
{
handle_write(bs, offset, bytes, false);
diff --git a/block/qcow.c b/block/qcow.c
index 1151b8d79b..c39940f33e 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -714,9 +714,9 @@ static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, int64_t offset,
return ret;
}
-static coroutine_fn int qcow_co_pwritev(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov,
- int flags)
+static coroutine_fn int qcow_co_pwritev(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
BDRVQcowState *s = bs->opaque;
int offset_in_cluster;
@@ -1047,8 +1047,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 34c5e1c073..1e55ab52b3 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2568,8 +2568,8 @@ static coroutine_fn int qcow2_co_pwritev_task_entry(AioTask *task)
}
static coroutine_fn int qcow2_co_pwritev_part(
- BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, size_t qiov_offset, int flags)
+ BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags)
{
BDRVQcow2State *s = bs->opaque;
int offset_in_cluster;
@@ -4568,7 +4568,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 1eba2ce7ff..353401ac08 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -714,8 +714,9 @@ static void write_quorum_entry(void *opaque)
}
}
-static int quorum_co_pwritev(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov, int flags)
+static int quorum_co_pwritev(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
BDRVQuorumState *s = bs->opaque;
QuorumAIOCB *acb = quorum_aio_get(bs, qiov, offset, bytes, flags);
diff --git a/block/raw-format.c b/block/raw-format.c
index e3f459b2cb..b0fe75f54a 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -216,9 +216,9 @@ static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
}
-static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov,
- int flags)
+static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
void *buf = NULL;
BlockDriver *drv;
@@ -259,7 +259,7 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
qiov = &local_qiov;
}
- ret = raw_adjust_offset(bs, (int64_t *)&offset, bytes, true);
+ ret = raw_adjust_offset(bs, &offset, bytes, true);
if (ret) {
goto fail;
}
@@ -294,7 +294,7 @@ static int coroutine_fn raw_co_pwrite_zeroes(BlockDriverState *bs,
{
int ret;
- ret = raw_adjust_offset(bs, (int64_t *)&offset, bytes, true);
+ ret = raw_adjust_offset(bs, &offset, bytes, true);
if (ret) {
return ret;
}
diff --git a/block/rbd.c b/block/rbd.c
index 69299e5f6f..598b801f96 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1003,8 +1003,9 @@ static BlockAIOCB *qemu_rbd_aio_preadv(BlockDriverState *bs,
}
static BlockAIOCB *qemu_rbd_aio_pwritev(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags,
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov,
+ BdrvRequestFlags flags,
BlockCompletionFunc *cb,
void *opaque)
{
diff --git a/block/throttle.c b/block/throttle.c
index 20362b5fe5..1330e844c3 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -124,8 +124,9 @@ static int coroutine_fn throttle_co_preadv(BlockDriverState *bs,
}
static int coroutine_fn throttle_co_pwritev(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+ int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
ThrottleGroupMember *tgm = bs->opaque;
throttle_group_co_io_limits_intercept(tgm, bytes, true);
@@ -153,8 +154,8 @@ static int coroutine_fn throttle_co_pdiscard(BlockDriverState *bs,
}
static int coroutine_fn throttle_co_pwritev_compressed(BlockDriverState *bs,
- uint64_t offset,
- uint64_t bytes,
+ int64_t offset,
+ int64_t bytes,
QEMUIOVector *qiov)
{
return throttle_co_pwritev(bs, offset, bytes, qiov,
diff --git a/block/vdi.c b/block/vdi.c
index c0e71bfa53..020f7a3cbc 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -600,8 +600,8 @@ vdi_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
}
static int coroutine_fn
-vdi_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+vdi_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BDRVVdiState *s = bs->opaque;
QEMUIOVector local_qiov;
diff --git a/block/vmdk.c b/block/vmdk.c
index 78592160d0..8d49e54bdd 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2068,8 +2068,8 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
}
static int coroutine_fn
-vmdk_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+vmdk_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
int ret;
BDRVVmdkState *s = bs->opaque;
@@ -2080,8 +2080,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 29c8517ff8..1b4c7333af 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -658,8 +658,8 @@ fail:
}
static int coroutine_fn
-vpc_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+vpc_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
BDRVVPCState *s = bs->opaque;
int64_t image_offset;
diff --git a/block/vvfat.c b/block/vvfat.c
index 8ac90166c5..0b6ed1c6d1 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3061,8 +3061,8 @@ DLOG(checkpoint());
}
static int coroutine_fn
-vvfat_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+vvfat_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
int ret;
BDRVVVFATState *s = bs->opaque;
@@ -3099,8 +3099,8 @@ static int coroutine_fn vvfat_co_block_status(BlockDriverState *bs,
}
static int coroutine_fn
-write_target_commit(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+write_target_commit(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
{
int ret;
diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
index b3f8f3ff8e..50b8718b2a 100644
--- a/tests/unit/test-block-iothread.c
+++ b/tests/unit/test-block-iothread.c
@@ -40,9 +40,9 @@ static int coroutine_fn bdrv_test_co_preadv(BlockDriverState *bs,
}
static int coroutine_fn bdrv_test_co_pwritev(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
+ int64_t offset, int64_t bytes,
QEMUIOVector *qiov,
- int flags)
+ BdrvRequestFlags flags)
{
return 0;
}
diff --git a/block/trace-events b/block/trace-events
index 1a12d634e2..3cbea9e2e2 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -75,7 +75,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 host_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.29.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 05/11] block: use int64_t instead of uint64_t in copy_range driver handlers
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
` (3 preceding siblings ...)
2021-03-24 20:51 ` [PATCH v4 04/11] block: use int64_t instead of uint64_t in driver write handlers Vladimir Sementsov-Ogievskiy
@ 2021-03-24 20:51 ` Vladimir Sementsov-Ogievskiy
2021-05-11 21:14 ` Eric Blake
2021-03-24 20:51 ` [PATCH v4 06/11] block: make BlockLimits::max_pwrite_zeroes 64bit Vladimir Sementsov-Ogievskiy
` (6 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-24 20:51 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, vsementsov, jsnow
We are generally moving to int64_t for both offset and bytes parameters
on all io paths.
Main motivation is realization of 64-bit write_zeroes operation for
fast zeroing large disk chunks, up to the whole disk.
We chose signed type, to be consistent with off_t (which is signed) and
with possibility for signed return type (where negative value means
error).
So, convert driver copy_range handlers parameters which are already
64bit to signed type.
Now let's consider all callers. Simple
git grep '\->bdrv_co_copy_range'
shows the only caller:
bdrv_co_copy_range_internal(), which doesn bdrv_check_request32(),
so everything is OK.
Still, the functions may be called directly, not only by drv->...
Let's check:
git grep '\.bdrv_co_copy_range_\(from\|to\)\s*=' | \
awk '{print $4}' | sed 's/,//' | sed 's/&//' | sort | uniq | \
while read func; do git grep "$func(" | \
grep -v "$func(BlockDriverState"; done
shows no more callers. So, we are done.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block_int.h | 12 ++++++------
block/file-posix.c | 10 +++++-----
block/iscsi.c | 12 ++++++------
block/qcow2.c | 12 ++++++------
block/raw-format.c | 16 ++++++++--------
5 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 928369e0bc..88b19db756 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -313,10 +313,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);
@@ -330,10 +330,10 @@ struct BlockDriver {
*/
int coroutine_fn (*bdrv_co_copy_range_to)(BlockDriverState *bs,
BdrvChild *src,
- uint64_t src_offset,
+ int64_t src_offset,
BdrvChild *dst,
- uint64_t dst_offset,
- uint64_t bytes,
+ int64_t dst_offset,
+ int64_t bytes,
BdrvRequestFlags read_flags,
BdrvRequestFlags write_flags);
diff --git a/block/file-posix.c b/block/file-posix.c
index add56cab00..6faf7eb96e 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3167,8 +3167,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,
@@ -3177,10 +3177,10 @@ static int coroutine_fn raw_co_copy_range_from(
static int coroutine_fn raw_co_copy_range_to(BlockDriverState *bs,
BdrvChild *src,
- uint64_t src_offset,
+ int64_t src_offset,
BdrvChild *dst,
- uint64_t dst_offset,
- uint64_t bytes,
+ int64_t dst_offset,
+ int64_t bytes,
BdrvRequestFlags read_flags,
BdrvRequestFlags write_flags)
{
diff --git a/block/iscsi.c b/block/iscsi.c
index 4d2a416ce7..6bcde6ec6b 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2172,10 +2172,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)
{
@@ -2313,10 +2313,10 @@ static void iscsi_xcopy_data(struct iscsi_data *data,
static int coroutine_fn iscsi_co_copy_range_to(BlockDriverState *bs,
BdrvChild *src,
- uint64_t src_offset,
+ int64_t src_offset,
BdrvChild *dst,
- uint64_t dst_offset,
- uint64_t bytes,
+ int64_t dst_offset,
+ int64_t bytes,
BdrvRequestFlags read_flags,
BdrvRequestFlags write_flags)
{
diff --git a/block/qcow2.c b/block/qcow2.c
index 1e55ab52b3..c786601d52 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3975,9 +3975,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;
@@ -4058,9 +4058,9 @@ out:
static int coroutine_fn
qcow2_co_copy_range_to(BlockDriverState *bs,
- BdrvChild *src, uint64_t src_offset,
- BdrvChild *dst, uint64_t dst_offset,
- uint64_t bytes, BdrvRequestFlags read_flags,
+ BdrvChild *src, int64_t src_offset,
+ BdrvChild *dst, int64_t dst_offset,
+ int64_t bytes, BdrvRequestFlags read_flags,
BdrvRequestFlags write_flags)
{
BDRVQcow2State *s = bs->opaque;
diff --git a/block/raw-format.c b/block/raw-format.c
index b0fe75f54a..051b00a4d4 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -532,16 +532,16 @@ static int raw_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
static int coroutine_fn raw_co_copy_range_from(BlockDriverState *bs,
BdrvChild *src,
- uint64_t src_offset,
+ int64_t src_offset,
BdrvChild *dst,
- uint64_t dst_offset,
- uint64_t bytes,
+ int64_t dst_offset,
+ int64_t bytes,
BdrvRequestFlags read_flags,
BdrvRequestFlags write_flags)
{
int ret;
- ret = raw_adjust_offset(bs, (int64_t *)&src_offset, bytes, false);
+ ret = raw_adjust_offset(bs, &src_offset, bytes, false);
if (ret) {
return ret;
}
@@ -551,16 +551,16 @@ static int coroutine_fn raw_co_copy_range_from(BlockDriverState *bs,
static int coroutine_fn raw_co_copy_range_to(BlockDriverState *bs,
BdrvChild *src,
- uint64_t src_offset,
+ int64_t src_offset,
BdrvChild *dst,
- uint64_t dst_offset,
- uint64_t bytes,
+ int64_t dst_offset,
+ int64_t bytes,
BdrvRequestFlags read_flags,
BdrvRequestFlags write_flags)
{
int ret;
- ret = raw_adjust_offset(bs, (int64_t *)&dst_offset, bytes, true);
+ ret = raw_adjust_offset(bs, &dst_offset, bytes, true);
if (ret) {
return ret;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 06/11] block: make BlockLimits::max_pwrite_zeroes 64bit
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
` (4 preceding siblings ...)
2021-03-24 20:51 ` [PATCH v4 05/11] block: use int64_t instead of uint64_t in copy_range driver handlers Vladimir Sementsov-Ogievskiy
@ 2021-03-24 20:51 ` Vladimir Sementsov-Ogievskiy
2021-05-11 21:19 ` Eric Blake
2021-03-24 20:51 ` [PATCH v4 07/11] block: use int64_t instead of int in driver write_zeroes handlers Vladimir Sementsov-Ogievskiy
` (5 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-24 20:51 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, vsementsov, jsnow
We are going to support 64 bit write-zeroes requests. Now update the
limit variable. It's absolutely safe. The variable is set in some
drivers, and used in bdrv_co_do_pwrite_zeroes().
Update also max_write_zeroes variable in bdrv_co_do_pwrite_zeroes(), so
that bdrv_co_do_pwrite_zeroes() is now prepared to 64bit requests. The
remaining logic including num, offset and bytes variables is already
supporting 64bit requests.
So the only thing that prevents 64 bit requests is limiting
max_write_zeroes variable to INT_MAX in bdrv_co_do_pwrite_zeroes().
We'll drop this limitation after updating all block drivers.
Ah, we also have bdrv_check_request32() in bdrv_co_pwritev_part(). It
will be modified to do bdrv_check_request() for write-zeroes path.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block_int.h | 7 +++----
block/io.c | 2 +-
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 88b19db756..71e5025534 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -676,10 +676,9 @@ typedef struct BlockLimits {
* that is set. May be 0 if bl.request_alignment is good enough */
uint32_t pdiscard_alignment;
- /* Maximum number of bytes that can zeroized at once (since it is
- * signed, it must be < 2G, if set). Must be multiple of
- * pwrite_zeroes_alignment. May be 0 if no inherent 32-bit limit */
- int32_t max_pwrite_zeroes;
+ /* Maximum number of bytes that can zeroized at once. Must be multiple of
+ * pwrite_zeroes_alignment. May be 0 if no inherent 64-bit limit */
+ int64_t max_pwrite_zeroes;
/* Optimal alignment for write zeroes requests in bytes. A power
* of 2 is best but not mandatory. Must be a multiple of
diff --git a/block/io.c b/block/io.c
index 55095dd08e..79e600af27 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1836,7 +1836,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
int head = 0;
int tail = 0;
- int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX);
+ int64_t max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX);
int alignment = MAX(bs->bl.pwrite_zeroes_alignment,
bs->bl.request_alignment);
int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, MAX_BOUNCE_BUFFER);
--
2.29.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 07/11] block: use int64_t instead of int in driver write_zeroes handlers
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
` (5 preceding siblings ...)
2021-03-24 20:51 ` [PATCH v4 06/11] block: make BlockLimits::max_pwrite_zeroes 64bit Vladimir Sementsov-Ogievskiy
@ 2021-03-24 20:51 ` Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 08/11] block/io: allow 64bit write-zeroes requests Vladimir Sementsov-Ogievskiy
` (4 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-24 20:51 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, vsementsov, jsnow
We are generally moving to int64_t for both offset and bytes parameters
on all io paths.
Main motivation is realization of 64-bit write_zeroes operation for
fast zeroing large disk chunks, up to the whole disk.
We chose signed type, to be consistent with off_t (which is signed) and
with possibility for signed return type (where negative value means
error).
So, convert driver write_zeroes handlers bytes parameter to int64_t.
The only caller of all updated function is bdrv_co_do_pwrite_zeroes().
bdrv_co_do_pwrite_zeroes() itself is of course OK with widening of
callee parameter type. Also, bdrv_co_do_pwrite_zeroes()'s
max_write_zeroes is limited to INT_MAX. So, updated functions all are
safe, the will not get "bytes" larger than before.
Still, let's look through all updated functions, and add assertions to
the ones which are actually unprepared to values larger than INT_MAX.
For these drivers also set explicit max_pwrite_zeroes limit.
Let's go:
backup-top: Calls backup_top_cbw() and bdrv_co_pwrite_zeroes, both
have 64bit argument.
blkdebug: calculations can't overflow, thanks to
bdrv_check_qiov_request() in generic layer. rule_check() and
bdrv_co_pwrite_zeroes() both have 64bit argument.
blklogwrites: pass to blk_log_writes_co_log() with 64bit argument.
blkreply, copy-on-read, filter-compress: pass to bdrv_co_pwrite_zeroes() which is OK
file-posix: both handler calls raw_do_pwrite_zeroes, which is updated.
In raw_do_pwrite_zeroes() calculations are OK due to
bdrv_check_qiov_request(), bytes go to RawPosixAIOData::aio_nbytes
which is uint64_t.
gluster: bytes go to GlusterAIOCB::size which is int64_t and to
glfs_zerofill_async works with off_t.
iscsi: Aha, here we deal with iscsi_writesame16_task() that has
uint32_t num_blocks argument and iscsi_writesame16_task() has
uint16_t argument. Make comments, add assertions and clarify
max_pwrite_zeroes calculation.
iscsi_allocmap_() functions already has int64_t argument
is_byte_request_lun_aligned is simple to update, do it.
mirror_top: pass to bdrv_mirror_top_do_write which has uint16_t
argument
nbd: Aha, here we have protocol limitation, and NBDRequest::len is
uint32_t. max_pwrite_zeroes is cleanly set to 32bit value, so we are
OK for now.
nvme: Again, protocol limitation. And no inherent limit for
write-zeroes at all. But from code that calculates cdw12 it's obvious
that we do have limit and alignment. Let's clarify it. Also,
obviously the code is not prepared to bytes=0. Let's handle this
case too.
trace events already 64bit
qcow2: offset + bytes and alignment still works good (thanks to
bdrv_check_qiov_request()), so tail calculation is OK
qcow2_subcluster_zeroize() has 64bit argument, should be OK
trace events updated
qed: qed_co_request wants int nb_sectors. Also in code we have size_t
used for request length which may be 32bit.. So, let's just keep
INT_MAX as a limit (aligning it down to pwrite_zeroes_alignment) and
don't care.
raw-format: Is OK. raw_adjust_offset and bdrv_co_pwrite_zeroes are both
64bit.
throttle: Both throttle_group_co_io_limits_intercept() and
bdrv_co_pwrite_zeroes() are 64bit.
vmdk: pass to vmdk_pwritev which is 64bit
quorum: pass to quorum_co_pwritev() which is 64bit
preallocated: pass to handle_write() and bdrv_co_pwrite_zeroes(), both
64bit.
Hooray!
At this point all block drivers are prepared to 64bit write-zero
requests or has explicitly set max_pwrite_zeroes.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block_int.h | 2 +-
block/backup-top.c | 2 +-
block/blkdebug.c | 2 +-
block/blklogwrites.c | 4 ++--
block/blkreplay.c | 2 +-
block/copy-on-read.c | 2 +-
block/file-posix.c | 6 +++---
block/filter-compress.c | 2 +-
block/gluster.c | 6 +++---
block/iscsi.c | 31 +++++++++++++++++++++----------
block/mirror.c | 2 +-
block/nbd.c | 6 ++++--
block/nvme.c | 24 +++++++++++++++++++++---
block/preallocate.c | 2 +-
block/qcow2.c | 2 +-
block/qed.c | 9 ++++++++-
block/quorum.c | 2 +-
block/raw-format.c | 2 +-
block/throttle.c | 2 +-
block/vmdk.c | 2 +-
block/trace-events | 4 ++--
21 files changed, 77 insertions(+), 39 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 71e5025534..d9e1f04b21 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -300,7 +300,7 @@ struct BlockDriver {
* will be called instead.
*/
int coroutine_fn (*bdrv_co_pwrite_zeroes)(BlockDriverState *bs,
- int64_t offset, int bytes, BdrvRequestFlags flags);
+ int64_t offset, int64_t bytes, BdrvRequestFlags flags);
int coroutine_fn (*bdrv_co_pdiscard)(BlockDriverState *bs,
int64_t offset, int bytes);
diff --git a/block/backup-top.c b/block/backup-top.c
index aa27afbb1b..838027981b 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -86,7 +86,7 @@ static int coroutine_fn backup_top_co_pdiscard(BlockDriverState *bs,
}
static int coroutine_fn backup_top_co_pwrite_zeroes(BlockDriverState *bs,
- int64_t offset, int bytes, BdrvRequestFlags flags)
+ int64_t offset, int64_t bytes, BdrvRequestFlags flags)
{
int ret = backup_top_cbw(bs, offset, bytes, flags);
if (ret < 0) {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index a7fedd80fd..c81cb9cb1a 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -667,7 +667,7 @@ static int blkdebug_co_flush(BlockDriverState *bs)
}
static int coroutine_fn blkdebug_co_pwrite_zeroes(BlockDriverState *bs,
- int64_t offset, int bytes,
+ int64_t offset, int64_t bytes,
BdrvRequestFlags flags)
{
uint32_t align = MAX(bs->bl.request_alignment,
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index ca174ab135..d7ae64c22d 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -468,8 +468,8 @@ blk_log_writes_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
}
static int coroutine_fn
-blk_log_writes_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes,
- BdrvRequestFlags flags)
+blk_log_writes_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, BdrvRequestFlags flags)
{
return blk_log_writes_co_log(bs, offset, bytes, NULL, flags,
blk_log_writes_co_do_file_pwrite_zeroes, 0,
diff --git a/block/blkreplay.c b/block/blkreplay.c
index 7ba62dcac1..89d74a3cca 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -94,7 +94,7 @@ static int coroutine_fn blkreplay_co_pwritev(BlockDriverState *bs,
}
static int coroutine_fn blkreplay_co_pwrite_zeroes(BlockDriverState *bs,
- int64_t offset, int bytes, BdrvRequestFlags flags)
+ int64_t offset, int64_t bytes, BdrvRequestFlags flags)
{
uint64_t reqid = blkreplay_next_id();
int ret = bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags);
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 7675565080..758a5d44d5 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -206,7 +206,7 @@ static int coroutine_fn cor_co_pwritev_part(BlockDriverState *bs,
static int coroutine_fn cor_co_pwrite_zeroes(BlockDriverState *bs,
- int64_t offset, int bytes,
+ int64_t offset, int64_t bytes,
BdrvRequestFlags flags)
{
return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags);
diff --git a/block/file-posix.c b/block/file-posix.c
index 6faf7eb96e..03618cc18b 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2929,7 +2929,7 @@ raw_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
}
static int coroutine_fn
-raw_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes,
+raw_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes,
BdrvRequestFlags flags, bool blkdev)
{
BDRVRawState *s = bs->opaque;
@@ -2997,7 +2997,7 @@ raw_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes,
static int coroutine_fn raw_co_pwrite_zeroes(
BlockDriverState *bs, int64_t offset,
- int bytes, BdrvRequestFlags flags)
+ int64_t bytes, BdrvRequestFlags flags)
{
return raw_do_pwrite_zeroes(bs, offset, bytes, flags, false);
}
@@ -3577,7 +3577,7 @@ hdev_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
}
static coroutine_fn int hdev_co_pwrite_zeroes(BlockDriverState *bs,
- int64_t offset, int bytes, BdrvRequestFlags flags)
+ int64_t offset, int64_t bytes, BdrvRequestFlags flags)
{
int rc;
diff --git a/block/filter-compress.c b/block/filter-compress.c
index 505822a44f..fb85686b69 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -86,7 +86,7 @@ static int coroutine_fn compress_co_pwritev_part(BlockDriverState *bs,
static int coroutine_fn compress_co_pwrite_zeroes(BlockDriverState *bs,
- int64_t offset, int bytes,
+ int64_t offset, int64_t bytes,
BdrvRequestFlags flags)
{
return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags);
diff --git a/block/gluster.c b/block/gluster.c
index e8ee14c8e9..6a17b37c0c 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1003,19 +1003,19 @@ static void qemu_gluster_reopen_abort(BDRVReopenState *state)
#ifdef CONFIG_GLUSTERFS_ZEROFILL
static coroutine_fn int qemu_gluster_co_pwrite_zeroes(BlockDriverState *bs,
int64_t offset,
- int size,
+ int64_t bytes,
BdrvRequestFlags flags)
{
int ret;
GlusterAIOCB acb;
BDRVGlusterState *s = bs->opaque;
- acb.size = size;
+ acb.size = bytes;
acb.ret = 0;
acb.coroutine = qemu_coroutine_self();
acb.aio_context = bdrv_get_aio_context(bs);
- ret = glfs_zerofill_async(s->fd, offset, size, gluster_finish_aiocb, &acb);
+ ret = glfs_zerofill_async(s->fd, offset, bytes, gluster_finish_aiocb, &acb);
if (ret < 0) {
return -errno;
}
diff --git a/block/iscsi.c b/block/iscsi.c
index 6bcde6ec6b..b90ed67377 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -427,14 +427,14 @@ static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
return sector * BDRV_SECTOR_SIZE / iscsilun->block_size;
}
-static bool is_byte_request_lun_aligned(int64_t offset, int count,
+static bool is_byte_request_lun_aligned(int64_t offset, int64_t bytes,
IscsiLun *iscsilun)
{
- if (offset % iscsilun->block_size || count % iscsilun->block_size) {
+ if (offset % iscsilun->block_size || bytes % iscsilun->block_size) {
error_report("iSCSI misaligned request: "
"iscsilun->block_size %u, offset %" PRIi64
- ", count %d",
- iscsilun->block_size, offset, count);
+ ", bytes %" PRIi64,
+ iscsilun->block_size, offset, bytes);
return false;
}
return true;
@@ -1205,15 +1205,16 @@ out_unlock:
static int
coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
- int bytes, BdrvRequestFlags flags)
+ int64_t bytes, BdrvRequestFlags flags)
{
IscsiLun *iscsilun = bs->opaque;
struct IscsiTask iTask;
uint64_t lba;
- uint32_t nb_blocks;
+ uint64_t nb_blocks;
bool use_16_for_ws = iscsilun->use_16_for_rw;
int r = 0;
+
if (!is_byte_request_lun_aligned(offset, bytes, iscsilun)) {
return -ENOTSUP;
}
@@ -1250,11 +1251,21 @@ coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
iscsi_co_init_iscsitask(iscsilun, &iTask);
retry:
if (use_16_for_ws) {
+ /*
+ * iscsi_writesame16_task num_blocks argument is uint32_t. We rely here
+ * on our max_pwrite_zeroes limit.
+ */
+ assert(nb_blocks < UINT32_MAX);
iTask.task = iscsi_writesame16_task(iscsilun->iscsi, iscsilun->lun, lba,
iscsilun->zeroblock, iscsilun->block_size,
nb_blocks, 0, !!(flags & BDRV_REQ_MAY_UNMAP),
0, 0, iscsi_co_generic_cb, &iTask);
} else {
+ /*
+ * iscsi_writesame10_task num_blocks argument is uint16_t. We rely here
+ * on our max_pwrite_zeroes limit.
+ */
+ assert(nb_blocks < UINT16_MAX);
iTask.task = iscsi_writesame10_task(iscsilun->iscsi, iscsilun->lun, lba,
iscsilun->zeroblock, iscsilun->block_size,
nb_blocks, 0, !!(flags & BDRV_REQ_MAY_UNMAP),
@@ -2074,10 +2085,10 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
bs->bl.pdiscard_alignment = iscsilun->block_size;
}
- if (iscsilun->bl.max_ws_len < 0xffffffff / block_size) {
- bs->bl.max_pwrite_zeroes =
- iscsilun->bl.max_ws_len * iscsilun->block_size;
- }
+ bs->bl.max_pwrite_zeroes =
+ MIN_NON_ZERO(iscsilun->bl.max_ws_len * iscsilun->block_size,
+ max_xfer_len * iscsilun->block_size);
+
if (iscsilun->lbp.lbpws) {
bs->bl.pwrite_zeroes_alignment =
iscsilun->bl.opt_unmap_gran * iscsilun->block_size;
diff --git a/block/mirror.c b/block/mirror.c
index f7b7f4d566..e40608b2d3 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1480,7 +1480,7 @@ static int coroutine_fn bdrv_mirror_top_flush(BlockDriverState *bs)
}
static int coroutine_fn bdrv_mirror_top_pwrite_zeroes(BlockDriverState *bs,
- int64_t offset, int bytes, BdrvRequestFlags flags)
+ int64_t offset, int64_t bytes, BdrvRequestFlags flags)
{
return bdrv_mirror_top_do_write(bs, MIRROR_METHOD_ZERO, offset, bytes, NULL,
flags);
diff --git a/block/nbd.c b/block/nbd.c
index 34cb28927b..9ec822c083 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1619,15 +1619,17 @@ static int nbd_client_co_pwritev(BlockDriverState *bs, int64_t offset,
}
static int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
- int bytes, BdrvRequestFlags flags)
+ int64_t bytes, BdrvRequestFlags flags)
{
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
NBDRequest request = {
.type = NBD_CMD_WRITE_ZEROES,
.from = offset,
- .len = bytes,
+ .len = bytes, /* .len is uint32_t actually */
};
+ assert(bytes < UINT32_MAX); /* relay on max_pwrite_zeroes */
+
assert(!(s->info.flags & NBD_FLAG_READ_ONLY));
if (!(s->info.flags & NBD_FLAG_SEND_WRITE_ZEROES)) {
return -ENOTSUP;
diff --git a/block/nvme.c b/block/nvme.c
index 1a128ef95e..1263022e1c 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1265,19 +1265,29 @@ static coroutine_fn int nvme_co_flush(BlockDriverState *bs)
static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
int64_t offset,
- int bytes,
+ int64_t bytes,
BdrvRequestFlags flags)
{
BDRVNVMeState *s = bs->opaque;
NVMeQueuePair *ioq = s->queues[INDEX_IO(0)];
NVMeRequest *req;
-
- uint32_t cdw12 = ((bytes >> s->blkshift) - 1) & 0xFFFF;
+ uint32_t cdw12;
if (!s->supports_write_zeroes) {
return -ENOTSUP;
}
+ if (bytes == 0) {
+ return 0;
+ }
+
+ cdw12 = ((bytes >> s->blkshift) - 1) & 0xFFFF;
+ /*
+ * We should not loose information. pwrite_zeroes_alignment and
+ * max_pwrite_zeroes guarantees it.
+ */
+ assert(((cdw12 + 1) << s->blkshift) == bytes);
+
NvmeCmd cmd = {
.opcode = NVME_CMD_WRITE_ZEROES,
.nsid = cpu_to_le32(s->nsid),
@@ -1441,6 +1451,14 @@ static void nvme_refresh_limits(BlockDriverState *bs, Error **errp)
bs->bl.opt_mem_alignment = s->page_size;
bs->bl.request_alignment = s->page_size;
bs->bl.max_transfer = s->max_transfer;
+
+ /*
+ * Look at nvme_co_pwrite_zeroes: after shift and decrement we should get
+ * at most 0xFFFF
+ */
+ bs->bl.max_pwrite_zeroes = 1ULL << (s->blkshift + 16);
+ bs->bl.pwrite_zeroes_alignment = MAX(bs->bl.request_alignment,
+ 1UL << s->blkshift);
}
static void nvme_detach_aio_context(BlockDriverState *bs)
diff --git a/block/preallocate.c b/block/preallocate.c
index c19885af17..99e28d9f08 100644
--- a/block/preallocate.c
+++ b/block/preallocate.c
@@ -337,7 +337,7 @@ static bool coroutine_fn handle_write(BlockDriverState *bs, int64_t offset,
}
static int coroutine_fn preallocate_co_pwrite_zeroes(BlockDriverState *bs,
- int64_t offset, int bytes, BdrvRequestFlags flags)
+ int64_t offset, int64_t bytes, BdrvRequestFlags flags)
{
bool want_merge_zero =
!(flags & ~(BDRV_REQ_ZERO_WRITE | BDRV_REQ_NO_FALLBACK));
diff --git a/block/qcow2.c b/block/qcow2.c
index c786601d52..c9bce9711d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3890,7 +3890,7 @@ static bool is_zero(BlockDriverState *bs, int64_t offset, int64_t bytes)
}
static coroutine_fn int qcow2_co_pwrite_zeroes(BlockDriverState *bs,
- int64_t offset, int bytes, BdrvRequestFlags flags)
+ int64_t offset, int64_t bytes, BdrvRequestFlags flags)
{
int ret;
BDRVQcow2State *s = bs->opaque;
diff --git a/block/qed.c b/block/qed.c
index f45c640513..09773c584b 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -582,6 +582,7 @@ static void bdrv_qed_refresh_limits(BlockDriverState *bs, Error **errp)
BDRVQEDState *s = bs->opaque;
bs->bl.pwrite_zeroes_alignment = s->header.cluster_size;
+ bs->bl.max_pwrite_zeroes = QEMU_ALIGN_DOWN(INT_MAX, s->header.cluster_size);
}
/* We have nothing to do for QED reopen, stubs just return
@@ -1397,7 +1398,7 @@ static int coroutine_fn bdrv_qed_co_writev(BlockDriverState *bs,
static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs,
int64_t offset,
- int bytes,
+ int64_t bytes,
BdrvRequestFlags flags)
{
BDRVQEDState *s = bs->opaque;
@@ -1408,6 +1409,12 @@ static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs,
*/
QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, bytes);
+ /*
+ * QED is not prepared for 62bit write-zero requests, so rely on
+ * max_pwrite_zeroes.
+ */
+ assert(bytes <= INT_MAX);
+
/* Fall back if the request is not aligned */
if (qed_offset_into_cluster(s, offset) ||
qed_offset_into_cluster(s, bytes)) {
diff --git a/block/quorum.c b/block/quorum.c
index 353401ac08..772de80a77 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -746,7 +746,7 @@ static int quorum_co_pwritev(BlockDriverState *bs, int64_t offset,
}
static int quorum_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
- int bytes, BdrvRequestFlags flags)
+ int64_t bytes, BdrvRequestFlags flags)
{
return quorum_co_pwritev(bs, offset, bytes, NULL,
diff --git a/block/raw-format.c b/block/raw-format.c
index 051b00a4d4..4e9304c63b 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -289,7 +289,7 @@ static int coroutine_fn raw_co_block_status(BlockDriverState *bs,
}
static int coroutine_fn raw_co_pwrite_zeroes(BlockDriverState *bs,
- int64_t offset, int bytes,
+ int64_t offset, int64_t bytes,
BdrvRequestFlags flags)
{
int ret;
diff --git a/block/throttle.c b/block/throttle.c
index 1330e844c3..c13fe9067f 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -135,7 +135,7 @@ static int coroutine_fn throttle_co_pwritev(BlockDriverState *bs,
}
static int coroutine_fn throttle_co_pwrite_zeroes(BlockDriverState *bs,
- int64_t offset, int bytes,
+ int64_t offset, int64_t bytes,
BdrvRequestFlags flags)
{
ThrottleGroupMember *tgm = bs->opaque;
diff --git a/block/vmdk.c b/block/vmdk.c
index 8d49e54bdd..fb4cc9da90 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2109,7 +2109,7 @@ vmdk_co_pwritev_compressed(BlockDriverState *bs, int64_t offset, int64_t bytes,
static int coroutine_fn vmdk_co_pwrite_zeroes(BlockDriverState *bs,
int64_t offset,
- int bytes,
+ int64_t bytes,
BdrvRequestFlags flags)
{
int ret;
diff --git a/block/trace-events b/block/trace-events
index 3cbea9e2e2..3edd2899c2 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -80,8 +80,8 @@ 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"
qcow2_writev_data(void *co, uint64_t offset) "co %p offset 0x%" PRIx64
-qcow2_pwrite_zeroes_start_req(void *co, int64_t offset, int count) "co %p offset 0x%" PRIx64 " count %d"
-qcow2_pwrite_zeroes(void *co, int64_t offset, int count) "co %p offset 0x%" PRIx64 " count %d"
+qcow2_pwrite_zeroes_start_req(void *co, int64_t offset, int64_t bytes) "co %p offset 0x%" PRIx64 " bytes %" PRId64
+qcow2_pwrite_zeroes(void *co, int64_t offset, int64_t bytes) "co %p offset 0x%" PRIx64 " bytes %" PRId64
qcow2_skip_cow(void *co, uint64_t offset, int nb_clusters) "co %p offset 0x%" PRIx64 " nb_clusters %d"
# qcow2-cluster.c
--
2.29.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 08/11] block/io: allow 64bit write-zeroes requests
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
` (6 preceding siblings ...)
2021-03-24 20:51 ` [PATCH v4 07/11] block: use int64_t instead of int in driver write_zeroes handlers Vladimir Sementsov-Ogievskiy
@ 2021-03-24 20:51 ` Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 09/11] block: make BlockLimits::max_pdiscard 64bit Vladimir Sementsov-Ogievskiy
` (3 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-24 20:51 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, vsementsov, jsnow
Now, when all drivers are updated by previous commit, we can drop two
last limiters on write-zeroes path: INT_MAX in
bdrv_co_do_pwrite_zeroes() and bdrv_check_request32() in
bdrv_co_pwritev_part().
Now everything is prepared for implementing incredibly cool and fast
big-write-zeroes in NBD and qcow2. And any other driver which wants it
of course.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/block/io.c b/block/io.c
index 79e600af27..a3c2b7740c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1836,7 +1836,8 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
int head = 0;
int tail = 0;
- int64_t max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX);
+ int64_t max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes,
+ INT64_MAX);
int alignment = MAX(bs->bl.pwrite_zeroes_alignment,
bs->bl.request_alignment);
int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, MAX_BOUNCE_BUFFER);
@@ -2212,7 +2213,11 @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
return -ENOMEDIUM;
}
- ret = bdrv_check_request32(offset, bytes, qiov, qiov_offset);
+ if (flags & BDRV_REQ_ZERO_WRITE) {
+ ret = bdrv_check_qiov_request(offset, bytes, qiov, qiov_offset, NULL);
+ } else {
+ ret = bdrv_check_request32(offset, bytes, qiov, qiov_offset);
+ }
if (ret < 0) {
return ret;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 09/11] block: make BlockLimits::max_pdiscard 64bit
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
` (7 preceding siblings ...)
2021-03-24 20:51 ` [PATCH v4 08/11] block/io: allow 64bit write-zeroes requests Vladimir Sementsov-Ogievskiy
@ 2021-03-24 20:51 ` Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 10/11] block: use int64_t instead of int in driver discard handlers Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-24 20:51 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, vsementsov, jsnow
We are going to support 64 bit discard requests. Now update the
limit variable. It's absolutely safe. The variable is set in some
drivers, and used in bdrv_co_pdiscard().
Update also max_pdiscard variable in bdrv_co_pdiscard(), so that
bdrv_co_pdiscard() is now prepared to 64bit requests. The remaining
logic including num, offset and bytes variables is already
supporting 64bit requests.
So the only thing that prevents 64 bit requests is limiting
max_pdiscard variable to INT_MAX in bdrv_co_pdiscard().
We'll drop this limitation after updating all block drivers.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block_int.h | 9 ++++-----
block/io.c | 3 ++-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index d9e1f04b21..0245620bf6 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -664,11 +664,10 @@ typedef struct BlockLimits {
* otherwise. */
uint32_t request_alignment;
- /* Maximum number of bytes that can be discarded at once (since it
- * is signed, it must be < 2G, if set). Must be multiple of
- * pdiscard_alignment, but need not be power of 2. May be 0 if no
- * inherent 32-bit limit */
- int32_t max_pdiscard;
+ /* Maximum number of bytes that can be discarded at once. Must be multiple
+ * of pdiscard_alignment, but need not be power of 2. May be 0 if no
+ * inherent 64-bit limit */
+ int64_t max_pdiscard;
/* Optimal alignment for discard requests in bytes. A power of 2
* is best but not mandatory. Must be a multiple of
diff --git a/block/io.c b/block/io.c
index a3c2b7740c..129cfba68e 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2964,7 +2964,8 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
int64_t bytes)
{
BdrvTrackedRequest req;
- int max_pdiscard, ret;
+ int ret;
+ int64_t max_pdiscard;
int head, tail, align;
BlockDriverState *bs = child->bs;
--
2.29.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 10/11] block: use int64_t instead of int in driver discard handlers
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
` (8 preceding siblings ...)
2021-03-24 20:51 ` [PATCH v4 09/11] block: make BlockLimits::max_pdiscard 64bit Vladimir Sementsov-Ogievskiy
@ 2021-03-24 20:51 ` Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 11/11] block/io: allow 64bit discard requests Vladimir Sementsov-Ogievskiy
2021-03-24 21:13 ` [PATCH v4 00/11] 64bit block-layer: part II no-reply
11 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-24 20:51 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, vsementsov, jsnow
We are generally moving to int64_t for both offset and bytes parameters
on all io paths.
Main motivation is realization of 64-bit write_zeroes operation for
fast zeroing large disk chunks, up to the whole disk.
We chose signed type, to be consistent with off_t (which is signed) and
with possibility for signed return type (where negative value means
error).
So, convert driver discard handlers bytes parameter to int64_t.
The only caller of all updated function is bdrv_co_pdiscard in
block/io.c. It is already prepared to work with 64bit requests, but
pass at most max(bs->bl.max_pdiscard, INT_MAX) to the driver.
Let's look at all updated functions:
backup-top: pass to bdrv_co_pdiscard which is 64bit
blkdebug: all calculations are still OK, thanks to
bdrv_check_qiov_request().
both rule_check and bdrv_co_pdiscard are 64bit
blklogwrites: pass to blk_loc_writes_co_log which is 64bit
blkreply, copy-on-read, filter-compress: pass to bdrv_co_pdiscard, OK
file-posix: one handler calls raw_account_discard() is 64bit and both
handlers calls raw_do_pdiscard(). Update raw_do_pdiscard, which pass
to RawPosixAIOData::aio_nbytes, which is 64bit (and calls
raw_account_discard())
gluster: somehow, third argument of glfs_discard_async is size_t.
Let's set max_pdiscard accordingly.
iscsi: iscsi_allocmap_set_invalid is 64bit,
!is_byte_request_lun_aligned is 64bit.
list.num is uint32_t. Let's clarify max_pdiscard and
pdiscard_alignment.
mirror_top, preallocate: pass to bdrv_mirror_top_do_write() which is
64bit
nbd: protocol limitation. max_pdiscard is alredy set strict enough,
keep it as is for now.
nvmd: buf.nlb is uint32_t and we do shift. So, add corresponding limits
to nvme_refresh_limits().
qcow2: calculations are still OK, thanks to bdrv_check_qiov_request(),
qcow2_cluster_discard() is 64bit.
raw-format: raw_adjust_offset() is 64bit, bdrv_co_pdiscard too.
sheepdog: the format is deprecated. Don't care and just make old
INT_MAX limit to be explicit
throttle: pass to bdrv_co_pdiscard() which is 64bit and to
throttle_group_co_io_limits_intercept() which is 64bit as well.
test-block-iothread: bytes argument is unused
Great! Now all drivers are prepared to 64bit discard requests or has
explicit max_pdiscard limit.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block_int.h | 2 +-
block/backup-top.c | 2 +-
block/blkdebug.c | 2 +-
block/blklogwrites.c | 4 ++--
block/blkreplay.c | 2 +-
block/copy-on-read.c | 2 +-
block/file-posix.c | 7 ++++---
block/filter-compress.c | 2 +-
block/gluster.c | 7 +++++--
block/iscsi.c | 10 +++++-----
block/mirror.c | 2 +-
block/nbd.c | 6 ++++--
block/nvme.c | 14 +++++++++++++-
block/preallocate.c | 2 +-
block/qcow2.c | 2 +-
block/raw-format.c | 2 +-
block/sheepdog.c | 15 ++++++++++++++-
block/throttle.c | 2 +-
tests/unit/test-block-iothread.c | 2 +-
block/trace-events | 4 ++--
20 files changed, 61 insertions(+), 30 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0245620bf6..735935841e 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -302,7 +302,7 @@ struct BlockDriver {
int coroutine_fn (*bdrv_co_pwrite_zeroes)(BlockDriverState *bs,
int64_t offset, int64_t bytes, BdrvRequestFlags flags);
int coroutine_fn (*bdrv_co_pdiscard)(BlockDriverState *bs,
- int64_t offset, int bytes);
+ int64_t offset, int64_t bytes);
/* Map [offset, offset + nbytes) range onto a child of @bs to copy from,
* and invoke bdrv_co_copy_range_from(child, ...), or invoke
diff --git a/block/backup-top.c b/block/backup-top.c
index 838027981b..c5ce84def4 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -75,7 +75,7 @@ static coroutine_fn int backup_top_cbw(BlockDriverState *bs, uint64_t offset,
}
static int coroutine_fn backup_top_co_pdiscard(BlockDriverState *bs,
- int64_t offset, int bytes)
+ int64_t offset, int64_t bytes)
{
int ret = backup_top_cbw(bs, offset, bytes, 0);
if (ret < 0) {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index c81cb9cb1a..2d98a33982 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -700,7 +700,7 @@ static int coroutine_fn blkdebug_co_pwrite_zeroes(BlockDriverState *bs,
}
static int coroutine_fn blkdebug_co_pdiscard(BlockDriverState *bs,
- int64_t offset, int bytes)
+ int64_t offset, int64_t bytes)
{
uint32_t align = bs->bl.pdiscard_alignment;
int err;
diff --git a/block/blklogwrites.c b/block/blklogwrites.c
index d7ae64c22d..f7a251e91f 100644
--- a/block/blklogwrites.c
+++ b/block/blklogwrites.c
@@ -484,9 +484,9 @@ static int coroutine_fn blk_log_writes_co_flush_to_disk(BlockDriverState *bs)
}
static int coroutine_fn
-blk_log_writes_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
+blk_log_writes_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
{
- return blk_log_writes_co_log(bs, offset, count, NULL, 0,
+ return blk_log_writes_co_log(bs, offset, bytes, NULL, 0,
blk_log_writes_co_do_file_pdiscard,
LOG_DISCARD_FLAG, false);
}
diff --git a/block/blkreplay.c b/block/blkreplay.c
index 89d74a3cca..dcbe780ddb 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -105,7 +105,7 @@ static int coroutine_fn blkreplay_co_pwrite_zeroes(BlockDriverState *bs,
}
static int coroutine_fn blkreplay_co_pdiscard(BlockDriverState *bs,
- int64_t offset, int bytes)
+ int64_t offset, int64_t bytes)
{
uint64_t reqid = blkreplay_next_id();
int ret = bdrv_co_pdiscard(bs->file, offset, bytes);
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 758a5d44d5..c29cfdd10e 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -214,7 +214,7 @@ static int coroutine_fn cor_co_pwrite_zeroes(BlockDriverState *bs,
static int coroutine_fn cor_co_pdiscard(BlockDriverState *bs,
- int64_t offset, int bytes)
+ int64_t offset, int64_t bytes)
{
return bdrv_co_pdiscard(bs->file, offset, bytes);
}
diff --git a/block/file-posix.c b/block/file-posix.c
index 03618cc18b..1ea2d0c889 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2899,7 +2899,8 @@ static void raw_account_discard(BDRVRawState *s, uint64_t nbytes, int ret)
}
static coroutine_fn int
-raw_do_pdiscard(BlockDriverState *bs, int64_t offset, int bytes, bool blkdev)
+raw_do_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ bool blkdev)
{
BDRVRawState *s = bs->opaque;
RawPosixAIOData acb;
@@ -2923,7 +2924,7 @@ raw_do_pdiscard(BlockDriverState *bs, int64_t offset, int bytes, bool blkdev)
}
static coroutine_fn int
-raw_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
+raw_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
{
return raw_do_pdiscard(bs, offset, bytes, false);
}
@@ -3563,7 +3564,7 @@ static int fd_open(BlockDriverState *bs)
}
static coroutine_fn int
-hdev_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
+hdev_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
{
BDRVRawState *s = bs->opaque;
int ret;
diff --git a/block/filter-compress.c b/block/filter-compress.c
index fb85686b69..d5be538619 100644
--- a/block/filter-compress.c
+++ b/block/filter-compress.c
@@ -94,7 +94,7 @@ static int coroutine_fn compress_co_pwrite_zeroes(BlockDriverState *bs,
static int coroutine_fn compress_co_pdiscard(BlockDriverState *bs,
- int64_t offset, int bytes)
+ int64_t offset, int64_t bytes)
{
return bdrv_co_pdiscard(bs->file, offset, bytes);
}
diff --git a/block/gluster.c b/block/gluster.c
index 6a17b37c0c..066fdf60fa 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -891,6 +891,7 @@ out:
static void qemu_gluster_refresh_limits(BlockDriverState *bs, Error **errp)
{
bs->bl.max_transfer = GLUSTER_MAX_TRANSFER;
+ bs->bl.max_pdiscard = SIZE_MAX;
}
static int qemu_gluster_reopen_prepare(BDRVReopenState *state,
@@ -1297,18 +1298,20 @@ error:
#ifdef CONFIG_GLUSTERFS_DISCARD
static coroutine_fn int qemu_gluster_co_pdiscard(BlockDriverState *bs,
- int64_t offset, int size)
+ int64_t offset, int64_t bytes)
{
int ret;
GlusterAIOCB acb;
BDRVGlusterState *s = bs->opaque;
+ assert(bytes <= SIZE_MAX); /* rely on max_pdiscard */
+
acb.size = 0;
acb.ret = 0;
acb.coroutine = qemu_coroutine_self();
acb.aio_context = bdrv_get_aio_context(bs);
- ret = glfs_discard_async(s->fd, offset, size, gluster_finish_aiocb, &acb);
+ ret = glfs_discard_async(s->fd, offset, bytes, gluster_finish_aiocb, &acb);
if (ret < 0) {
return -errno;
}
diff --git a/block/iscsi.c b/block/iscsi.c
index b90ed67377..297919ebc2 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1141,7 +1141,8 @@ iscsi_getlength(BlockDriverState *bs)
}
static int
-coroutine_fn iscsi_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
+coroutine_fn iscsi_co_pdiscard(BlockDriverState *bs, int64_t offset,
+ int64_t bytes)
{
IscsiLun *iscsilun = bs->opaque;
struct IscsiTask iTask;
@@ -2075,10 +2076,9 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
}
if (iscsilun->lbp.lbpu) {
- if (iscsilun->bl.max_unmap < 0xffffffff / block_size) {
- bs->bl.max_pdiscard =
- iscsilun->bl.max_unmap * iscsilun->block_size;
- }
+ bs->bl.max_pdiscard =
+ MIN_NON_ZERO(iscsilun->bl.max_unmap * iscsilun->block_size,
+ (uint64_t)UINT32_MAX * iscsilun->block_size);
bs->bl.pdiscard_alignment =
iscsilun->bl.opt_unmap_gran * iscsilun->block_size;
} else {
diff --git a/block/mirror.c b/block/mirror.c
index e40608b2d3..e2038a06d3 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1487,7 +1487,7 @@ static int coroutine_fn bdrv_mirror_top_pwrite_zeroes(BlockDriverState *bs,
}
static int coroutine_fn bdrv_mirror_top_pdiscard(BlockDriverState *bs,
- int64_t offset, int bytes)
+ int64_t offset, int64_t bytes)
{
return bdrv_mirror_top_do_write(bs, MIRROR_METHOD_DISCARD, offset, bytes,
NULL, 0);
diff --git a/block/nbd.c b/block/nbd.c
index 9ec822c083..cc522a765c 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1669,15 +1669,17 @@ static int nbd_client_co_flush(BlockDriverState *bs)
}
static int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset,
- int bytes)
+ int64_t bytes)
{
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
NBDRequest request = {
.type = NBD_CMD_TRIM,
.from = offset,
- .len = bytes,
+ .len = bytes, /* len is uint32_t */
};
+ assert(bytes <= UINT32_MAX); /* rely on max_pdiscard */
+
assert(!(s->info.flags & NBD_FLAG_READ_ONLY));
if (!(s->info.flags & NBD_FLAG_SEND_TRIM) || !bytes) {
return 0;
diff --git a/block/nvme.c b/block/nvme.c
index 1263022e1c..0c6c4031f3 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1329,7 +1329,7 @@ static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
int64_t offset,
- int bytes)
+ int64_t bytes)
{
BDRVNVMeState *s = bs->opaque;
NVMeQueuePair *ioq = s->queues[INDEX_IO(0)];
@@ -1356,6 +1356,14 @@ static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
assert(s->queue_count > 1);
+ /*
+ * Filling the @buf requires @offset and @bytes to satisfy restrictions
+ * defined in nvme_refresh_limits().
+ */
+ assert(QEMU_IS_ALIGNED(bytes, 1UL << s->blkshift));
+ assert(QEMU_IS_ALIGNED(offset, 1UL << s->blkshift));
+ assert((bytes >> s->blkshift) <= UINT32_MAX);
+
buf = qemu_try_memalign(s->page_size, s->page_size);
if (!buf) {
return -ENOMEM;
@@ -1459,6 +1467,10 @@ static void nvme_refresh_limits(BlockDriverState *bs, Error **errp)
bs->bl.max_pwrite_zeroes = 1ULL << (s->blkshift + 16);
bs->bl.pwrite_zeroes_alignment = MAX(bs->bl.request_alignment,
1UL << s->blkshift);
+
+ bs->bl.max_pdiscard = (uint64_t)UINT32_MAX << s->blkshift;
+ bs->bl.pdiscard_alignment = MAX(bs->bl.request_alignment,
+ 1UL << s->blkshift);
}
static void nvme_detach_aio_context(BlockDriverState *bs)
diff --git a/block/preallocate.c b/block/preallocate.c
index 99e28d9f08..1d4233f730 100644
--- a/block/preallocate.c
+++ b/block/preallocate.c
@@ -235,7 +235,7 @@ static coroutine_fn int preallocate_co_preadv_part(
}
static int coroutine_fn preallocate_co_pdiscard(BlockDriverState *bs,
- int64_t offset, int bytes)
+ int64_t offset, int64_t bytes)
{
return bdrv_co_pdiscard(bs->file, offset, bytes);
}
diff --git a/block/qcow2.c b/block/qcow2.c
index c9bce9711d..955e488046 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3945,7 +3945,7 @@ static coroutine_fn int qcow2_co_pwrite_zeroes(BlockDriverState *bs,
}
static coroutine_fn int qcow2_co_pdiscard(BlockDriverState *bs,
- int64_t offset, int bytes)
+ int64_t offset, int64_t bytes)
{
int ret;
BDRVQcow2State *s = bs->opaque;
diff --git a/block/raw-format.c b/block/raw-format.c
index 4e9304c63b..45846e42d5 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -302,7 +302,7 @@ static int coroutine_fn raw_co_pwrite_zeroes(BlockDriverState *bs,
}
static int coroutine_fn raw_co_pdiscard(BlockDriverState *bs,
- int64_t offset, int bytes)
+ int64_t offset, int64_t bytes)
{
int ret;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index a45c73826d..80e04dccfd 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -3101,7 +3101,7 @@ static int sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
- int bytes)
+ int64_t bytes)
{
SheepdogAIOCB acb;
BDRVSheepdogState *s = bs->opaque;
@@ -3113,6 +3113,8 @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
return 0;
}
+ assert(bytes <= INT_MAX); /* thanks to max_pdiscard */
+
memset(&discard_iov, 0, sizeof(discard_iov));
memset(&iov, 0, sizeof(iov));
iov.iov_base = &zero;
@@ -3186,6 +3188,11 @@ static int64_t sd_get_allocated_file_size(BlockDriverState *bs)
return size;
}
+static void sd_refresh_limits(BlockDriverState *bs, Error **errp)
+{
+ bs->bl.max_pdiscard = INT_MAX;
+}
+
static QemuOptsList sd_create_opts = {
.name = "sheepdog-create-opts",
.head = QTAILQ_HEAD_INITIALIZER(sd_create_opts.head),
@@ -3269,6 +3276,8 @@ static BlockDriver bdrv_sheepdog = {
.create_opts = &sd_create_opts,
.strong_runtime_opts = sd_strong_runtime_opts,
+
+ .bdrv_refresh_limits = sd_refresh_limits,
};
static BlockDriver bdrv_sheepdog_tcp = {
@@ -3307,6 +3316,8 @@ static BlockDriver bdrv_sheepdog_tcp = {
.create_opts = &sd_create_opts,
.strong_runtime_opts = sd_strong_runtime_opts,
+
+ .bdrv_refresh_limits = sd_refresh_limits,
};
static BlockDriver bdrv_sheepdog_unix = {
@@ -3345,6 +3356,8 @@ static BlockDriver bdrv_sheepdog_unix = {
.create_opts = &sd_create_opts,
.strong_runtime_opts = sd_strong_runtime_opts,
+
+ .bdrv_refresh_limits = sd_refresh_limits,
};
static void bdrv_sheepdog_init(void)
diff --git a/block/throttle.c b/block/throttle.c
index c13fe9067f..6e8d52fa24 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -145,7 +145,7 @@ static int coroutine_fn throttle_co_pwrite_zeroes(BlockDriverState *bs,
}
static int coroutine_fn throttle_co_pdiscard(BlockDriverState *bs,
- int64_t offset, int bytes)
+ int64_t offset, int64_t bytes)
{
ThrottleGroupMember *tgm = bs->opaque;
throttle_group_co_io_limits_intercept(tgm, bytes, true);
diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
index 50b8718b2a..9656814814 100644
--- a/tests/unit/test-block-iothread.c
+++ b/tests/unit/test-block-iothread.c
@@ -48,7 +48,7 @@ static int coroutine_fn bdrv_test_co_pwritev(BlockDriverState *bs,
}
static int coroutine_fn bdrv_test_co_pdiscard(BlockDriverState *bs,
- int64_t offset, int bytes)
+ int64_t offset, int64_t bytes)
{
return 0;
}
diff --git a/block/trace-events b/block/trace-events
index 3edd2899c2..3b86c03b2f 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -152,8 +152,8 @@ nvme_write_zeroes(void *s, uint64_t offset, uint64_t bytes, int flags) "s %p off
nvme_qiov_unaligned(const void *qiov, int n, void *base, size_t size, int align) "qiov %p n %d base %p size 0x%zx align 0x%x"
nvme_prw_buffered(void *s, uint64_t offset, uint64_t bytes, int niov, int is_write) "s %p offset 0x%"PRIx64" bytes %"PRId64" niov %d is_write %d"
nvme_rw_done(void *s, int is_write, uint64_t offset, uint64_t bytes, int ret) "s %p is_write %d offset 0x%"PRIx64" bytes %"PRId64" ret %d"
-nvme_dsm(void *s, uint64_t offset, uint64_t bytes) "s %p offset 0x%"PRIx64" bytes %"PRId64""
-nvme_dsm_done(void *s, uint64_t offset, uint64_t bytes, int ret) "s %p offset 0x%"PRIx64" bytes %"PRId64" ret %d"
+nvme_dsm(void *s, int64_t offset, int64_t bytes) "s %p offset 0x%"PRIx64" bytes %"PRId64""
+nvme_dsm_done(void *s, int64_t offset, int64_t bytes, int ret) "s %p offset 0x%"PRIx64" bytes %"PRId64" ret %d"
nvme_dma_map_flush(void *s) "s %p"
nvme_free_req_queue_wait(void *s, unsigned q_index) "s %p q #%u"
nvme_create_queue_pair(unsigned q_index, void *q, unsigned size, void *aio_context, int fd) "index %u q %p size %u aioctx %p fd %d"
--
2.29.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 11/11] block/io: allow 64bit discard requests
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
` (9 preceding siblings ...)
2021-03-24 20:51 ` [PATCH v4 10/11] block: use int64_t instead of int in driver discard handlers Vladimir Sementsov-Ogievskiy
@ 2021-03-24 20:51 ` Vladimir Sementsov-Ogievskiy
2021-03-24 21:13 ` [PATCH v4 00/11] 64bit block-layer: part II no-reply
11 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-24 20:51 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, vsementsov, jsnow
Now, when all drivers are updated by previous commit, we can drop the
last limiter on pdiscard path: INT_MAX in bdrv_co_pdiscard().
Now everything is prepared for implementing incredibly cool and fast
big-discard requests in NBD and qcow2. And any other driver which wants
it of course.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/io.c b/block/io.c
index 129cfba68e..e3d0c1ae30 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3009,7 +3009,7 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
goto out;
}
- max_pdiscard = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_pdiscard, INT_MAX),
+ max_pdiscard = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_pdiscard, INT64_MAX),
align);
assert(max_pdiscard >= bs->bl.request_alignment);
--
2.29.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v4 00/11] 64bit block-layer: part II
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
` (10 preceding siblings ...)
2021-03-24 20:51 ` [PATCH v4 11/11] block/io: allow 64bit discard requests Vladimir Sementsov-Ogievskiy
@ 2021-03-24 21:13 ` no-reply
2021-03-25 7:42 ` Vladimir Sementsov-Ogievskiy
11 siblings, 1 reply; 23+ messages in thread
From: no-reply @ 2021-03-24 21:13 UTC (permalink / raw)
To: vsementsov
Cc: fam, kwolf, integration, berto, stefanha, qemu-block,
pavel.dovgaluk, pl, qemu-devel, mreitz, jsnow, pbonzini,
vsementsov, ronniesahlberg, sw, namei.unix, dillaman, ari
Patchew URL: https://patchew.org/QEMU/20210324205132.464899-1-vsementsov@virtuozzo.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210324205132.464899-1-vsementsov@virtuozzo.com
Subject: [PATCH v4 00/11] 64bit block-layer: part II
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/20210323221539.3532660-1-crosa@redhat.com -> patchew/20210323221539.3532660-1-crosa@redhat.com
* [new tag] patchew/20210324205132.464899-1-vsementsov@virtuozzo.com -> patchew/20210324205132.464899-1-vsementsov@virtuozzo.com
Switched to a new branch 'test'
bed608a block/io: allow 64bit discard requests
9b3b5c7 block: use int64_t instead of int in driver discard handlers
9d5776f block: make BlockLimits::max_pdiscard 64bit
1dc4bab block/io: allow 64bit write-zeroes requests
05ca540 block: use int64_t instead of int in driver write_zeroes handlers
5864b0d block: make BlockLimits::max_pwrite_zeroes 64bit
9698c13 block: use int64_t instead of uint64_t in copy_range driver handlers
4e60566 block: use int64_t instead of uint64_t in driver write handlers
8aa3af1 block: use int64_t instead of uint64_t in driver read handlers
fc695f9 qcow2: check request on vmstate save/load path
a13a9ef block/io: bring request check to bdrv_co_{read, write}v_vmstate
=== OUTPUT BEGIN ===
1/11 Checking commit a13a9efd128c (block/io: bring request check to bdrv_co_{read, write}v_vmstate)
ERROR: Author email address is mangled by the mailing list
#2:
Author: Vladimir Sementsov-Ogievskiy via <qemu-devel@nongnu.org>
total: 1 errors, 0 warnings, 42 lines checked
Patch 1/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/11 Checking commit fc695f91df62 (qcow2: check request on vmstate save/load path)
3/11 Checking commit 8aa3af15763f (block: use int64_t instead of uint64_t in driver read handlers)
4/11 Checking commit 4e60566f8a2c (block: use int64_t instead of uint64_t in driver write handlers)
WARNING: line over 80 characters
#379: FILE: block/nvme.c:1233:
+ QEMUIOVector *qiov, BdrvRequestFlags flags)
total: 0 errors, 1 warnings, 440 lines checked
Patch 4/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/11 Checking commit 9698c13fe02d (block: use int64_t instead of uint64_t in copy_range driver handlers)
6/11 Checking commit 5864b0dfbf2d (block: make BlockLimits::max_pwrite_zeroes 64bit)
WARNING: Block comments use a leading /* on a separate line
#56: FILE: include/block/block_int.h:679:
+ /* Maximum number of bytes that can zeroized at once. Must be multiple of
WARNING: Block comments use a trailing */ on a separate line
#57: FILE: include/block/block_int.h:680:
+ * pwrite_zeroes_alignment. May be 0 if no inherent 64-bit limit */
total: 0 errors, 2 warnings, 21 lines checked
Patch 6/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/11 Checking commit 05ca54013452 (block: use int64_t instead of int in driver write_zeroes handlers)
8/11 Checking commit 1dc4bab7ab4e (block/io: allow 64bit write-zeroes requests)
9/11 Checking commit 9d5776fcbb03 (block: make BlockLimits::max_pdiscard 64bit)
WARNING: Block comments use a leading /* on a separate line
#55: FILE: include/block/block_int.h:667:
+ /* Maximum number of bytes that can be discarded at once. Must be multiple
WARNING: Block comments use a trailing */ on a separate line
#57: FILE: include/block/block_int.h:669:
+ * inherent 64-bit limit */
total: 0 errors, 2 warnings, 24 lines checked
Patch 9/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/11 Checking commit 9b3b5c7f1465 (block: use int64_t instead of int in driver discard handlers)
11/11 Checking commit bed608a58181 (block/io: allow 64bit discard requests)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210324205132.464899-1-vsementsov@virtuozzo.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 00/11] 64bit block-layer: part II
2021-03-24 21:13 ` [PATCH v4 00/11] 64bit block-layer: part II no-reply
@ 2021-03-25 7:42 ` Vladimir Sementsov-Ogievskiy
2021-03-25 8:10 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-25 7:42 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, jsnow
25.03.2021 00:13, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20210324205132.464899-1-vsementsov@virtuozzo.com/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Type: series
> Message-id: 20210324205132.464899-1-vsementsov@virtuozzo.com
> Subject: [PATCH v4 00/11] 64bit block-layer: part II
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
>
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
> - [tag update] patchew/20210323221539.3532660-1-crosa@redhat.com -> patchew/20210323221539.3532660-1-crosa@redhat.com
> * [new tag] patchew/20210324205132.464899-1-vsementsov@virtuozzo.com -> patchew/20210324205132.464899-1-vsementsov@virtuozzo.com
> Switched to a new branch 'test'
> bed608a block/io: allow 64bit discard requests
> 9b3b5c7 block: use int64_t instead of int in driver discard handlers
> 9d5776f block: make BlockLimits::max_pdiscard 64bit
> 1dc4bab block/io: allow 64bit write-zeroes requests
> 05ca540 block: use int64_t instead of int in driver write_zeroes handlers
> 5864b0d block: make BlockLimits::max_pwrite_zeroes 64bit
> 9698c13 block: use int64_t instead of uint64_t in copy_range driver handlers
> 4e60566 block: use int64_t instead of uint64_t in driver write handlers
> 8aa3af1 block: use int64_t instead of uint64_t in driver read handlers
> fc695f9 qcow2: check request on vmstate save/load path
> a13a9ef block/io: bring request check to bdrv_co_{read, write}v_vmstate
>
> === OUTPUT BEGIN ===
> 1/11 Checking commit a13a9efd128c (block/io: bring request check to bdrv_co_{read, write}v_vmstate)
> ERROR: Author email address is mangled by the mailing list
> #2:
> Author: Vladimir Sementsov-Ogievskiy via <qemu-devel@nongnu.org>
That's a strange false-positive.
Look at 1/11: it's not mangled in any way. Looking at the source I see clean "From:" header:
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
And there is no any "Author" in the message source at all. "qemu-devel" is noted only in Cc: list.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 00/11] 64bit block-layer: part II
2021-03-25 7:42 ` Vladimir Sementsov-Ogievskiy
@ 2021-03-25 8:10 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-25 8:10 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, integration, namei.unix, dillaman, berto, eblake, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, jsnow
25.03.2021 10:42, Vladimir Sementsov-Ogievskiy wrote:
> 25.03.2021 00:13, no-reply@patchew.org wrote:
>> Patchew URL: https://patchew.org/QEMU/20210324205132.464899-1-vsementsov@virtuozzo.com/
>>
>>
>>
>> Hi,
>>
>> This series seems to have some coding style problems. See output below for
>> more information:
>>
>> Type: series
>> Message-id: 20210324205132.464899-1-vsementsov@virtuozzo.com
>> Subject: [PATCH v4 00/11] 64bit block-layer: part II
>>
>> === TEST SCRIPT BEGIN ===
>> #!/bin/bash
>> git rev-parse base > /dev/null || exit 0
>> git config --local diff.renamelimit 0
>> git config --local diff.renames True
>> git config --local diff.algorithm histogram
>> ./scripts/checkpatch.pl --mailback base..
>> === TEST SCRIPT END ===
>>
>> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
>> From https://github.com/patchew-project/qemu
>> - [tag update] patchew/20210323221539.3532660-1-crosa@redhat.com -> patchew/20210323221539.3532660-1-crosa@redhat.com
>> * [new tag] patchew/20210324205132.464899-1-vsementsov@virtuozzo.com -> patchew/20210324205132.464899-1-vsementsov@virtuozzo.com
>> Switched to a new branch 'test'
>> bed608a block/io: allow 64bit discard requests
>> 9b3b5c7 block: use int64_t instead of int in driver discard handlers
>> 9d5776f block: make BlockLimits::max_pdiscard 64bit
>> 1dc4bab block/io: allow 64bit write-zeroes requests
>> 05ca540 block: use int64_t instead of int in driver write_zeroes handlers
>> 5864b0d block: make BlockLimits::max_pwrite_zeroes 64bit
>> 9698c13 block: use int64_t instead of uint64_t in copy_range driver handlers
>> 4e60566 block: use int64_t instead of uint64_t in driver write handlers
>> 8aa3af1 block: use int64_t instead of uint64_t in driver read handlers
>> fc695f9 qcow2: check request on vmstate save/load path
>> a13a9ef block/io: bring request check to bdrv_co_{read, write}v_vmstate
>>
>> === OUTPUT BEGIN ===
>> 1/11 Checking commit a13a9efd128c (block/io: bring request check to bdrv_co_{read, write}v_vmstate)
>> ERROR: Author email address is mangled by the mailing list
>> #2:
>> Author: Vladimir Sementsov-Ogievskiy via <qemu-devel@nongnu.org>
>
> That's a strange false-positive.
>
> Look at 1/11: it's not mangled in any way. Looking at the source I see clean "From:" header:
>
> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> And there is no any "Author" in the message source at all. "qemu-devel" is noted only in Cc: list.
>
Hmm, but if look at mail on patchew, https://patchew.org/QEMU/20210324205132.464899-1-vsementsov@virtuozzo.com/20210324205132.464899-2-vsementsov@virtuozzo.com/
yes it is mangled..
I hope everyone who is in CC (as me) gets this email not-mangled.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 01/11] block/io: bring request check to bdrv_co_{read,write}v_vmstate
2021-03-24 20:51 ` [PATCH v4 01/11] block/io: bring request check to bdrv_co_{read, write}v_vmstate Vladimir Sementsov-Ogievskiy via
@ 2021-05-11 17:54 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2021-05-11 17:54 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: fam, kwolf, integration, berto, stefanha, pavel.dovgaluk, sw, pl,
qemu-devel, mreitz, jsnow, ronniesahlberg, pbonzini, namei.unix,
dillaman, ari
On 3/24/21 3:51 PM, Vladimir Sementsov-Ogievskiy wrote:
> There are only two drivers supporting vmstate: qcow2 and sheepdog.
> Sheepdog is deprecated. In qcow2 these requests go through
> .bdrv_co_p{read,write}v_part handlers.
>
> So, let's do our basic check for the request on vmstate generic
> handlers.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/io.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 02/11] qcow2: check request on vmstate save/load path
2021-03-24 20:51 ` [PATCH v4 02/11] qcow2: check request on vmstate save/load path Vladimir Sementsov-Ogievskiy
@ 2021-05-11 17:57 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2021-05-11 17:57 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: fam, kwolf, integration, berto, stefanha, pavel.dovgaluk, sw, pl,
qemu-devel, mreitz, jsnow, ronniesahlberg, pbonzini, namei.unix,
dillaman, ari
On 3/24/21 3:51 PM, Vladimir Sementsov-Ogievskiy wrote:
> We modify the request by adding an offset to vmstate. Let's check the
> modified request. It will help us to safely move .bdrv_co_preadv_part
> and .bdrv_co_pwritev_part to int64_t type of offset and bytes.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> include/block/block_int.h | 3 +++
> block/io.c | 6 +++---
> block/qcow2.c | 43 +++++++++++++++++++++++++++++++++------
> 3 files changed, 43 insertions(+), 9 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 03/11] block: use int64_t instead of uint64_t in driver read handlers
2021-03-24 20:51 ` [PATCH v4 03/11] block: use int64_t instead of uint64_t in driver read handlers Vladimir Sementsov-Ogievskiy
@ 2021-05-11 19:22 ` Eric Blake
2021-05-24 12:57 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 23+ messages in thread
From: Eric Blake @ 2021-05-11 19:22 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: fam, kwolf, integration, berto, stefanha, pavel.dovgaluk, sw, pl,
qemu-devel, mreitz, jsnow, ronniesahlberg, pbonzini, namei.unix,
dillaman, ari
On 3/24/21 3:51 PM, Vladimir Sementsov-Ogievskiy wrote:
> We are going to convert .bdrv_co_preadv_part and .bdrv_co_pwritev_part
> to int64_t type for offset and bytes parameters (as it's already done
> for generic block/io.c layer).
>
> In qcow2 .bdrv_co_preadv_part is used in some places, so let's add
> corresponding checks and assertions.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Unusual line, especially since...
>
> block: use int64_t instead of uint64_t in driver read handlers
>
> We are generally moving to int64_t for both offset and bytes parameters
> on all io paths.
>
> Main motivation is realization of 64-bit write_zeroes operation for
> fast zeroing large disk chunks, up to the whole disk.
>
> We chose signed type, to be consistent with off_t (which is signed) and
> with possibility for signed return type (where negative value means
> error).
>
> So, convert driver read handlers parameters which are already 64bit to
> signed type.
>
> While being here, convert also flags parameter to be BdrvRequestFlags.
>
> Now let's consider all callers. Simple
>
> git grep '\->bdrv_\(aio\|co\)_preadv\(_part\)\?'
>
> shows that's there three callers of driver function:
>
> bdrv_driver_preadv() in block/io.c, passes int64_t, checked by
> bdrv_check_qiov_request() to be non-negative.
>
> qcow2_load_vmstate() does bdrv_check_qiov_request().
>
> do_perform_cow_read() has uint64_t argument. And a lot of things in
> qcow2 driver are uint64_t, so converting it is big job. But we must
> not work with requests that doesn't satisfy bdrv_check_qiov_request(),
s/doesn't/don't/
> so let's just assert it here.
>
> Still, the functions may be called directly, not only by drv->...
> Let's check:
>
> git grep '\.bdrv_\(aio\|co\)_preadv\(_part\)\?\s*=' | \
> awk '{print $4}' | sed 's/,//' | sed 's/&//' | sort | uniq | \
> while read func; do git grep "$func(" | \
> grep -v "$func(BlockDriverState"; done
>
> The only one such caller:
>
> QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, &data, 1);
> ...
> ret = bdrv_replace_test_co_preadv(bs, 0, 1, &qiov, 0);
>
> in tesTS/unit/test-bdrv-drain.c, and it's OK obviously.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
...it is repeated here. I'm fine dropping the first.
> ---
> include/block/block_int.h | 11 ++++++-----
> block/backup-top.c | 4 ++--
> 35 files changed, 120 insertions(+), 89 deletions(-)
>
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index db7a909ea9..e6bcf74e46 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -234,8 +234,8 @@ struct BlockDriver {
>
> /* aio */
> BlockAIOCB *(*bdrv_aio_preadv)(BlockDriverState *bs,
> - uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
> - BlockCompletionFunc *cb, void *opaque);
> + int64_t offset, int64_t bytes, QEMUIOVector *qiov,
> + BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
> BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs,
> uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
> BlockCompletionFunc *cb, void *opaque);
> @@ -264,10 +264,11 @@ struct BlockDriver {
> * The buffer in @qiov may point directly to guest memory.
> */
> int coroutine_fn (*bdrv_co_preadv)(BlockDriverState *bs,
> - uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
> + int64_t offset, int64_t bytes, QEMUIOVector *qiov,
> + BdrvRequestFlags flags);
> int coroutine_fn (*bdrv_co_preadv_part)(BlockDriverState *bs,
> - uint64_t offset, uint64_t bytes,
> - QEMUIOVector *qiov, size_t qiov_offset, int flags);
> + int64_t offset, int64_t bytes,
> + QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
Lots of fallout, but I'm in favor of this signature change.
> +++ b/block/blkdebug.c
> @@ -614,8 +614,8 @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> }
>
> static int coroutine_fn
> -blkdebug_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> - QEMUIOVector *qiov, int flags)
> +blkdebug_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
> + QEMUIOVector *qiov, BdrvRequestFlags flags)
> {
> int err;
Still calls rule_check() with uint64_t parameters, but since we've
asserted the caller was within range, no behavior change.
> +++ b/block/blkverify.c
> @@ -221,8 +221,8 @@ blkverify_co_prwv(BlockDriverState *bs, BlkverifyRequest *r, uint64_t offset,
> }
>
> static int coroutine_fn
> -blkverify_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> - QEMUIOVector *qiov, int flags)
> +blkverify_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
> + QEMUIOVector *qiov, BdrvRequestFlags flags)
> {
Similarly for the call to blkverify_co_prwv(), and probably elsewhere in
the patch.
> +++ b/block/crypto.c
> @@ -397,8 +397,8 @@ static int block_crypto_reopen_prepare(BDRVReopenState *state,
> #define BLOCK_CRYPTO_MAX_IO_SIZE (1024 * 1024)
>
> static coroutine_fn int
> -block_crypto_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> - QEMUIOVector *qiov, int flags)
> +block_crypto_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
> + QEMUIOVector *qiov, BdrvRequestFlags flags)
> {
> BlockCrypto *crypto = bs->opaque;
> uint64_t cur_bytes; /* number of bytes in current iteration */
We could perhaps change the type of local variables like cur_bytes and
bytes_done; but it doesn't affect semantics.
> +++ b/block/curl.c
> @@ -896,7 +896,8 @@ out:
> }
>
> static int coroutine_fn curl_co_preadv(BlockDriverState *bs,
> - uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
> + int64_t offset, int64_t bytes, QEMUIOVector *qiov,
> + BdrvRequestFlags flags)
> {
> CURLAIOCB acb = {
> .co = qemu_coroutine_self(),
Likewise changing the type of CURLAIOCB.offset and .bytes. Cleanups
like that can be followups.
> +++ b/block/file-posix.c
> @@ -2033,9 +2033,9 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
> return raw_thread_pool_submit(bs, handle_aiocb_rw, &acb);
> }
>
> -static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset,
> - uint64_t bytes, QEMUIOVector *qiov,
> - int flags)
> +static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
> + int64_t bytes, QEMUIOVector *qiov,
> + BdrvRequestFlags flags)
> {
> return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_READ);
Similarly for fixing the signature of raw_co_prw() (after the
counterpart change to raw_co_pwritev)
> +++ b/block/nvme.c
> @@ -1221,8 +1221,9 @@ static int nvme_co_prw(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> }
>
> static coroutine_fn int nvme_co_preadv(BlockDriverState *bs,
> - uint64_t offset, uint64_t bytes,
> - QEMUIOVector *qiov, int flags)
> + int64_t offset, int64_t bytes,
> + QEMUIOVector *qiov,
> + BdrvRequestFlags flags)
> {
> return nvme_co_prw(bs, offset, bytes, qiov, false, flags);
> }
And for nvme_co_prw().
> +++ b/block/qcow2.c
> @@ -2281,9 +2281,10 @@ static coroutine_fn int qcow2_co_preadv_task_entry(AioTask *task)
> }
>
> static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
> - uint64_t offset, uint64_t bytes,
> + int64_t offset, int64_t bytes,
> QEMUIOVector *qiov,
> - size_t qiov_offset, int flags)
> + size_t qiov_offset,
> + BdrvRequestFlags flags)
> {
> BDRVQcow2State *s = bs->opaque;
> int ret = 0;
Unrelated to this patch; the loop sets cur_bytes = MIN(bytes, INT_MAX);
but it would be smarter to choose a cluster-aligned max instead of
INT_MAX. It doesn't matter as long as the block layer has pre-clamped
requests to be < 32 bit to begin with, and then our later call to
qcow2_get_host_offset() further clamps it down to actual cluster
boundaries. But it does look odd, compared to computing the original
MIN() to an aligned boundary in the first place, whether or not we ever
change the block layer to allow individual drivers to opt in to reading
more than 2G in one transaction.
qcow2_add_task() is another internal function worth improving in a followup.
> +++ b/block/quorum.c
> @@ -663,8 +663,8 @@ static int read_fifo_child(QuorumAIOCB *acb)
> return ret;
> }
>
> -static int quorum_co_preadv(BlockDriverState *bs, uint64_t offset,
> - uint64_t bytes, QEMUIOVector *qiov, int flags)
> +static int quorum_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
> + QEMUIOVector *qiov, BdrvRequestFlags flags)
> {
> BDRVQuorumState *s = bs->opaque;
> QuorumAIOCB *acb = quorum_aio_get(bs, qiov, offset, bytes, flags);
and quorum_aio_get().
> diff --git a/block/raw-format.c b/block/raw-format.c
> index 7717578ed6..e3f459b2cb 100644
> --- a/block/raw-format.c
> +++ b/block/raw-format.c
> @@ -181,8 +181,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;
>
> @@ -201,9 +201,9 @@ static inline int raw_adjust_offset(BlockDriverState *bs, uint64_t *offset,
> return 0;
> }
>
> -static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset,
> - uint64_t bytes, QEMUIOVector *qiov,
> - int flags)
> +static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
> + int64_t bytes, QEMUIOVector *qiov,
> + BdrvRequestFlags flags)
> {
> int ret;
>
> @@ -259,7 +259,7 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
> qiov = &local_qiov;
> }
>
> - ret = raw_adjust_offset(bs, &offset, bytes, true);
> + ret = raw_adjust_offset(bs, (int64_t *)&offset, bytes, true);
Ugly type-punning; thankfully it's transient until the counterpart patch
to driver write handlers.
> if (ret) {
> goto fail;
> }
> @@ -294,7 +294,7 @@ static int coroutine_fn raw_co_pwrite_zeroes(BlockDriverState *bs,
> {
> int ret;
>
> - ret = raw_adjust_offset(bs, (uint64_t *)&offset, bytes, true);
> + ret = raw_adjust_offset(bs, (int64_t *)&offset, bytes, true);
And now you should lose this cast...
> if (ret) {
> return ret;
> }
> @@ -306,7 +306,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);
...like you did here.
> if (ret) {
> return ret;
> }
> @@ -541,7 +541,7 @@ static int coroutine_fn raw_co_copy_range_from(BlockDriverState *bs,
> {
> int ret;
>
> - ret = raw_adjust_offset(bs, &src_offset, bytes, false);
> + ret = raw_adjust_offset(bs, (int64_t *)&src_offset, bytes, false);
> if (ret) {
> return ret;
> }
> @@ -560,7 +560,7 @@ static int coroutine_fn raw_co_copy_range_to(BlockDriverState *bs,
> {
> int ret;
>
> - ret = raw_adjust_offset(bs, &dst_offset, bytes, true);
> + ret = raw_adjust_offset(bs, (int64_t *)&dst_offset, bytes, true);
Also transient casts.
Easy enough to fix my nits for qcow2 and the commit message.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 04/11] block: use int64_t instead of uint64_t in driver write handlers
2021-03-24 20:51 ` [PATCH v4 04/11] block: use int64_t instead of uint64_t in driver write handlers Vladimir Sementsov-Ogievskiy
@ 2021-05-11 21:00 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2021-05-11 21:00 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: fam, kwolf, integration, berto, stefanha, pavel.dovgaluk, sw, pl,
qemu-devel, mreitz, jsnow, ronniesahlberg, pbonzini, namei.unix,
dillaman, ari
On 3/24/21 3:51 PM, Vladimir Sementsov-Ogievskiy wrote:
> We are generally moving to int64_t for both offset and bytes parameters
> on all io paths.
>
> Main motivation is realization of 64-bit write_zeroes operation for
> fast zeroing large disk chunks, up to the whole disk.
>
> We chose signed type, to be consistent with off_t (which is signed) and
> with possibility for signed return type (where negative value means
> error).
>
> So, convert driver write handlers parameters which are already 64bit to
> signed type.
>
> While being here, convert also flags parameter to be BdrvRequestFlags.
>
> Now let's consider all callers. Simple
>
> git grep '\->bdrv_\(aio\|co\)_pwritev\(_part\)\?'
>
> shows that's there two callers of driver function:
>
> bdrv_driver_pwritev() in block/io.c, passes int64_t, checked by
> bdrv_check_qiov_request() to be non-negative.
>
> qcow2_save_vmstate() does bdrv_check_qiov_request().
>
> Still, the functions may be called directly, not only by drv->...
> Let's check:
>
> git grep '\.bdrv_\(aio\|co\)_pwritev\(_part\)\?\s*=' | \
> awk '{print $4}' | sed 's/,//' | sed 's/&//' | sort | uniq | \
> while read func; do git grep "$func(" | \
> grep -v "$func(BlockDriverState"; done
>
> shows several callers:
...
Thanks for recording the audit.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> include/block/block_int.h | 16 ++++++++--------
> block/backup-top.c | 6 +++---
> 30 files changed, 95 insertions(+), 86 deletions(-)
>
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index e6bcf74e46..928369e0bc 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -237,8 +237,8 @@ struct BlockDriver {
> int64_t offset, int64_t bytes, QEMUIOVector *qiov,
> BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
> BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs,
> - uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
> - BlockCompletionFunc *cb, void *opaque);
> + int64_t offset, int64_t bytes, QEMUIOVector *qiov,
> + BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
> BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
> BlockCompletionFunc *cb, void *opaque);
> BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
> @@ -287,10 +287,11 @@ struct BlockDriver {
> * The buffer in @qiov may point directly to guest memory.
> */
> int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs,
> - uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
> + int64_t offset, int64_t bytes, QEMUIOVector *qiov,
> + BdrvRequestFlags flags);
> int coroutine_fn (*bdrv_co_pwritev_part)(BlockDriverState *bs,
> - uint64_t offset, uint64_t bytes,
> - QEMUIOVector *qiov, size_t qiov_offset, int flags);
> + int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset,
> + BdrvRequestFlags flags);
>
> /*
> * Efficiently zero a region of the disk image. Typically an image format
> @@ -428,10 +429,9 @@ struct BlockDriver {
> Error **errp);
>
> int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
> - uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
> + int64_t offset, int64_t bytes, QEMUIOVector *qiov);
> int coroutine_fn (*bdrv_co_pwritev_compressed_part)(BlockDriverState *bs,
> - uint64_t offset, uint64_t bytes, QEMUIOVector *qiov,
> - size_t qiov_offset);
> + int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset);
Someday it may be nice to convert remaining drivers of the old callbacks
to use the newer ones instead, for fewer callbacks to manage here. But
not the problem of this patch.
> +++ b/block/backup-top.c
> @@ -97,9 +97,9 @@ static int coroutine_fn backup_top_co_pwrite_zeroes(BlockDriverState *bs,
> }
>
> static coroutine_fn int backup_top_co_pwritev(BlockDriverState *bs,
> - uint64_t offset,
> - uint64_t bytes,
> - QEMUIOVector *qiov, int flags)
> + int64_t offset, int64_t bytes,
> + QEMUIOVector *qiov,
> + BdrvRequestFlags flags)
> {
> int ret = backup_top_cbw(bs, offset, bytes, flags);
We should clean up the signature of backup_top_cbw() in a followup, now
that pdiscard, pwrite_zeroes, pwritev all pass int64_t.
> +++ b/block/blkdebug.c
> @@ -635,8 +635,8 @@ blkdebug_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
> }
>
> static int coroutine_fn
> -blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> - QEMUIOVector *qiov, int flags)
> +blkdebug_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
> + QEMUIOVector *qiov, BdrvRequestFlags flags)
> {
> int err;
Similarly for rule_check(). I probably called out a lot of the same
spots in 3/11.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 05/11] block: use int64_t instead of uint64_t in copy_range driver handlers
2021-03-24 20:51 ` [PATCH v4 05/11] block: use int64_t instead of uint64_t in copy_range driver handlers Vladimir Sementsov-Ogievskiy
@ 2021-05-11 21:14 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2021-05-11 21:14 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: fam, kwolf, integration, berto, stefanha, pavel.dovgaluk, sw, pl,
qemu-devel, mreitz, jsnow, ronniesahlberg, pbonzini, namei.unix,
dillaman, ari
On 3/24/21 3:51 PM, Vladimir Sementsov-Ogievskiy wrote:
> We are generally moving to int64_t for both offset and bytes parameters
> on all io paths.
>
> Main motivation is realization of 64-bit write_zeroes operation for
> fast zeroing large disk chunks, up to the whole disk.
>
> We chose signed type, to be consistent with off_t (which is signed) and
> with possibility for signed return type (where negative value means
> error).
>
> So, convert driver copy_range handlers parameters which are already
> 64bit to signed type.
>
> Now let's consider all callers. Simple
>
> git grep '\->bdrv_co_copy_range'
>
> shows the only caller:
>
> bdrv_co_copy_range_internal(), which doesn bdrv_check_request32(),
s/doesn/does/
> so everything is OK.
>
> Still, the functions may be called directly, not only by drv->...
> Let's check:
>
> git grep '\.bdrv_co_copy_range_\(from\|to\)\s*=' | \
> awk '{print $4}' | sed 's/,//' | sed 's/&//' | sort | uniq | \
> while read func; do git grep "$func(" | \
> grep -v "$func(BlockDriverState"; done
>
> shows no more callers. So, we are done.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> include/block/block_int.h | 12 ++++++------
> block/file-posix.c | 10 +++++-----
> block/iscsi.c | 12 ++++++------
> block/qcow2.c | 12 ++++++------
> block/raw-format.c | 16 ++++++++--------
> 5 files changed, 31 insertions(+), 31 deletions(-)
Fewer drivers implement this, so easier review :)
> +++ b/block/qcow2.c
> @@ -3975,9 +3975,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;
The use of cur_bytes = MIN(bytes, INT_MAX) looks odd, when we could
instead clamp to a nicer aligned boundary. As noted before,
qcow2_get_host_offset() then further clamps cur_bytes, and the caller is
already splitting up requests larger than 2G, but copy_from is one of
those interfaces that is actually designed to have potentially nice
speedups with large byte ranges within the same filesystem (faster than
what is possible with usual pread/pwrite). Then again, that's probably
more for file-posix (where we know the bytes are contiguous) than qcow2
(where we do have to worry about whether clusters are contiguous), so
we'll still have to keep a while(bytes) fragmenting loop in this
function. Thoughts for a future patch, doesn't affect correctness of
this one.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 06/11] block: make BlockLimits::max_pwrite_zeroes 64bit
2021-03-24 20:51 ` [PATCH v4 06/11] block: make BlockLimits::max_pwrite_zeroes 64bit Vladimir Sementsov-Ogievskiy
@ 2021-05-11 21:19 ` Eric Blake
2021-05-12 6:33 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 23+ messages in thread
From: Eric Blake @ 2021-05-11 21:19 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: fam, kwolf, integration, berto, stefanha, pavel.dovgaluk, sw, pl,
qemu-devel, mreitz, jsnow, ronniesahlberg, pbonzini, namei.unix,
dillaman, ari
On 3/24/21 3:51 PM, Vladimir Sementsov-Ogievskiy wrote:
> We are going to support 64 bit write-zeroes requests. Now update the
> limit variable. It's absolutely safe. The variable is set in some
> drivers, and used in bdrv_co_do_pwrite_zeroes().
>
> Update also max_write_zeroes variable in bdrv_co_do_pwrite_zeroes(), so
> that bdrv_co_do_pwrite_zeroes() is now prepared to 64bit requests. The
> remaining logic including num, offset and bytes variables is already
> supporting 64bit requests.
>
> So the only thing that prevents 64 bit requests is limiting
> max_write_zeroes variable to INT_MAX in bdrv_co_do_pwrite_zeroes().
> We'll drop this limitation after updating all block drivers.
>
> Ah, we also have bdrv_check_request32() in bdrv_co_pwritev_part(). It
> will be modified to do bdrv_check_request() for write-zeroes path.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> include/block/block_int.h | 7 +++----
> block/io.c | 2 +-
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> +++ b/include/block/block_int.h
> @@ -676,10 +676,9 @@ typedef struct BlockLimits {
> * that is set. May be 0 if bl.request_alignment is good enough */
> uint32_t pdiscard_alignment;
>
> - /* Maximum number of bytes that can zeroized at once (since it is
> - * signed, it must be < 2G, if set). Must be multiple of
> - * pwrite_zeroes_alignment. May be 0 if no inherent 32-bit limit */
> - int32_t max_pwrite_zeroes;
> + /* Maximum number of bytes that can zeroized at once. Must be multiple of
> + * pwrite_zeroes_alignment. May be 0 if no inherent 64-bit limit */
Is the comment still right?
Leaving as 0 is the easiest way for a driver to say "default limit", but
I would feel safer with the default being 2G-align rather than 63-bit
limit. And it is a 63-bit limit, not 64-bit, if the driver opts in to
INT64_MAX.
> + int64_t max_pwrite_zeroes;
>
> /* Optimal alignment for write zeroes requests in bytes. A power
> * of 2 is best but not mandatory. Must be a multiple of
> diff --git a/block/io.c b/block/io.c
> index 55095dd08e..79e600af27 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1836,7 +1836,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
> int head = 0;
> int tail = 0;
>
> - int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX);
> + int64_t max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX);
You are correct that for now we have no behavior change; a driver opting
in to a larger limit will still be clamped until we revisit this patch
later to drop the MIN() - but I agree with your approach of keeping
MIN() here until all drivers are audited.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 06/11] block: make BlockLimits::max_pwrite_zeroes 64bit
2021-05-11 21:19 ` Eric Blake
@ 2021-05-12 6:33 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-12 6:33 UTC (permalink / raw)
To: Eric Blake, qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, jsnow
12.05.2021 00:19, Eric Blake wrote:
> On 3/24/21 3:51 PM, Vladimir Sementsov-Ogievskiy wrote:
>> We are going to support 64 bit write-zeroes requests. Now update the
>> limit variable. It's absolutely safe. The variable is set in some
>> drivers, and used in bdrv_co_do_pwrite_zeroes().
>>
>> Update also max_write_zeroes variable in bdrv_co_do_pwrite_zeroes(), so
>> that bdrv_co_do_pwrite_zeroes() is now prepared to 64bit requests. The
>> remaining logic including num, offset and bytes variables is already
>> supporting 64bit requests.
>>
>> So the only thing that prevents 64 bit requests is limiting
>> max_write_zeroes variable to INT_MAX in bdrv_co_do_pwrite_zeroes().
>> We'll drop this limitation after updating all block drivers.
>>
>> Ah, we also have bdrv_check_request32() in bdrv_co_pwritev_part(). It
>> will be modified to do bdrv_check_request() for write-zeroes path.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> include/block/block_int.h | 7 +++----
>> block/io.c | 2 +-
>> 2 files changed, 4 insertions(+), 5 deletions(-)
>>
>
>> +++ b/include/block/block_int.h
>> @@ -676,10 +676,9 @@ typedef struct BlockLimits {
>> * that is set. May be 0 if bl.request_alignment is good enough */
>> uint32_t pdiscard_alignment;
>>
>> - /* Maximum number of bytes that can zeroized at once (since it is
>> - * signed, it must be < 2G, if set). Must be multiple of
>> - * pwrite_zeroes_alignment. May be 0 if no inherent 32-bit limit */
>> - int32_t max_pwrite_zeroes;
>> + /* Maximum number of bytes that can zeroized at once. Must be multiple of
>> + * pwrite_zeroes_alignment. May be 0 if no inherent 64-bit limit */
>
> Is the comment still right?
>
> Leaving as 0 is the easiest way for a driver to say "default limit", but
> I would feel safer with the default being 2G-align rather than 63-bit
> limit. And it is a 63-bit limit, not 64-bit, if the driver opts in to
> INT64_MAX.
May be, just s/no inherent 64-bit limit/no limit/ ?
I think that no-limit is better default: let's motivate drivers to be 64bit. And if they don't want, they should specify limit by hand.
Hmm, also, you missed v5 of this series, rebased on master. There almost no changes, so your answers should apply to it as well.
>
>> + int64_t max_pwrite_zeroes;
>>
>> /* Optimal alignment for write zeroes requests in bytes. A power
>> * of 2 is best but not mandatory. Must be a multiple of
>> diff --git a/block/io.c b/block/io.c
>> index 55095dd08e..79e600af27 100644
>> --- a/block/io.c
>> +++ b/block/io.c
>> @@ -1836,7 +1836,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
>> int head = 0;
>> int tail = 0;
>>
>> - int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX);
>> + int64_t max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX);
>
> You are correct that for now we have no behavior change; a driver opting
> in to a larger limit will still be clamped until we revisit this patch
> later to drop the MIN() - but I agree with your approach of keeping
> MIN() here until all drivers are audited.
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 03/11] block: use int64_t instead of uint64_t in driver read handlers
2021-05-11 19:22 ` Eric Blake
@ 2021-05-24 12:57 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 23+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-24 12:57 UTC (permalink / raw)
To: Eric Blake, qemu-block
Cc: qemu-devel, integration, namei.unix, dillaman, berto, pl,
ronniesahlberg, fam, sw, stefanha, pbonzini, pavel.dovgaluk, ari,
mreitz, kwolf, jsnow
11.05.2021 22:22, Eric Blake wrote:
> On 3/24/21 3:51 PM, Vladimir Sementsov-Ogievskiy wrote:
>> We are going to convert .bdrv_co_preadv_part and .bdrv_co_pwritev_part
>> to int64_t type for offset and bytes parameters (as it's already done
>> for generic block/io.c layer).
>>
>> In qcow2 .bdrv_co_preadv_part is used in some places, so let's add
>> corresponding checks and assertions.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> Unusual line, especially since...
>
>>
>> block: use int64_t instead of uint64_t in driver read handlers
>>
>> We are generally moving to int64_t for both offset and bytes parameters
>> on all io paths.
>>
>> Main motivation is realization of 64-bit write_zeroes operation for
>> fast zeroing large disk chunks, up to the whole disk.
>>
>> We chose signed type, to be consistent with off_t (which is signed) and
>> with possibility for signed return type (where negative value means
>> error).
>>
>> So, convert driver read handlers parameters which are already 64bit to
>> signed type.
>>
>> While being here, convert also flags parameter to be BdrvRequestFlags.
>>
>> Now let's consider all callers. Simple
>>
>> git grep '\->bdrv_\(aio\|co\)_preadv\(_part\)\?'
>>
>> shows that's there three callers of driver function:
>>
>> bdrv_driver_preadv() in block/io.c, passes int64_t, checked by
>> bdrv_check_qiov_request() to be non-negative.
>>
>> qcow2_load_vmstate() does bdrv_check_qiov_request().
>>
>> do_perform_cow_read() has uint64_t argument. And a lot of things in
>> qcow2 driver are uint64_t, so converting it is big job. But we must
>> not work with requests that doesn't satisfy bdrv_check_qiov_request(),
>
> s/doesn't/don't/
>
>> so let's just assert it here.
>>
>> Still, the functions may be called directly, not only by drv->...
>> Let's check:
>>
>> git grep '\.bdrv_\(aio\|co\)_preadv\(_part\)\?\s*=' | \
>> awk '{print $4}' | sed 's/,//' | sed 's/&//' | sort | uniq | \
>> while read func; do git grep "$func(" | \
>> grep -v "$func(BlockDriverState"; done
>>
>> The only one such caller:
>>
>> QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, &data, 1);
>> ...
>> ret = bdrv_replace_test_co_preadv(bs, 0, 1, &qiov, 0);
>>
>> in tesTS/unit/test-bdrv-drain.c, and it's OK obviously.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> ...it is repeated here. I'm fine dropping the first.
>
>> ---
>> include/block/block_int.h | 11 ++++++-----
>> block/backup-top.c | 4 ++--
>
>> 35 files changed, 120 insertions(+), 89 deletions(-)
>>
>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> index db7a909ea9..e6bcf74e46 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -234,8 +234,8 @@ struct BlockDriver {
>>
>> /* aio */
>> BlockAIOCB *(*bdrv_aio_preadv)(BlockDriverState *bs,
>> - uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
>> - BlockCompletionFunc *cb, void *opaque);
>> + int64_t offset, int64_t bytes, QEMUIOVector *qiov,
>> + BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque);
>> BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs,
>> uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
>> BlockCompletionFunc *cb, void *opaque);
>> @@ -264,10 +264,11 @@ struct BlockDriver {
>> * The buffer in @qiov may point directly to guest memory.
>> */
>> int coroutine_fn (*bdrv_co_preadv)(BlockDriverState *bs,
>> - uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
>> + int64_t offset, int64_t bytes, QEMUIOVector *qiov,
>> + BdrvRequestFlags flags);
>> int coroutine_fn (*bdrv_co_preadv_part)(BlockDriverState *bs,
>> - uint64_t offset, uint64_t bytes,
>> - QEMUIOVector *qiov, size_t qiov_offset, int flags);
>> + int64_t offset, int64_t bytes,
>> + QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
>
> Lots of fallout, but I'm in favor of this signature change.
>
>
>> +++ b/block/blkdebug.c
>> @@ -614,8 +614,8 @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
>> }
>>
>> static int coroutine_fn
>> -blkdebug_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
>> - QEMUIOVector *qiov, int flags)
>> +blkdebug_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
>> + QEMUIOVector *qiov, BdrvRequestFlags flags)
>> {
>> int err;
>
> Still calls rule_check() with uint64_t parameters, but since we've
> asserted the caller was within range, no behavior change.
>
>> +++ b/block/blkverify.c
>> @@ -221,8 +221,8 @@ blkverify_co_prwv(BlockDriverState *bs, BlkverifyRequest *r, uint64_t offset,
>> }
>>
>> static int coroutine_fn
>> -blkverify_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
>> - QEMUIOVector *qiov, int flags)
>> +blkverify_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
>> + QEMUIOVector *qiov, BdrvRequestFlags flags)
>> {
>
> Similarly for the call to blkverify_co_prwv(), and probably elsewhere in
> the patch.
>
>> +++ b/block/crypto.c
>> @@ -397,8 +397,8 @@ static int block_crypto_reopen_prepare(BDRVReopenState *state,
>> #define BLOCK_CRYPTO_MAX_IO_SIZE (1024 * 1024)
>>
>> static coroutine_fn int
>> -block_crypto_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
>> - QEMUIOVector *qiov, int flags)
>> +block_crypto_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
>> + QEMUIOVector *qiov, BdrvRequestFlags flags)
>> {
>> BlockCrypto *crypto = bs->opaque;
>> uint64_t cur_bytes; /* number of bytes in current iteration */
>
> We could perhaps change the type of local variables like cur_bytes and
> bytes_done; but it doesn't affect semantics.
>
>> +++ b/block/curl.c
>> @@ -896,7 +896,8 @@ out:
>> }
>>
>> static int coroutine_fn curl_co_preadv(BlockDriverState *bs,
>> - uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
>> + int64_t offset, int64_t bytes, QEMUIOVector *qiov,
>> + BdrvRequestFlags flags)
>> {
>> CURLAIOCB acb = {
>> .co = qemu_coroutine_self(),
>
> Likewise changing the type of CURLAIOCB.offset and .bytes. Cleanups
> like that can be followups.
>
>> +++ b/block/file-posix.c
>> @@ -2033,9 +2033,9 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
>> return raw_thread_pool_submit(bs, handle_aiocb_rw, &acb);
>> }
>>
>> -static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset,
>> - uint64_t bytes, QEMUIOVector *qiov,
>> - int flags)
>> +static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
>> + int64_t bytes, QEMUIOVector *qiov,
>> + BdrvRequestFlags flags)
>> {
>> return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_READ);
>
> Similarly for fixing the signature of raw_co_prw() (after the
> counterpart change to raw_co_pwritev)
>
>> +++ b/block/nvme.c
>> @@ -1221,8 +1221,9 @@ static int nvme_co_prw(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
>> }
>>
>> static coroutine_fn int nvme_co_preadv(BlockDriverState *bs,
>> - uint64_t offset, uint64_t bytes,
>> - QEMUIOVector *qiov, int flags)
>> + int64_t offset, int64_t bytes,
>> + QEMUIOVector *qiov,
>> + BdrvRequestFlags flags)
>> {
>> return nvme_co_prw(bs, offset, bytes, qiov, false, flags);
>> }
>
> And for nvme_co_prw().
>
>> +++ b/block/qcow2.c
>> @@ -2281,9 +2281,10 @@ static coroutine_fn int qcow2_co_preadv_task_entry(AioTask *task)
>> }
>>
>> static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
>> - uint64_t offset, uint64_t bytes,
>> + int64_t offset, int64_t bytes,
>> QEMUIOVector *qiov,
>> - size_t qiov_offset, int flags)
>> + size_t qiov_offset,
>> + BdrvRequestFlags flags)
>> {
>> BDRVQcow2State *s = bs->opaque;
>> int ret = 0;
>
> Unrelated to this patch; the loop sets cur_bytes = MIN(bytes, INT_MAX);
> but it would be smarter to choose a cluster-aligned max instead of
> INT_MAX. It doesn't matter as long as the block layer has pre-clamped
> requests to be < 32 bit to begin with, and then our later call to
> qcow2_get_host_offset() further clamps it down to actual cluster
> boundaries. But it does look odd, compared to computing the original
> MIN() to an aligned boundary in the first place, whether or not we ever
> change the block layer to allow individual drivers to opt in to reading
> more than 2G in one transaction.
>
> qcow2_add_task() is another internal function worth improving in a followup.
>
>> +++ b/block/quorum.c
>> @@ -663,8 +663,8 @@ static int read_fifo_child(QuorumAIOCB *acb)
>> return ret;
>> }
>>
>> -static int quorum_co_preadv(BlockDriverState *bs, uint64_t offset,
>> - uint64_t bytes, QEMUIOVector *qiov, int flags)
>> +static int quorum_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
>> + QEMUIOVector *qiov, BdrvRequestFlags flags)
>> {
>> BDRVQuorumState *s = bs->opaque;
>> QuorumAIOCB *acb = quorum_aio_get(bs, qiov, offset, bytes, flags);
>
> and quorum_aio_get().
>
>> diff --git a/block/raw-format.c b/block/raw-format.c
>> index 7717578ed6..e3f459b2cb 100644
>> --- a/block/raw-format.c
>> +++ b/block/raw-format.c
>> @@ -181,8 +181,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;
>>
>> @@ -201,9 +201,9 @@ static inline int raw_adjust_offset(BlockDriverState *bs, uint64_t *offset,
>> return 0;
>> }
>>
>> -static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset,
>> - uint64_t bytes, QEMUIOVector *qiov,
>> - int flags)
>> +static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
>> + int64_t bytes, QEMUIOVector *qiov,
>> + BdrvRequestFlags flags)
>> {
>> int ret;
>>
>> @@ -259,7 +259,7 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, uint64_t offset,
>> qiov = &local_qiov;
>> }
>>
>> - ret = raw_adjust_offset(bs, &offset, bytes, true);
>> + ret = raw_adjust_offset(bs, (int64_t *)&offset, bytes, true);
>
> Ugly type-punning; thankfully it's transient until the counterpart patch
> to driver write handlers.
>
>> if (ret) {
>> goto fail;
>> }
>> @@ -294,7 +294,7 @@ static int coroutine_fn raw_co_pwrite_zeroes(BlockDriverState *bs,
>> {
>> int ret;
>>
>> - ret = raw_adjust_offset(bs, (uint64_t *)&offset, bytes, true);
>> + ret = raw_adjust_offset(bs, (int64_t *)&offset, bytes, true);
>
> And now you should lose this cast...
>
>> if (ret) {
>> return ret;
>> }
>> @@ -306,7 +306,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);
>
> ...like you did here.
>
>> if (ret) {
>> return ret;
>> }
>> @@ -541,7 +541,7 @@ static int coroutine_fn raw_co_copy_range_from(BlockDriverState *bs,
>> {
>> int ret;
>>
>> - ret = raw_adjust_offset(bs, &src_offset, bytes, false);
>> + ret = raw_adjust_offset(bs, (int64_t *)&src_offset, bytes, false);
>> if (ret) {
>> return ret;
>> }
>> @@ -560,7 +560,7 @@ static int coroutine_fn raw_co_copy_range_to(BlockDriverState *bs,
>> {
>> int ret;
>>
>> - ret = raw_adjust_offset(bs, &dst_offset, bytes, true);
>> + ret = raw_adjust_offset(bs, (int64_t *)&dst_offset, bytes, true);
>
> Also transient casts.
>
> Easy enough to fix my nits for qcow2 and the commit message.
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
But you said that qcow2 note is unrelated to that patch. So, as I understand, only commit message
and raw_co_pwrite_zeroes (drop extra case) should be fixed here, yes?
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2021-05-24 12:58 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-24 20:51 [PATCH v4 00/11] 64bit block-layer: part II Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 01/11] block/io: bring request check to bdrv_co_{read, write}v_vmstate Vladimir Sementsov-Ogievskiy via
2021-05-11 17:54 ` [PATCH v4 01/11] block/io: bring request check to bdrv_co_{read,write}v_vmstate Eric Blake
2021-03-24 20:51 ` [PATCH v4 02/11] qcow2: check request on vmstate save/load path Vladimir Sementsov-Ogievskiy
2021-05-11 17:57 ` Eric Blake
2021-03-24 20:51 ` [PATCH v4 03/11] block: use int64_t instead of uint64_t in driver read handlers Vladimir Sementsov-Ogievskiy
2021-05-11 19:22 ` Eric Blake
2021-05-24 12:57 ` Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 04/11] block: use int64_t instead of uint64_t in driver write handlers Vladimir Sementsov-Ogievskiy
2021-05-11 21:00 ` Eric Blake
2021-03-24 20:51 ` [PATCH v4 05/11] block: use int64_t instead of uint64_t in copy_range driver handlers Vladimir Sementsov-Ogievskiy
2021-05-11 21:14 ` Eric Blake
2021-03-24 20:51 ` [PATCH v4 06/11] block: make BlockLimits::max_pwrite_zeroes 64bit Vladimir Sementsov-Ogievskiy
2021-05-11 21:19 ` Eric Blake
2021-05-12 6:33 ` Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 07/11] block: use int64_t instead of int in driver write_zeroes handlers Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 08/11] block/io: allow 64bit write-zeroes requests Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 09/11] block: make BlockLimits::max_pdiscard 64bit Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 10/11] block: use int64_t instead of int in driver discard handlers Vladimir Sementsov-Ogievskiy
2021-03-24 20:51 ` [PATCH v4 11/11] block/io: allow 64bit discard requests Vladimir Sementsov-Ogievskiy
2021-03-24 21:13 ` [PATCH v4 00/11] 64bit block-layer: part II no-reply
2021-03-25 7:42 ` Vladimir Sementsov-Ogievskiy
2021-03-25 8:10 ` Vladimir Sementsov-Ogievskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).