On Thu 30 Apr 2020 01:10:20 PM CEST, 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 wrappers parameters which are already 64bit to > signed type. > > Patch-correctness audit by Eric Blake: > > bdrv_driver_preadv > > Remains 64 bits, the question is whether any caller could pass in > something larger than 63 bits. Callers are: > > bdrv_co_do_copy_on_readv() - passes 'int64_t pnum', but sets pnum > in a fragmenting loop, MAX_BOUNCE_BUFFER when copy-on-read is > needed, or max_transfer bounded by BDRV_REQUEST_MAX_BYTES > otherwise > bdrv_aligned_preadv() - passes 'unsigned int bytes' further limited > by fragmenting loop on max_transfer <= INT_MAX > > Input is bounded to < 2G, internal use of 'bytes' is: > > drv->bdrv_co_preadv_part(uint64_t) - safe > qemu_iovec_init_slice(size_t) - potential truncation on 32-bit > platform [*] > drv->bdrv_co_preadv(uint64_t) - safe > drv->bdrv_aio_preadv(uint64_t) - safe > drv->bdrv_co_readv(int) after assertion of <2G - safe > > bdrv_driver_pwritev > > Remains 64 bits, the question is whether any caller could pass in > something larger than 63. Callers are: > > bdrv_co_do_copy_on_readv() - passes 'int64_t pnum', but set in a > fragmenting loop bounded by MAX_BOUNCE_BUFFER > bdrv_co_do_pwrite_zeroes() - passes 'int num' > bdrv_aligned_pwritev() - passes 'unsigned int bytes' further > limited by fragmenting loop on max_transfer <= INT_MAX > > Input is bounded to < 2G, internal use of 'bytes' is: > > drv->bdrv_co_pwritev_part(uint64_t) - safe > qemu_iovec_init_slice(size_t) - potential truncation on 32-bit > platform [*] > drv->bdrv_co_pwritev(uint64_t) - safe > drv->bdrv_aio_pwritev(uint64_t) - safe > drv->bdrv_co_writev(int) after assertion of <2G - safe > > bdrv_driver_pwritev_compressed > > bdrv_aligned_pwritev() - passes 'unsigned int bytes' > > Input is bounded to < 4G, internal use of 'bytes' is: > > drv->bdrv_co_pwritev_compressed_part(uint64_t) - safe > drv->bdrv_co_pwritev_compressed(uint64_t) - safe > qemu_iovec_init_slice(size_t) - potential truncation on 32-bit > platform [*] > > [*]: QEMUIOVector is in-RAM data, so size_t seems a valid type for > it's operation. To avoid truncations we should require max_transfer to > be not greater than SIZE_MAX, limiting RAM-occupying operations and > QEMUIOVector usage. Still, 64bit discard and write_zeroes (which > doesn't use QEMUIOVector) should work even on 32bit machines, not being > limited by max_transfer. > > For now, we safe anyway, as all input goes through > bdrv_aligned_pwritev() and bdrv_aligned_preadv(), which are limiting > max_transfer to INT_MAX. > > Series: 64bit-block-status > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > Reviewed-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Alberto Garcia <be...@igalia.com> Berto