[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 17/22] block: Switch discard length bounds to
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [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
- [Qemu-devel] [PATCH v3 05/22] nbd: Advertise realistic limits to block layer, (continued)
- [Qemu-devel] [PATCH v3 05/22] nbd: Advertise realistic limits to block layer, Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 07/22] scsi: Advertise limits by blocksize, not 512, Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 01/22] block: Tighter assertions on bdrv_aligned_pwritev(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 13/22] block: Set request_alignment during .bdrv_refresh_limits(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 10/22] iscsi: Set request_alignment during .bdrv_refresh_limits(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 15/22] block: Switch transfer length bounds to byte-based, Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 17/22] block: Switch discard length bounds to byte-based, Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 14/22] block: Set default request_alignment during bdrv_refresh_limits(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 21/22] block: Fix error message style, Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 03/22] block: Fix harmless off-by-one in bdrv_aligned_preadv(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 12/22] raw-win32: Set request_alignment during .bdrv_refresh_limits(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 11/22] qcow2: Set request_alignment during .bdrv_refresh_limits(), Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 22/22] block: Use bool as appropriate for BDS members, Eric Blake, 2016/06/23
- [Qemu-devel] [PATCH v3 19/22] block: Split bdrv_merge_limits() from bdrv_refresh_limits(), Eric Blake, 2016/06/23