Re: [Qemu-block] [PATCH] block: Swap request limit definitions

[Max Reitz]

On 13.02.2017 09:39, Alberto Garcia wrote:
> On Sun 12 Feb 2017 02:47:24 AM CET, Max Reitz <address@hidden> wrote:
> 
>> -#define BDRV_REQUEST_MAX_SECTORS MIN(SIZE_MAX >> BDRV_SECTOR_BITS, \
>> -                                     INT_MAX >> BDRV_SECTOR_BITS)
>> -#define BDRV_REQUEST_MAX_BYTES (BDRV_REQUEST_MAX_SECTORS << 
>> BDRV_SECTOR_BITS)
>> +#define BDRV_REQUEST_MAX_BYTES      MIN(SIZE_MAX, INT_MAX)
>> +#define BDRV_REQUEST_MAX_SECTORS    (BDRV_REQUEST_MAX_BYTES >> 
>> BDRV_SECTOR_BITS)
> 
> I'm just pointing it out because I don't know if this can cause
> problems, but this patch would make BDRV_REQUEST_MAX_BYTES not a
> multiple of the sector size (INT_MAX is actually a prime number).

Very good point. I don't think this could be an issue, though. For one
thing, the use of BDRV_REQUEST_MAX_BYTES is very limited.

Apart from that, I guess the main issue would be that if you send an
unaligned head/tail the automatic round-up would exceed
BDRV_REQUEST_MAX_SECTORS. But this is a pre-existing issue, actually:
When you issue a request with an unaligned head and 2G - 512 bytes
length, bdrv_co_pwritev() will align it to 2G (which is greater than
INT_MAX).

(Can be tested trivially with:

$ qemu-io -c 'write 511 2147483136' --image-opts driver=null-co,size=4G

and some debugging code in block/io.c)

You can make the request even bigger by increasing the
bs->bl.request_alignment.

Not a big issue, though, as bdrv_aligned_p{read,write}v() actually take
an unsigned int and break down requests greater than INT_MAX
automatically. As long as bs->bl.request_alignment stays under 1G or so
we'll be fine (i.e. as long as we don't get an unsigned int overflow) --
and when it breaks it won't be because BDRV_REQUEST_MAX_BYTES is not
aligned to sectors.

Max

signature.asc
Description: OpenPGP digital signature