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

[Max Reitz]

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 <address@hidden>
> ---
>  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

> +}

signature.asc
Description: OpenPGP digital signature