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

[John Snow]

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.

--js