We're not being consistent about this. If it's in use by an operation,
the user should not be able to change the behavior of that bitmap.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 blockdev.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index cccd36b5e7..d0febfca79 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2058,6 +2058,13 @@ static void block_dirty_bitmap_enable_prepare(BlkActionState *common,
         return;
     }
 
+    if (bdrv_dirty_bitmap_user_locked(state->bitmap)) {
+        error_setg(errp,
+                   "Bitmap '%s' is currently in use by another operation"
+                   " and cannot be enabled", action->name);
+        return;
+    }
+
     state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
     bdrv_enable_dirty_bitmap(state->bitmap);
 }
@@ -2092,6 +2099,13 @@ static void block_dirty_bitmap_disable_prepare(BlkActionState *common,
         return;
     }
 
+    if (bdrv_dirty_bitmap_user_locked(state->bitmap)) {
+        error_setg(errp,
+                   "Bitmap '%s' is currently in use by another operation"
+                   " and cannot be disabled", action->name);
+        return;
+    }
+
     state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
     bdrv_disable_dirty_bitmap(state->bitmap);
 }
@@ -2933,10 +2947,10 @@ void qmp_x_block_dirty_bitmap_enable(const char *node, const char *name,
         return;
     }
 
-    if (bdrv_dirty_bitmap_frozen(bitmap)) {
+    if (bdrv_dirty_bitmap_user_locked(bitmap)) {
         error_setg(errp,
-                   "Bitmap '%s' is currently frozen and cannot be enabled",
-                   name);
+                   "Bitmap '%s' is currently in use by another operation"
+                   " and cannot be enabled", name);
         return;
     }
 
@@ -2954,10 +2968,10 @@ void qmp_x_block_dirty_bitmap_disable(const char *node, const char *name,
         return;
     }
 
-    if (bdrv_dirty_bitmap_frozen(bitmap)) {
+    if (bdrv_dirty_bitmap_user_locked(bitmap)) {
         error_setg(errp,
-                   "Bitmap '%s' is currently frozen and cannot be disabled",
-                   name);
+                   "Bitmap '%s' is currently in use by another operation"
+                   " and cannot be disabled", name);
         return;
     }
 
-- 
2.14.4

On 10/2/18 6:02 PM, John Snow wrote:
> We're not being consistent about this. If it's in use by an operation,
> the user should not be able to change the behavior of that bitmap.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   blockdev.c | 26 ++++++++++++++++++++------
>   1 file changed, 20 insertions(+), 6 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

03.10.2018 02:02, John Snow wrote:
> We're not being consistent about this. If it's in use by an operation,
> the user should not be able to change the behavior of that bitmap.
>
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> ---
>   blockdev.c | 26 ++++++++++++++++++++------
>   1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index cccd36b5e7..d0febfca79 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2058,6 +2058,13 @@ static void block_dirty_bitmap_enable_prepare(BlkActionState *common,
>           return;
>       }
>   
> +    if (bdrv_dirty_bitmap_user_locked(state->bitmap)) {
> +        error_setg(errp,
> +                   "Bitmap '%s' is currently in use by another operation"
> +                   " and cannot be enabled", action->name);
> +        return;
> +    }
> +
>       state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
>       bdrv_enable_dirty_bitmap(state->bitmap);
>   }
> @@ -2092,6 +2099,13 @@ static void block_dirty_bitmap_disable_prepare(BlkActionState *common,
>           return;
>       }
>   
> +    if (bdrv_dirty_bitmap_user_locked(state->bitmap)) {
> +        error_setg(errp,
> +                   "Bitmap '%s' is currently in use by another operation"
> +                   " and cannot be disabled", action->name);
> +        return;
> +    }
> +
>       state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
>       bdrv_disable_dirty_bitmap(state->bitmap);
>   }
> @@ -2933,10 +2947,10 @@ void qmp_x_block_dirty_bitmap_enable(const char *node, const char *name,
>           return;
>       }
>   
> -    if (bdrv_dirty_bitmap_frozen(bitmap)) {
> +    if (bdrv_dirty_bitmap_user_locked(bitmap)) {
>           error_setg(errp,
> -                   "Bitmap '%s' is currently frozen and cannot be enabled",
> -                   name);
> +                   "Bitmap '%s' is currently in use by another operation"
> +                   " and cannot be enabled", name);
>           return;
>       }
>   
> @@ -2954,10 +2968,10 @@ void qmp_x_block_dirty_bitmap_disable(const char *node, const char *name,
>           return;
>       }
>   
> -    if (bdrv_dirty_bitmap_frozen(bitmap)) {
> +    if (bdrv_dirty_bitmap_user_locked(bitmap)) {
>           error_setg(errp,
> -                   "Bitmap '%s' is currently frozen and cannot be disabled",
> -                   name);
> +                   "Bitmap '%s' is currently in use by another operation"
> +                   " and cannot be disabled", name);
>           return;
>       }
>   


-- 
Best regards,
Vladimir