[v4] dirty-bitmaps: fix QMP command permissions

[Qemu-devel] [PATCH v4 6/6] nbd: forbid use of frozen bitmaps

Posted by John Snow 7 years, 4 months ago

Whether it's "locked" or "frozen", it's in use and should
not be allowed for the purposes of this operation.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 nbd/server.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index c3dd402b45..940b7aa0cd 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2478,8 +2478,8 @@ void nbd_export_bitmap(NBDExport *exp, const char *bitmap,
         return;
     }
 
-    if (bdrv_dirty_bitmap_qmp_locked(bm)) {
-        error_setg(errp, "Bitmap '%s' is locked", bitmap);
+    if (bdrv_dirty_bitmap_user_locked(bm)) {
+        error_setg(errp, "Bitmap '%s' is in use", bitmap);
         return;
     }
 
-- 
2.14.4

Re: [Qemu-devel] [PATCH v4 6/6] nbd: forbid use of frozen bitmaps

Posted by Eric Blake 7 years, 4 months ago

On 10/2/18 6:02 PM, John Snow wrote:
> Whether it's "locked" or "frozen", it's in use and should
> not be allowed for the purposes of this operation.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   nbd/server.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/nbd/server.c b/nbd/server.c
> index c3dd402b45..940b7aa0cd 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -2478,8 +2478,8 @@ void nbd_export_bitmap(NBDExport *exp, const char *bitmap,
>           return;
>       }
>   
> -    if (bdrv_dirty_bitmap_qmp_locked(bm)) {
> -        error_setg(errp, "Bitmap '%s' is locked", bitmap);
> +    if (bdrv_dirty_bitmap_user_locked(bm)) {
> +        error_setg(errp, "Bitmap '%s' is in use", bitmap);

Again, I'm not sure about this. A locked bitmap is unchanging.  A frozen 
bitmap is split, and might change when the split is resolved, so we are 
correct that we cannot tolerate a frozen bitmap. But if locking were 
changed to a refcount instead of a bool, I don't see why we have to 
forbid a locked bitmap.  Then again, as in 5/6, the user can work around 
this limitation by cloning a locked bitmap into an unlocked temporary 
one, where we are trading ease of maintenance for a bit more user effort.

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

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

Re: [Qemu-devel] [PATCH v4 6/6] nbd: forbid use of frozen bitmaps

Posted by Vladimir Sementsov-Ogievskiy 7 years, 4 months ago

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

03.10.2018 02:02, John Snow wrote:
> Whether it's "locked" or "frozen", it's in use and should
> not be allowed for the purposes of this operation.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   nbd/server.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index c3dd402b45..940b7aa0cd 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -2478,8 +2478,8 @@ void nbd_export_bitmap(NBDExport *exp, const char *bitmap,
>           return;
>       }
>   
> -    if (bdrv_dirty_bitmap_qmp_locked(bm)) {
> -        error_setg(errp, "Bitmap '%s' is locked", bitmap);
> +    if (bdrv_dirty_bitmap_user_locked(bm)) {
> +        error_setg(errp, "Bitmap '%s' is in use", bitmap);
>           return;
>       }
>   


-- 
Best regards,
Vladimir