Am 22.06.2016 um 00:26 hat Eric Blake geschrieben:
> On 06/21/2016 08:16 AM, Kevin Wolf wrote:
> > Am 14.06.2016 um 23:30 hat Eric Blake geschrieben:
> >> It makes more sense to have ALL block size limit constraints
> >> in the same struct. Improve the documentation while at it.
> >>
> >> Signed-off-by: Eric Blake <ebl...@redhat.com>
> >>
> >> ---
>
> >> struct BlockLimits {
> >> + /* Alignment requirement, in bytes, for offset/length of I/O
> >> + * requests. Must be a power of 2 less than INT_MAX. A value of 0
> >> + * defaults to 1 for drivers with modern byte interfaces, and to
> >> + * 512 otherwise. */
> >
> > No, a value of zero probably crashes qemu. The defaults apply when they
> > aren't overridden by the driver, but this field is always non-zero.
> >
>
> Then why does block.c have:
>
> --- a/block.c
> +++ b/block.c
> @@ -1016,7 +1016,7 @@ static int bdrv_open_common(BlockDriverState *bs,
> BdrvChild *file,
>
> assert(bdrv_opt_mem_align(bs) != 0);
> assert(bdrv_min_mem_align(bs) != 0);
> - assert(is_power_of_2(bs->request_alignment) || bdrv_is_sg(bs));
> + assert(is_power_of_2(bs->bl.request_alignment) || bdrv_is_sg(bs));
>
> That says that if bdrv_is_sg(bs), we can indeed have request_alignment
> be 0. Should I track that down and fix it first, so that
> request_alignment is always nonzero? If so, is using '1' for
> bdrv_is_sg(bs) the ideal solution?Hm, yes, forgot about this. bdrv_is_sg(bs) means that we never use the I/O functions, so there is no point in specifying any alignment. If we want to say something about 0 in the comment maybe mention bs->sg explicitly and that you can't use read/write functions then. But in fact, I think even sg devices already get a non-zero alignment, so the second part of the assertion might be redundant. Didn't test it, though, just had a quick look at the raw-posix code. Kevin
pgp5N4U19TZJ7.pgp
Description: PGP signature
