Re: [Qemu-devel] [PATCH 2/4] block/dirty-bitmap: add hide/unhide API

[John Snow]

On 6/3/19 8:00 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add functionality to make bitmap temporary anonymous. It will be used
> to implement bitmap remove transaction action. We need hide bitmap
> persistence too, as there are should not be unnamed persistent bitmaps.
> 

Ah, so this effectively ... "hides" a bitmap from any further
transaction actions. It also "hides" it from getting flushed to disk...
sort of?

The outer loop in store works with bdrv_dirty_bitmap_next, and we'll
skip this bitmap because it's anonymous/not persistent.

There's a second loop where we iterate bm_list, and we'll skip storing
this bitmap because that entry won't have an in-memory bitmap associated
with it in bm_list.

...But then we'll call update_ext_header_and_dir with the stale entries
in bm_list?

> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  include/block/dirty-bitmap.h |  2 ++
>  block/dirty-bitmap.c         | 26 ++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 8044ace63e..542e437123 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -116,5 +116,7 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap 
> *bitmap,
>  BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
>                                                    BdrvDirtyBitmap *bitmap,
>                                                    Error **errp);
> +void bdrv_dirty_bitmap_hide(BdrvDirtyBitmap *bitmap);
> +void bdrv_dirty_bitmap_unhide(BdrvDirtyBitmap *bitmap);
>  
>  #endif
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 49646a30e6..592964635e 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -35,6 +35,10 @@ struct BdrvDirtyBitmap {
>      bool busy;                  /* Bitmap is busy, it can't be used via QMP 
> */
>      BdrvDirtyBitmap *successor; /* Anonymous child, if any. */
>      char *name;                 /* Optional non-empty unique ID */
> +    char *hidden_name;          /* Backup of @name for removal transaction
> +                                   action. Used for hide/unhide API. */
> +    bool hidden_persistent;     /* Backup of @persistent for removal 
> transaction
> +                                   action. */
>      int64_t size;               /* Size of the bitmap, in bytes */
>      bool disabled;              /* Bitmap is disabled. It ignores all writes 
> to
>                                     the device */
> @@ -849,3 +853,25 @@ out:
>          qemu_mutex_unlock(src->mutex);
>      }
>  }
> +
> +void bdrv_dirty_bitmap_hide(BdrvDirtyBitmap *bitmap)
> +{
> +    qemu_mutex_lock(bitmap->mutex);
> +    assert(!bitmap->hidden_name);
> +    bitmap->hidden_name = bitmap->name;
> +    bitmap->hidden_persistent = bitmap->persistent;
> +    bitmap->name = NULL;
> +    bitmap->persistent = false;
> +    qemu_mutex_unlock(bitmap->mutex);
> +}
> +
> +void bdrv_dirty_bitmap_unhide(BdrvDirtyBitmap *bitmap)
> +{
> +    qemu_mutex_lock(bitmap->mutex);
> +    assert(!bitmap->name);
> +    bitmap->name = bitmap->hidden_name;
> +    bitmap->persistent = bitmap->hidden_persistent;
> +    bitmap->hidden_name = NULL;
> +    bitmap->hidden_persistent = false;
> +    qemu_mutex_unlock(bitmap->mutex);
> +}
>