Re: [Qemu-block] [PATCH] util/hbitmap: fix unaligned reset

[Vladimir Sementsov-Ogievskiy]

03.08.2019 0:19, Max Reitz wrote:
> On 02.08.19 20:58, Vladimir Sementsov-Ogievskiy wrote:
>> hbitmap_reset is broken: it rounds up the requested region. It leads to
>> the following bug, which is shown by fixed test:
>>
>> assume granularity = 2
>> set(0, 3) # count becomes 4
>> reset(0, 1) # count becomes 2
>>
>> But user of the interface assume that virtual bit 1 should be still
>> dirty, so hbitmap should report count to be 4!
>>
>> In other words, because of granularity, when we set one "virtual" bit,
>> yes, we make all "virtual" bits in same chunk to be dirty. But this
>> should not be so for reset.
>>
>> Fix this, aligning bound correctly.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> ---
>>
>> Hi all!
>>
>> Hmm, is it a bug or feature? :)
>> I don't have a test for mirror yet, but I think that sync mirror may be 
>> broken
>> because of this, as do_sync_target_write() seems to be using unaligned reset.
> 
> Crap.
> 
> 
> Yes, you’re right.  This would fix it, and it wouldn’t fix it in the
> worst way.
> 
> But I don’t know whether this patch is the best way forward still.  I
> think call hbitmap_reset() with unaligned boundaries generally calls for
> trouble, as John has laid out.  If mirror’s do_sync_target_write() is
> the only offender right now, 

Another thing is migration/block. Should we care of it, is it supported at all?


-- 
Best regards,
Vladimir