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

[Vladimir Sementsov-Ogievskiy]

09.12.2019 18:26, John Snow wrote:
> (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

Ah, missed it, sorry.

>>      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.

No, actually, it's no real difference, now we fixed it in our branch.
So, it's up to you.

> 
> Which version(s) are you using?

2.12.0-33

> 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
> 


-- 
Best regards,
Vladimir