Re: [Qemu-devel] [PATCH v2 8/9] blkdebug: Add ability to override unmap geometries

[Eric Blake]

On 11/21/2016 03:11 PM, Eric Blake wrote:

>>> @@ -398,6 +429,53 @@ static int blkdebug_open(BlockDriverState *bs, QDict 
>>> *options, int flags,
>>>          ret = -EINVAL;
>>>          goto fail_unref;
>>>      }
>>> +    max_transfer = qemu_opt_get_size(opts, "max-transfer", 0);
>>> +    if (max_transfer < INT_MAX &&
>>> +        QEMU_IS_ALIGNED(max_transfer, MAX(align, BDRV_SECTOR_SIZE))) {
>>> +        s->max_transfer = max_transfer;
>>> +    } else if (max_transfer) {
>>> +        error_setg(errp, "Invalid argument");
>>
>> Could you be more specific? Same in all of the error_setg() calls below.
>>
>>> +        ret = -EINVAL;
>>> +        goto fail_unref;
>>> +    }
>>
>> Also, the way this is formatted seems not intuitive to me. I know it's
>> the same as it's been done for "align", but normally I'd use the following:
>>
>> s->value = qemu_opt_get_size(...);
>> if (s->value is set and invalid) {
>>     /* error out */
>> }
> 
> I'll see what I can do.

Unfortunately, part of the problem is type casting.  qemu_opt_get_size()
returns a 64-bit number, but s->align is 'int'. You can't detect
wraparound unless you store into a temporary and check bounds prior to
assigning to the narrower type.  I guess I could always change the
struct to store 64-bit values that have been validated to fit within 32
bits.

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

signature.asc
Description: OpenPGP digital signature