Re: [PATCH v2] migration/dirty-bitmaps: change bitmap enumeration method

[John Snow]

(off list)

On 12/9/19 10:22 AM, John Snow wrote:
> 
> 
> On 12/6/19 5:31 PM, Vladimir Sementsov-Ogievskiy wrote:
>> 14.05.2019 23:19, John Snow wrote:
>>> Shift from looking at every root BDS to *every* BDS. This will migrate
>>> bitmaps that are attached to blockdev created nodes instead of just ones
>>> attached to emulated storage devices.
>>>
>>> Note that this will not migrate anonymous or internal-use bitmaps, as
>>> those are defined as having no name.
>>>
>>> This will also fix the Coverity issues Peter Maydell has been asking
>>> about for the past several releases, as well as fixing a real bug.
>>>
>>> Reported-by: Peter Maydell <address@hidden>
>>> Reported-by: Coverity 😅
>>
>> What was the coverity number (I don't believe that it was smile:)?
>>
> 
>     Reported-by: Peter Maydell <address@hidden>
>     Reported-by: Coverity 😅
>     Reported-by: aihua liang <address@hidden>
>     Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>     Signed-off-by: John Snow <address@hidden>
>     Message-id: address@hidden
>     Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1652490
>     Fixes: Coverity CID 1390625
>     CC: Stefan Hajnoczi <address@hidden>
>     Signed-off-by: John Snow <address@hidden>
> 
> 
>> Do someone know, that this patch fixes very-very-very terrible bug?
>>
>> Before this patch, here were bdrv_next-based loop, with exists from it,
>> but not using bdrv_next_cleanup(). This leads to leaked (incremented) refcnt 
>> of
>> bds on any failure during this loop!
>>
>> Now we faced this bug, in Rhel-based Qemu, so I strongly recommend to fix it 
>> in Rhel.
> 
> OK, this was fixed for 4.1, and was introduced in b35ebdf076d for
> 2.12.0, so all versions between have the problem.
> 

As far as I know, we don't "support" incremental backup for RHEL based
packages, because we only support what you can do directly through
libvirt. And since RHEL libvirt doesn't have incremental backup, ...

I can try to fix it anyway, though, if it makes your life easier especially.

Which version(s) are you using? I'll try to target a fix for that
version, but it will likely be a special fix that just fixes the leak
without changing the enumeration method, to keep migration ABI
consistent with what we expect from the different versions.

--js