[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 17/22] block: Switch discard length bounds to
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v3 17/22] block: Switch discard length bounds to byte-based |
Date: |
Fri, 24 Jun 2016 08:15:53 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 06/24/2016 12:43 AM, Fam Zheng wrote:
> On Thu, 06/23 16:37, Eric Blake wrote:
>> Sector-based limits are awkward to think about; in our on-going
>> quest to move to byte-based interfaces, convert max_discard and
>> discard_alignment. Rename them, using 'pdiscard' as an aid to
>> track which remaining discard interfaces need conversion, and so
>> that the compiler will help us catch the change in semantics
>> across any rebased code. The BlockLimits type is now completely
>> byte-based; and in iscsi.c, sector_limits_lun2qemu() is no
>> longer needed.
>>
>> pdiscard_alignment is made unsigned (we use power-of-2 alignments
>> as bitmasks, where unsigned is easier to think about) while
>> leaving max_pdiscard signed (since we still have an 'int'
>> interface); this is comparable to what commit cf081fc did for
>> write zeroes limits. We may later want to make everything an
>> unsigned 64-bit limit - but that requires a bigger code audit.
>>
>> - /* optimal alignment for discard requests in sectors */
>> - int64_t discard_alignment;
>> + /* optimal alignment for discard requests in bytes, must be power
>> + * of 2, less than max_discard if that is set, and multiple of
>
> s/max_discard/max_pdiscard/
Maintainer could touch it up on pull request.
>> /* align request */
>> - if (bs->bl.discard_alignment &&
>> - num >= bs->bl.discard_alignment &&
>> - sector_num % bs->bl.discard_alignment) {
>> - if (num > bs->bl.discard_alignment) {
>> - num = bs->bl.discard_alignment;
>> + if (discard_alignment &&
>> + num >= discard_alignment &&
>> + sector_num % discard_alignment) {
>> + if (num > discard_alignment) {
>> + num = discard_alignment;
>> }
>> - num -= sector_num % bs->bl.discard_alignment;
>> + num -= sector_num % discard_alignment;
>
> Or just
>
> num = discard_alignment - sector_num % discard_alignment;
>
> without the if.
>
Sure. It all gets simplified later when I switch to bdrv_co_pdiscard().
Up to the maintainer.
> Otherwise looks good,
>
> Reviewed-by: Fam Zheng <address@hidden>
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature