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