Re: [Qemu-devel] [PATCH] block: Fix max nb_sectors in bdrv_make_zero

[Markus Armbruster]

Fam Zheng <address@hidden> writes:

> On Mon, 11/10 09:33, Markus Armbruster wrote:
>> Fam Zheng <address@hidden> 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 :)

Okay, that's enough for my R-by.

Aside: using bytes makes sense, using int not so much.

Reviewed-by: Markus Armbruster <address@hidden>

[...]