Re: [Qemu-devel] [PATCH 02/13] block: Track write zero limits in bytes

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2428 bytes --]

On 05/25/2016 04:30 AM, Kevin Wolf wrote:
> Am 25.05.2016 um 00:25 hat Eric Blake geschrieben:
>> Another step towards removing sector-based interfaces: convert
>> the maximum write and minimum alignment values from sectorss to
> 
> s/sectorss/sectors/
> 
>> bytes.  Alignment is changed to 'int', since it makes no sense
>> to have an alignment larger than the maximum write.  Add an
>> assert that no one was trying to use sectors to get a write
>> zeroes larger than 2G.  Rename the variables to let the compiler
>> check that all users are converted to the new semantics.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -1706,12 +1706,10 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp)
>>      }
>>
>>      if (iscsilun->bl.max_ws_len < 0xffffffff) {
>> -        bs->bl.max_write_zeroes =
>> -            sector_limits_lun2qemu(iscsilun->bl.max_ws_len, iscsilun);
>> +        bs->bl.max_pwrite_zeroes = iscsilun->bl.max_ws_len;
> 
> Wrong unit, I think. You need to multiply by iscsi_lun->block_size.

Hmm, I think you're right.  What's more, I need to make sure the result
doesn't wrap around INT_MAX (a device with 4k block size that supports
8G limits via 2M max blocks should still allow up to 2G transactions
from qemu). I'm also thinking that in v2, it may be easier to reason
about alignment limits if I convert alignment numbers to uint32_t,
although we are still capped by INT_MAX in our various blk_* interfaces
(worrying about signed overflow is a pain).

>> +++ b/block/qcow2.c
>> @@ -1193,7 +1193,7 @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
>>  {
>>      BDRVQcow2State *s = bs->opaque;
>>
>> -    bs->bl.write_zeroes_alignment = s->cluster_sectors;
>> +    bs->bl.pwrite_zeroes_alignment = s->cluster_sectors << BDRV_SECTOR_BITS;
> 
> This is s->cluster_size. I hope to get rid of s->cluster_sectors
> eventually. :-)

Should I go ahead and convert ALL of BlockLimits to be byte-based
limits, rather than an odd mix of sector vs. byte limits?  Should I add
any assertions for power-of-2 limits?  Do we want to allow 0x80000000 as
a valid length limit?

[/me Should I be regretting touching this can of worms in the first
place? :) ]

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]