Re: [Qemu-devel] [PATCH 12/13] vmdk: Convert to bdrv_co_pwrite_zeroes()

[Eric Blake <eblake@redhat.com>]

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

On 05/25/2016 08:23 AM, Kevin Wolf wrote:
> Am 25.05.2016 um 00:25 hat Eric Blake geschrieben:
>> Another step on our continuing quest to switch to byte-based
>> interfaces.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>  block/vmdk.c | 13 ++++++-------
>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/block/vmdk.c b/block/vmdk.c
>> index 8494d63..284d7a0 100644
>> --- a/block/vmdk.c
>> +++ b/block/vmdk.c
>> @@ -1704,15 +1704,14 @@ static int vmdk_write_compressed(BlockDriverState *bs,
>>      }
>>  }
>>
>> -static int coroutine_fn vmdk_co_write_zeroes(BlockDriverState *bs,
>> -                                             int64_t sector_num,
>> -                                             int nb_sectors,
>> -                                             BdrvRequestFlags flags)
>> +static int coroutine_fn vmdk_co_pwrite_zeroes(BlockDriverState *bs,
>> +                                              int64_t offset,
>> +                                              int count,
>> +                                              BdrvRequestFlags flags)
>>  {
>>      int ret;
>>      BDRVVmdkState *s = bs->opaque;
>> -    uint64_t offset = sector_num * BDRV_SECTOR_SIZE;
>> -    uint64_t bytes = nb_sectors * BDRV_SECTOR_SIZE;
>> +    uint64_t bytes = count;
> 
> That's an unnecessary variable again. Whether you decide to change it or
> not:
> 
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Unnecessary, except that it is 64-bit instead of the block layer
interface 32-bit, and I didn't want to have to think too hard about how
'bytes' was used in the rest of the function if I used the narrower type
from the get-go.  I also think that 'int count' is fishy, because it
forces us to think about negative values and placating code sanitizers
on undefined shift values; maybe we'd be better with making all byte
interfaces use 'uint32_t' (but still limiting ourselves to 0x80000000 or
2G for any power-of-two limit, and 0xffffffff size transactions would
not be possible if request_alignment is larger than 1).  If we made that
switch, I'd still want to keep 0 as a no-op transaction, and not a
special case for a 4G transaction.  Still, now might be the time to do it.

-- 
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 --]