On Mon, 11/10 09:33, Markus Armbruster wrote: > Fam Zheng <f...@redhat.com> writes: > > > In bdrv_rw_co we report -EINVAL for nb_sectors > INT_MAX / > > BDRV_SECTOR_SIZE, so a caller shouldn't exceed it.
I noticed this while testing unmap / zero write with scsi_debug: # dd if=/dev/zero of=/tmp/a bs=1M count=32 # modprobe scsi_debug dev_size_mb=1024 lbpu=1 # qemu-img convert -t none /tmp/a /dev/sde qemu-img: error writing zeroes at sector 0: Invalid argument > > It's not obvious to me why we do that there. iovec member iov_len is > size_t, not int. Deeper in the call stack we use int bytes everywhere: static int coroutine_fn bdrv_co_do_preadv(BlockDriverState *bs, int64_t offset, unsigned int bytes, QEMUIOVector *qiov, BdrvRequestFlags flags); static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs, int64_t offset, unsigned int bytes, QEMUIOVector *qiov, BdrvRequestFlags flags); static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs, BdrvTrackedRequest *req, int64_t offset, unsigned int bytes, int64_t align, QEMUIOVector *qiov, int flags) So this is a easier way to fix the specific bug :) > > > Signed-off-by: Fam Zheng <f...@redhat.com> > > --- > > block.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/block.c b/block.c > > index dacd881..5513379 100644 > > --- a/block.c > > +++ b/block.c > > @@ -2790,8 +2790,8 @@ int bdrv_make_zero(BlockDriverState *bs, > > BdrvRequestFlags flags) > > if (nb_sectors <= 0) { > > return 0; > > } > > - if (nb_sectors > INT_MAX) { > > - nb_sectors = INT_MAX; > > + if (nb_sectors > INT_MAX / BDRV_SECTOR_SIZE) { > > + nb_sectors = INT_MAX / BDRV_SECTOR_SIZE; > > } > > ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n); > > if (ret < 0) { > > Noticed while checkout out bdrv_get_block_status(): function comment of > bdrv_co_get_block_status() claims it returns true/false. It doesn't. > Yeah, we should fix that. Fam