Re: [Qemu-devel] [PATCH v3 1/8] dirty-bitmap: improve bdrv_dirty_bitmap_next_zero

[John Snow]

On 09/10/2018 01:00 PM, Vladimir Sementsov-Ogievskiy wrote:
> 10.09.2018 19:55, Eric Blake wrote:
>> On 9/10/18 11:49 AM, Vladimir Sementsov-Ogievskiy wrote:
>>
>>>>> -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.
>>>
>>> interface with constant end parameter is more comfortable for loop:
>>> we don't need to update 'bytes' parameter on each iteration
>>
>> But there's still the question of WHO should be calculating end. Your
>> interface argues for the caller:
>>
>> hbitmap_next_zero(start, start + bytes)
>>
>> int64_t hbitmap_next_zero(...)
>> {
>>     while (offset != end) ...
>> }
>>
>> while we're asking about a consistent interface for the caller (if
>> most callers already have a 'bytes' rather than an 'end' computed):
>>
>> hbitmap_next_zero(start, bytes)
>>
>> int64_t hbitmap_next_zero(...)
>> {
>>     int64_t end = start + bytes;
>>     while (offset != end) ...
>> }
>>
> 
> Yes, that's an issue. Ok, if you are not comfortable with start,end, I
> can switch to start,bytes.
> 

The series looks pretty close, I can merge the next version if you think
it's worth changing the interface.

--js