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

[Vladimir Sementsov-Ogievskiy]

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 <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

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..

> > +}


--
Best regards,
Vladimir