01.04.2022 23:17, Eric Blake wrote:
> On Fri, Apr 01, 2022 at 01:08:01PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> v3: rebase on master, one patch is already merged.
>>
>> Vladimir Sementsov-Ogievskiy (3):
>> block: block_dirty_bitmap_merge(): fix error path
>> block: improve block_dirty_bitmap_merge(): don't allocate extra bitmap
>> block: simplify handling of try to merge different sized bitmaps
>
> Is any of this series worth getting into 7.0, or are we safe letting
> it slide to 7.1?
>
Let's check.
Only first patch is a fix.
anon bitmap is created on same bs where dst is found, so they should be compatible in size.
But bdrv_merge_dirty_bitmap also do some checks on dst status, which may actually fail..
So, in bad case we set errp, but return non-NULL dst bitmap.
Look at callers of block_dirty_bitmap_merge:
1. qmp_block_dirty_bitmap_merge() is OK, it ignores return value.
2. qmp_transaction use local_err to detect error, so we'll go through error path, that's good. state->bitmap is set, but it's not really matter. What makes sense is state->backup set or not?
state->backup is initialized with zero, as qmp_transaction() use g_malloc0 to allocate state buffer.
And bdrv_merge_dirty_bitmap() will do all checks prior to call bdrv_dirty_bitmap_merge_internal(), which actually can set @backup. So, state->backup is not set in our bad case.
So that all should be OK to postpone for 7.1.
--
Best regards,
Vladimir