[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v2 08/17] blkdebug: Set request_alignment during

From: Eric Blake
Subject: Re: [Qemu-block] [PATCH v2 08/17] blkdebug: Set request_alignment during .bdrv_refresh_limits()
Date: Wed, 22 Jun 2016 11:33:54 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

On 06/22/2016 04:05 AM, Kevin Wolf wrote:

>>>>      /* Set request alignment */
>>>> -    align = qemu_opt_get_size(opts, "align", bs->request_alignment);
>>>> -    if (align > 0 && align < INT_MAX && !(align & (align - 1))) {
>>>> -        bs->request_alignment = align;
>>>> +    align = qemu_opt_get_size(opts, "align",
>>>> +                              bs->request_alignment ?: BDRV_SECTOR_SIZE);

>>> I think you should either just unconditionally use BDRV_SECTOR_SIZE or
>>> even better just store 0 in s->align if the option isn't given. Your
>>> implementation of blkdebug_refresh_limits() already leaves the default
>>> bs->request_alignment alone if s->align == 0.
>> I like that idea; I guess that means I instead need to tweak the logic
>> to test if "align" is present in opts (in which case it must be
>> non-zero), or absent (in which case leaving things as 0 is a nicer
>> approach than trying to guess which default to stick things to).
> Except that you don't have to check all of that explicitly, the default
> value for absent options is what the third parameter is for:
>     align = qemu_opt_get_size(opts, "align", 0);

Previously, we explicitly reject the user passing in an explicit
'align':0.  If I go with this approach of 0 as the default, then I may
need to tweak docs.  But that sounds better, so I'll try it.

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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]