Re: [Qemu-block] [PATCH v3 3/5] block/dirty-bitmaps: allow clear on disabled bitmaps

[Vladimir Sementsov-Ogievskiy]

26.09.2018 02:49, John Snow wrote:
> Similarly to merge, it's OK to allow clear operations on disabled
> bitmaps, as this condition only means that they are not recording
> new writes. We are free to clear it if the user requests it.
>
> Signed-off-by: John Snow <address@hidden>

Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>

> ---
>   block/dirty-bitmap.c | 1 -
>   blockdev.c           | 8 --------
>   2 files changed, 9 deletions(-)
>
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 53b7d282c4..ff4a86cea6 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -625,7 +625,6 @@ void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>   
>   void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)
>   {
> -    assert(bdrv_dirty_bitmap_enabled(bitmap));
>       assert(!bdrv_dirty_bitmap_readonly(bitmap));
>       bdrv_dirty_bitmap_lock(bitmap);
>       if (!out) {
> diff --git a/blockdev.c b/blockdev.c
> index dedcebb0fa..e178aae178 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2012,9 +2012,6 @@ static void 
> block_dirty_bitmap_clear_prepare(BlkActionState *common,
>       if (!bdrv_dirty_bitmap_user_modifiable(state->bitmap)) {
>           error_setg(errp, "Cannot modify a bitmap in-use by another 
> operation");
>           return;
> -    } else if (!bdrv_dirty_bitmap_enabled(state->bitmap)) {
> -        error_setg(errp, "Cannot clear a disabled bitmap");
> -        return;
>       } else if (bdrv_dirty_bitmap_readonly(state->bitmap)) {
>           error_setg(errp, "Cannot clear a readonly bitmap");
>           return;
> @@ -2917,11 +2914,6 @@ void qmp_block_dirty_bitmap_clear(const char *node, 
> const char *name,
>                      "Bitmap '%s' is currently in-use by another operation"
>                      " and cannot be cleared", name);
>           return;
> -    } else if (!bdrv_dirty_bitmap_enabled(bitmap)) {
> -        error_setg(errp,
> -                   "Bitmap '%s' is currently disabled and cannot be cleared",
> -                   name);
> -        return;
>       } else if (bdrv_dirty_bitmap_readonly(bitmap)) {
>           error_setg(errp, "Bitmap '%s' is readonly and cannot be cleared", 
> name);
>           return;


--
Best regards,
Vladimir