[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH] util/hbitmap: fix unaligned reset
Date: Tue, 6 Aug 2019 13:47:55 +0000

06.08.2019 16:30, John Snow wrote:
> On 8/6/19 8:39 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 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?
> Downstream products always have time and room to get additional fixes; I
> think this is supported from an upstream POV so we should investigate this.
> I assume migration/block has the same problem that it fully clears
> unaligned blocks?

Hmm, after closer look, it seems like it's OK. It just a bit more difficult to
see than in other places with reset.

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]