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

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

Hi!

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

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

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

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

Any ideas?

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

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

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


-- 
Best regards,
Vladimir