Re: [Qemu-devel] [RFC PATCHv2] block: optimize zero writes with bdrv_write_zeroes

[Peter Lieven <pl@kamp.de>]

Am 21.03.2014 15:50, schrieb Eric Blake:
> On 03/21/2014 08:36 AM, Peter Lieven wrote:
>> this patch tries to optimize zero write requests
>> by automatically using bdrv_write_zeroes if it is
>> supported by the format.
>>
>> this should significantly speed up file system initialization and
>> should speed zero write test used to test backend storage performance.
>>
>> the difference can simply be tested by e.g.
>>
>> dd if=/dev/zero of=/dev/vdX bs=1M
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>> v1->v2: - add detect-zeroes=off|on|unmap knob to drive cmdline parameter
>>         - call zero detection only for format (bs->file != NULL)
>>
>> +static int bdrv_set_detect_zeroes(BlockDriverState *bs,
>> +                                  const char *detect_zeroes,
>> +                                  Error **errp)
>> +{
>> +    if (!detect_zeroes || !strncmp(detect_zeroes, "off", 3)) {
>> +        bs->detect_zeroes = BDRV_DETECT_ZEROES_OFF;
> This parses "offer"
>
>> +    } else if (!strncmp(detect_zeroes, "on", 2)) {
>> +        bs->detect_zeroes = BDRV_DETECT_ZEROES_ON;
> and this parses "onto".
>
>> +    } else if (!strncmp(detect_zeroes, "unmap", 5)) {
>> +        bs->detect_zeroes = BDRV_DETECT_ZEROES_UNMAP;
> In all three cases, shouldn't you be using strcmp() instead of strncmp()?

yes, you are right.

Thanks,
Peter

>
>> +    } else {
>> +        error_setg(errp, "invalid value for detect-zeroes: %s",
>> +                   detect_zeroes);
> Especially since you warn about other unknown spellings, it feels weird
> to not warn about the spellings where the prefix matches but the overall
> spelling is unknown.
>>  file sectors into the image file.
>> +@item detect-zeroes=@var{detect-zeroes}
>> +@var{detect-zeroes} is "off", "on" or "unmap" and enables the automatic
>> +conversion of plain zero writes by the OS to driver specific optimized
>> +zero write commands. If "unmap" is choosen and @var{discard} is "on"
> s/choosen/chosen/
>