[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
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v2 16/17] block: Split bdrv_merge_limits() from bdrv_refresh_limits(), (continued)
Re: [Qemu-block] [PATCH v2 00/17] Byte-based block limits, Kevin Wolf, 2016/06/21