Re: [PATCH 0/5] block/dirty-bitmap: check number and size constraints against queued bitmaps

[Vladimir Sementsov-Ogievskiy]

09.10.2019 23:44, John Snow wrote:
> 
> 
> On 10/9/19 2:57 PM, Eric Blake wrote:
>> On 6/6/19 1:41 PM, John Snow wrote:
>>> When adding new persistent dirty bitmaps, we only check constraints
>>> against currently stored bitmaps, and ignore the pending number and size
>>> of any bitmaps yet to be stored.
>>>
>>> Rework the "can_store" and "remove" interface to explicit "add" and
>>> "remove",
>>> and begin keeping track of the queued burden when adding new bitmaps.
>>>
>>> Reported-by: aihua liang <address@hidden>
>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1712636
>>>
>>> John Snow (5):
>>>     block/qcow2-bitmap: allow bitmap_list_load to return an error code
>>>     block/dirty-bitmap: Refactor bdrv_can_store_new_bitmap
>>>     block/dirty-bitmap: rework bdrv_remove_persistent_dirty_bitmap
>>>     block/qcow2-bitmap: Count queued bitmaps towards nb_bitmaps
>>>     block/qcow2-bitmap: Count queued bitmaps towards directory_size
>>
>> Is this series worth reviving as a v2, as it solves a corner-case bug?
>>
>>
> 
> Yes, but IIRC there were some disagreements about the methodology for
> the fix, but can't recall exactly what right now.
> 
> The way I remember it is that I wanted to make our qcow2 functions more
> like "do_store_bitmap" and "do_remove_bitmap" for direct addition and
> removal as I find that conceptual model 'simpler'.
> 
> (I think it had something to do with additional complexities in the
> different contexts that list_load is used in for when and if it performs
> certain consistency checks...?)
> 
> I think Vladimir wanted to use a pending-flush style cache to check
> requirements against instead of actual writes.
> 

Actually, here is my counter-proposal:

[PATCH] qcow2-bitmaps: fix qcow2_can_store_new_dirty_bitmap
     <address@hidden>
     https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg01696.html


-- 
Best regards,
Vladimir