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

[Eric Blake]

On 11/17/2016 05:02 PM, Max Reitz wrote:
> On 17.11.2016 21:14, Eric Blake wrote:
>> Make it easier to simulate the Dell Equallogic iSCSI with its
> 
> Somehow I feel bad putting company and product names into commit messages...

Not the first time I've done it - see commit b8d0a980.

Keeping it in the commit message is one thing (so I probably won't
change this one), but having it in the iotest is another (so I probably
will rework the comments in 9/9 to avoid the specific mention).

> 
>> unusual 15M preferred and maximum unmap and write zero sizing,
>> or to simulate Linux loopback block devices enforcing a small
>> max_transfer of 64k, by allowing blkdebug to wrap any other
>> device with further restrictions on various alignments.
>>

>> +        {
>> +            .name = "opt-write-zero",
>> +            .type = QEMU_OPT_SIZE,
>> +            .help = "Optimum write zero size in bytes",
> 
> s/size/alignment/?

Yes, will fix.

>> @@ -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.

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

signature.asc
Description: OpenPGP digital signature