On 26/03/2018 12:24, Vladimir Sementsov-Ogievskiy wrote:
> 23.03.2018 19:42, Paolo Bonzini wrote:
>> QLIST_REMOVE does not require walking the list, and once the "bitmap"
>> argument is removed from bdrv_do_release_matching_dirty_bitmap_locked
>> the code simplifies a lot and it is worth inlining everything in the
>> callers of bdrv_do_release_matching_dirty_bitmap.
>>
>> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
>> ---
>> block/dirty-bitmap.c | 81
>> +++++++++++++++++++---------------------------------
>> 1 file changed, 30 insertions(+), 51 deletions(-)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index 13bb5066a4..c0fb49bf7b 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -250,48 +250,15 @@ void
>> bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap)
>> }
>> /* Called within bdrv_dirty_bitmap_lock..unlock */
>
> Worth adding "Called with BQL taken" to the comment?
Yes, good idea.
>> +static void bdrv_release_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap)
> worth adding "Called with BQL taken." to the comment?
> bdrv_release_named_dirty_bitmaps functions has it.
>
>> void bdrv_release_persistent_dirty_bitmaps(BlockDriverState *bs)
This is pre-existing, but it's also a good idea.
Paolo
>> - bdrv_do_release_matching_dirty_bitmap(bs, NULL,
>> -
>> bdrv_dirty_bitmap_get_persistance);
>> + BdrvDirtyBitmap *bm, *next;
>> +
>> + bdrv_dirty_bitmaps_lock(bs);
>> + QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
>> + if (bdrv_dirty_bitmap_get_persistance(bm)) {
>> + bdrv_release_dirty_bitmap_locked(bm);
>> + }
>> + }
>> + bdrv_dirty_bitmaps_unlock(bs);
>> }
>> /**
>
> anyway,
>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
>
