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

[Vladimir Sementsov-Ogievskiy]

03.10.2018 02:02, 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 8a6e07930f..035f97e5c2 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 d775f228fe..cccd36b5e7 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_locked(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