On Mon, Mar 30, 2020 at 05:18:17PM +0300, Vladimir Sementsov-Ogievskiy wrote: > @@ -875,9 +875,9 @@ static bool coroutine_fn > bdrv_wait_serialising_requests(BdrvTrackedRequest *self > } > > static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset, > - size_t size) > + int64_t bytes) > { > - if (size > BDRV_REQUEST_MAX_BYTES) { > + if (offset < 0 || bytes < 0 || bytes > BDRV_REQUEST_MAX_BYTES) { > return -EIO; > } > > @@ -885,10 +885,6 @@ static int bdrv_check_byte_request(BlockDriverState *bs, > int64_t offset, > return -ENOMEDIUM; > } > > - if (offset < 0) { > - return -EIO; > - } > - > return 0; > }
Moving this if statement was unnecessary. I'm not sure if there are cases where callers will now see EIO instead of ENOMEDIUM and change their behavior :(. More conservative code changes are easier to review and merge because they are less risky. > @@ -1743,7 +1740,7 @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child, > } > > static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, > - int64_t offset, int bytes, BdrvRequestFlags flags) > + int64_t offset, int64_t bytes, BdrvRequestFlags flags) > { > BlockDriver *drv = bs->drv; > QEMUIOVector qiov; > @@ -1773,7 +1770,7 @@ static int coroutine_fn > bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, > assert(max_write_zeroes >= bs->bl.request_alignment); > > while (bytes > 0 && !ret) { > - int num = bytes; > + int64_t num = bytes; > > /* Align request. Block drivers can expect the "bulk" of the request > * to be aligned, and that unaligned requests do not cross cluster > @@ -1799,6 +1796,7 @@ static int coroutine_fn > bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, > ret = -ENOTSUP; > /* First try the efficient write zeroes operation */ > if (drv->bdrv_co_pwrite_zeroes) { > + assert(num <= INT_MAX); max_write_zeroes already enforces this, so the assertion is always false: int max_write_zeroes = MIN_NON_ZERO(bs->bl.max_pwrite_zeroes, INT_MAX); ... /* limit request size */ if (num > max_write_zeroes) { num = max_write_zeroes; }
signature.asc
Description: PGP signature