| Message ID | 20211006131718.214235-11-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | block: 64bit blk io | expand |
On Wed, Oct 06, 2021 at 03:17:16PM +0200, Vladimir Sementsov-Ogievskiy wrote: > 1. Convert bytes in BlkAioEmAIOCB: > aio->bytes is only passed to already int64_t interfaces, and set in > blk_aio_prwv, which is updated here. > > 2. For all updated functions parameter type becomes wider so callers > are safe. > > 3. In blk_aio_prwv we only store bytes to BlkAioEmAIOCB, which is > updated here. > > 4. Other updated functions are wrappers on blk_aio_prwv. > > Note that blk_aio_preadv and blk_aio_pwritev become safer: before this > commit, it's theoretically possible to pass qiov with size exceeding > INT_MAX, which than converted to int argument of blk_aio_prwv. Now it's > converted to int64_t which is a lot better. Still add assertions. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > include/sysemu/block-backend.h | 4 ++-- > block/block-backend.c | 13 ++++++++----- > 2 files changed, 10 insertions(+), 7 deletions(-) > > @@ -1530,6 +1531,7 @@ BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset, > QEMUIOVector *qiov, BdrvRequestFlags flags, > BlockCompletionFunc *cb, void *opaque) > { > + assert(qiov->size <= INT64_MAX); I hope this doesn't cause 32-bit compilers to warn about an always-true expression; but if it does, we'll figure something out. That's not enough for me to ask for you to respin this, though, so: Reviewed-by: Eric Blake <eblake@redhat.com>
10/6/21 23:29, Eric Blake wrote: > On Wed, Oct 06, 2021 at 03:17:16PM +0200, Vladimir Sementsov-Ogievskiy wrote: >> 1. Convert bytes in BlkAioEmAIOCB: >> aio->bytes is only passed to already int64_t interfaces, and set in >> blk_aio_prwv, which is updated here. >> >> 2. For all updated functions parameter type becomes wider so callers >> are safe. >> >> 3. In blk_aio_prwv we only store bytes to BlkAioEmAIOCB, which is >> updated here. >> >> 4. Other updated functions are wrappers on blk_aio_prwv. >> >> Note that blk_aio_preadv and blk_aio_pwritev become safer: before this >> commit, it's theoretically possible to pass qiov with size exceeding >> INT_MAX, which than converted to int argument of blk_aio_prwv. Now it's >> converted to int64_t which is a lot better. Still add assertions. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> include/sysemu/block-backend.h | 4 ++-- >> block/block-backend.c | 13 ++++++++----- >> 2 files changed, 10 insertions(+), 7 deletions(-) >> >> @@ -1530,6 +1531,7 @@ BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset, >> QEMUIOVector *qiov, BdrvRequestFlags flags, >> BlockCompletionFunc *cb, void *opaque) >> { >> + assert(qiov->size <= INT64_MAX); > > I hope this doesn't cause 32-bit compilers to warn about an > always-true expression; but if it does, we'll figure something out. > That's not enough for me to ask for you to respin this, though, so: > > Reviewed-by: Eric Blake <eblake@redhat.com> > So, here we need assert((uint64_t)qiov->size <= INT64_MAX); as in recent fix by Hanna. Eric, will you stage this as continuation of 64bit series, or do we wait for Kevin or Hanna, or for me resending it with this small fix and your wording fixes?
On Tue, Oct 12, 2021 at 07:13:23PM +0300, Vladimir Sementsov-Ogievskiy wrote: > > > @@ -1530,6 +1531,7 @@ BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset, > > > QEMUIOVector *qiov, BdrvRequestFlags flags, > > > BlockCompletionFunc *cb, void *opaque) > > > { > > > + assert(qiov->size <= INT64_MAX); > > > > I hope this doesn't cause 32-bit compilers to warn about an > > always-true expression; but if it does, we'll figure something out. > > That's not enough for me to ask for you to respin this, though, so: > > > > Reviewed-by: Eric Blake <eblake@redhat.com> > > > > So, here we need > > assert((uint64_t)qiov->size <= INT64_MAX); > > as in recent fix by Hanna. > Indeed. > Eric, will you stage this as continuation of 64bit series, or do we wait for Kevin or Hanna, or for me resending it with this small fix and your wording fixes? At this point, I think I'm fine touching up the series and staging it through my tree.
13.10.2021 00:37, Eric Blake wrote: > On Tue, Oct 12, 2021 at 07:13:23PM +0300, Vladimir Sementsov-Ogievskiy wrote: >>>> @@ -1530,6 +1531,7 @@ BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset, >>>> QEMUIOVector *qiov, BdrvRequestFlags flags, >>>> BlockCompletionFunc *cb, void *opaque) >>>> { >>>> + assert(qiov->size <= INT64_MAX); >>> >>> I hope this doesn't cause 32-bit compilers to warn about an >>> always-true expression; but if it does, we'll figure something out. >>> That's not enough for me to ask for you to respin this, though, so: >>> >>> Reviewed-by: Eric Blake <eblake@redhat.com> >>> >> >> So, here we need >> >> assert((uint64_t)qiov->size <= INT64_MAX); >> >> as in recent fix by Hanna. >> > > Indeed. > >> Eric, will you stage this as continuation of 64bit series, or do we wait for Kevin or Hanna, or for me resending it with this small fix and your wording fixes? > > At this point, I think I'm fine touching up the series and staging it > through my tree. > Great, thanks!
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 134c442754..979829b325 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -157,7 +157,7 @@ static inline int coroutine_fn blk_co_pwrite(BlockBackend *blk, int64_t offset, int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset, int64_t bytes, BdrvRequestFlags flags); BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset, - int bytes, BdrvRequestFlags flags, + int64_t bytes, BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque); int blk_make_zero(BlockBackend *blk, BdrvRequestFlags flags); int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int bytes); @@ -174,7 +174,7 @@ BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t offset, BlockCompletionFunc *cb, void *opaque); BlockAIOCB *blk_aio_flush(BlockBackend *blk, BlockCompletionFunc *cb, void *opaque); -BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk, int64_t offset, int bytes, +BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes, BlockCompletionFunc *cb, void *opaque); void blk_aio_cancel(BlockAIOCB *acb); void blk_aio_cancel_async(BlockAIOCB *acb); diff --git a/block/block-backend.c b/block/block-backend.c index f051ea00e9..ef0f65be4b 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1380,7 +1380,7 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk, typedef struct BlkAioEmAIOCB { BlockAIOCB common; BlkRwCo rwco; - int bytes; + int64_t bytes; bool has_returned; } BlkAioEmAIOCB; @@ -1412,7 +1412,8 @@ static void blk_aio_complete_bh(void *opaque) blk_aio_complete(acb); } -static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes, +static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, + int64_t bytes, void *iobuf, CoroutineEntry co_entry, BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque) @@ -1469,10 +1470,10 @@ static void blk_aio_write_entry(void *opaque) } BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset, - int count, BdrvRequestFlags flags, + int64_t bytes, BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque) { - return blk_aio_prwv(blk, offset, count, NULL, blk_aio_write_entry, + return blk_aio_prwv(blk, offset, bytes, NULL, blk_aio_write_entry, flags | BDRV_REQ_ZERO_WRITE, cb, opaque); } @@ -1530,6 +1531,7 @@ BlockAIOCB *blk_aio_preadv(BlockBackend *blk, int64_t offset, QEMUIOVector *qiov, BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque) { + assert(qiov->size <= INT64_MAX); return blk_aio_prwv(blk, offset, qiov->size, qiov, blk_aio_read_entry, flags, cb, opaque); } @@ -1538,6 +1540,7 @@ BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t offset, QEMUIOVector *qiov, BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque) { + assert(qiov->size <= INT64_MAX); return blk_aio_prwv(blk, offset, qiov->size, qiov, blk_aio_write_entry, flags, cb, opaque); } @@ -1618,7 +1621,7 @@ static void blk_aio_pdiscard_entry(void *opaque) } BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk, - int64_t offset, int bytes, + int64_t offset, int64_t bytes, BlockCompletionFunc *cb, void *opaque) { return blk_aio_prwv(blk, offset, bytes, NULL, blk_aio_pdiscard_entry, 0,
1. Convert bytes in BlkAioEmAIOCB: aio->bytes is only passed to already int64_t interfaces, and set in blk_aio_prwv, which is updated here. 2. For all updated functions parameter type becomes wider so callers are safe. 3. In blk_aio_prwv we only store bytes to BlkAioEmAIOCB, which is updated here. 4. Other updated functions are wrappers on blk_aio_prwv. Note that blk_aio_preadv and blk_aio_pwritev become safer: before this commit, it's theoretically possible to pass qiov with size exceeding INT_MAX, which than converted to int argument of blk_aio_prwv. Now it's converted to int64_t which is a lot better. Still add assertions. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- include/sysemu/block-backend.h | 4 ++-- block/block-backend.c | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-)