Re: [Qemu-devel] [PATCH 09/22] block: introduce persistent dirty bitmaps

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

On 12.10.2016 14:38, Vladimir Sementsov-Ogievskiy wrote:
> On 07.10.2016 22:28, Max Reitz wrote:
>> On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote:
>>> New field BdrvDirtyBitmap.persistent means, that bitmap should be saved
>>> on bdrv_close, using format driver. Format driver should maintain 
>>> bitmap
>>> storing.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   block.c                      | 30 ++++++++++++++++++++++++++++++
>>>   block/dirty-bitmap.c         | 27 +++++++++++++++++++++++++++
>>>   block/qcow2-bitmap.c         |  1 +
>>>   include/block/block.h        |  2 ++
>>>   include/block/block_int.h    |  2 ++
>>>   include/block/dirty-bitmap.h |  6 ++++++
>>>   6 files changed, 68 insertions(+)
>> [...]
>>
>>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>>> index 623e1d1..0314581 100644
>>> --- a/block/dirty-bitmap.c
>>> +++ b/block/dirty-bitmap.c
>> [...]
>>
>>> @@ -555,3 +559,26 @@ bool bdrv_dirty_bitmap_get_autoload(const 
>>> BdrvDirtyBitmap *bitmap)
>>>   {
>>>       return bitmap->autoload;
>>>   }
>>> +
>>> +void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
>>> +                                                bool persistent)
>>> +{
>>> +    bitmap->persistent = persistent;
>> After some thinking, I think this function should be more complex: It
>> should check whether the node the bitmap is attached to actually can
>> handle persistent bitmaps and whether it would actually support storing
>> *this* bitmap.
>>
>> For instance, a qcow2 node would not support writing overly large
>> bitmaps (limited by BME_MAX_TABLE_SIZE and BME_MAX_PHYS_SIZE) or bitmaps
>> with overly large granularities (BME_MAX_GRANULARITY_BITS) or bitmaps
>> whose name is already occupied by some bitmap that is already stored in
>> the file but has not been loaded.
>>
>> Checking this here will trivially prevent users from creating such
>> bitmaps and will also preempt detection of such failures during
>> bdrv_close() when they cannot be handled gracefully.
>>
>> Max
>
> Good point, but I can't do it exactly as you say, because I call this 
> function from qcow2_read_bitmaps, for just created bitmap and it 
> should not be checked and of course it's name is occupied..

So, I'll add an additional checking function, to call it from 
qmp_block_dirty_bitmap_add, if persistent parameter is set to true.

>
>>
>>> +}
>
>


-- 
Best regards,
Vladimir