Re: [Qemu-devel] [PATCH v4 12/20] qed: Switch to .bdrv_co_block_status()

[Eric Blake <eblake@redhat.com>]

On 11/30/2017 04:27 AM, Vladimir Sementsov-Ogievskiy wrote:
> 12.10.2017 21:59, Eric Blake wrote:
>> We are gradually moving away from sector-based interfaces, towards
>> byte-based.  Update the qed driver accordingly, taking the opportunity
>> to inline qed_is_allocated_cb() into its lone caller (the callback
>> used to be important, until we switched qed to coroutines).  There is
>> no intent to optimize based on the want_zero flag for this format.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>

>>   {
>>       BDRVQEDState *s = bs->opaque;
>> -    size_t len = (size_t)nb_sectors * BDRV_SECTOR_SIZE;
>> -    QEDIsAllocatedCB cb = {
>> -        .bs = bs,
>> -        .pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE,
>> -        .status = BDRV_BLOCK_OFFSET_MASK,
>> -        .pnum = pnum,
>> -        .file = file,
>> -    };
>> +    size_t len;
> 
> size_t len = bytes;
> 

Or rather, size_t len = MIN(bytes, SIZE_MAX); thanks to 32-bit platforms.

> with that:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
>> +    int status;
>>       QEDRequest request = { .l2_table = NULL };
>>       uint64_t offset;
>>       int ret;
>>
>>       qemu_co_mutex_lock(&s->table_lock);
>> -    ret = qed_find_cluster(s, &request, cb.pos, &len, &offset);
>> -    qed_is_allocated_cb(&cb, ret, offset, len);
>> +    ret = qed_find_cluster(s, &request, pos, &len, &offset);
> 
> len is in-out parameter, you can't use it uninitialized.

Good catch.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org