On 12.10.2016 14:30, Vladimir Sementsov-Ogievskiy wrote:
> 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.

That would work just as well, yes. Thanks!

Max