[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero |
Date: |
Mon, 10 Sep 2018 11:49:44 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 09/10/2018 10:59 AM, Eric Blake wrote:
> On 9/7/18 4:49 PM, John Snow wrote:
>>
>>
>> On 08/14/2018 08:14 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> Add bytes parameter to the function, to limit searched range.
>>>
>>
>> I'm going to assume that Eric Blake has been through here and commented
>> on the interface itself.
>
> Actually, I haven't had time to look at this series in depth. Do you
> need me to?
>
Not necessarily, it's just that I didn't read v1 or v2 so I was just
being cautious against recommending changes that maybe we already
recommended against in a different direction.
Historically you've cared the most about start/end/offset/bytes naming
and conventions, so I just made an assumption.
--js
>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>> ---
>>> include/block/dirty-bitmap.h | 3 ++-
>>> include/qemu/hbitmap.h | 10 ++++++++--
>>> block/backup.c | 2 +-
>>> block/dirty-bitmap.c | 5 +++--
>>> nbd/server.c | 2 +-
>>> tests/test-hbitmap.c | 2 +-
>>> util/hbitmap.c | 25 ++++++++++++++++++++-----
>>> 7 files changed, 36 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>>> index 259bd27c40..27f5299c4e 100644
>>> --- a/include/block/dirty-bitmap.h
>>> +++ b/include/block/dirty-bitmap.h
>>> @@ -98,7 +98,8 @@ bool
>>> bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
>>> BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
>>> BdrvDirtyBitmap *bitmap);
>>> char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error
>>> **errp);
>>> -int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap,
>>> uint64_t start);
>>> +int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap,
>>> uint64_t start,
>>> + int64_t end);
>
> It's already seeming a bit odd to mix uint64_t AND int64_t for the two
> parameters. Is the intent to allow -1 to mean "end of the bitmap
> instead of a specific end range"? But you can get that with UINT64_MAX
> just as easily, and still get away with spelling it -1 in the source.
>
>
>>> + * the bitmap end.
>>> */
>>> -int64_t hbitmap_next_zero(const HBitmap *hb, uint64_t start);
>>> +int64_t hbitmap_next_zero(const HBitmap *hb, uint64_t start, int64_t
>>> end);
>>>
>>
>> The interface looks weird because we can define a 'start' that's beyond
>> the 'end'.
>>
>> I realize that you need a signed integer for 'end' to signify EOF...
>> should we do a 'bytes' parameter instead? (Did you already do that in an
>> earlier version and we changed it?)
>>
>> Well, it's not a big deal to me personally.
>
> It should always be possible to convert in either direction between
> [start,end) and [start,start+bytes); it boils down to a question of
> convenience (which form is easier for the majority of callers) and
> consistency (which form do we use more frequently in the block layer). I
> haven't checked closely, but I think start+bytes is more common than end
> in our public block layer APIs.
>
- Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero, John Snow, 2018/09/07
- Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero, Eric Blake, 2018/09/10
- Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero,
John Snow <=
- Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero, Vladimir Sementsov-Ogievskiy, 2018/09/10
- Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero, Eric Blake, 2018/09/10
- Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero, Vladimir Sementsov-Ogievskiy, 2018/09/10
- Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero, John Snow, 2018/09/14
- Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero, Vladimir Sementsov-Ogievskiy, 2018/09/14
- Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero, John Snow, 2018/09/14
- Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero, John Snow, 2018/09/14