[Qemu-devel] [PATCH for-2.11 0/2] Fix 'qemu-img info' on mirror target

[Qemu-devel] [PATCH for-2.11 0/2] Fix 'qemu-img info' on mirror target

[Kevin Wolf]

There is no reason to forbid 'qemu-img info' for an image that is used
as a mirror target (or intermediate image of a commit job) at the same
time.

This fixes the problem reported on qemu-discuss:
https://lists.gnu.org/archive/html/qemu-discuss/2017-11/msg00039.html

Kevin Wolf (2):
  block: Don't use BLK_PERM_CONSISTENT_READ for format probing
  block: Don't request I/O permission with BDRV_O_NO_IO

 block.c               |  5 ++++-
 block/block-backend.c | 10 ++++++----
 2 files changed, 10 insertions(+), 5 deletions(-)

-- 
2.13.6

[Qemu-devel] [PATCH for-2.11 1/2] block: Don't use BLK_PERM_CONSISTENT_READ for format probing

[Kevin Wolf]

For format probing, we don't really care whether all of the image
content is consistent. The only thing we're looking at is the image
header, and specifically the magic numbers that are expected to never
change, no matter how inconsistent the guest visible disk content is.

Therefore, don't request BLK_PERM_CONSISTENT_READ. This allows to use
format probing, e.g. in the context of 'qemu-img info', even while the
guest visible data in the image is inconsistent during a running block
job.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 6c8ef98dfa..68b724206d 100644
--- a/block.c
+++ b/block.c
@@ -2579,7 +2579,10 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
             goto fail;
         }
         if (file_bs != NULL) {
-            file = blk_new(BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL);
+            /* Not requesting BLK_PERM_CONSISTENT_READ because we're only
+             * looking at the header to guess the image format. This works even
+             * in cases where a guest would not see a consistent state. */
+            file = blk_new(0, BLK_PERM_ALL);
             blk_insert_bs(file, file_bs, &local_err);
             bdrv_unref(file_bs);
             if (local_err) {
-- 
2.13.6

[Qemu-devel] [PATCH for-2.11 2/2] block: Don't request I/O permission with BDRV_O_NO_IO

[Kevin Wolf]

'qemu-img info' makes sense even when BLK_PERM_CONSISTENT_READ cannot be
granted because of a block job in a running qemu process. It already
sets BDRV_O_NO_IO to indicate that it doesn't access the guest visible
data at all.

Check the BDRV_O_NO_IO flags in blk_new_open(), so that I/O related
permissions are not unnecessarily requested and 'qemu-img info' can work
even if BLK_PERM_CONSISTENT_READ cannot be granted.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/block-backend.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 5836cb3087..baef8e7abc 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -299,7 +299,7 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
 {
     BlockBackend *blk;
     BlockDriverState *bs;
-    uint64_t perm;
+    uint64_t perm = 0;
 
     /* blk_new_open() is mainly used in .bdrv_create implementations and the
      * tools where sharing isn't a concern because the BDS stays private, so we
@@ -309,9 +309,11 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
      * caller of blk_new_open() doesn't make use of the permissions, but they
      * shouldn't hurt either. We can still share everything here because the
      * guest devices will add their own blockers if they can't share. */
-    perm = BLK_PERM_CONSISTENT_READ;
-    if (flags & BDRV_O_RDWR) {
-        perm |= BLK_PERM_WRITE;
+    if ((flags & BDRV_O_NO_IO) == 0) {
+        perm |= BLK_PERM_CONSISTENT_READ;
+        if (flags & BDRV_O_RDWR) {
+            perm |= BLK_PERM_WRITE;
+        }
     }
     if (flags & BDRV_O_RESIZE) {
         perm |= BLK_PERM_RESIZE;
-- 
2.13.6

Re: [Qemu-devel] [Qemu-block] [PATCH for-2.11 2/2] block: Don't request I/O permission with BDRV_O_NO_IO

[Alberto Garcia]

On Mon 20 Nov 2017 03:11:41 PM CET, Kevin Wolf wrote:
> 'qemu-img info' makes sense even when BLK_PERM_CONSISTENT_READ cannot be
> granted because of a block job in a running qemu process. It already
> sets BDRV_O_NO_IO to indicate that it doesn't access the guest visible
> data at all.
>
> Check the BDRV_O_NO_IO flags in blk_new_open(), so that I/O related
> permissions are not unnecessarily requested and 'qemu-img info' can work
> even if BLK_PERM_CONSISTENT_READ cannot be granted.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [PATCH for-2.11 0/2] Fix 'qemu-img info' on mirror target

[Fam Zheng]

On Mon, 11/20 15:11, Kevin Wolf wrote:
> There is no reason to forbid 'qemu-img info' for an image that is used
> as a mirror target (or intermediate image of a commit job) at the same
> time.
> 
> This fixes the problem reported on qemu-discuss:
> https://lists.gnu.org/archive/html/qemu-discuss/2017-11/msg00039.html
> 
> Kevin Wolf (2):
>   block: Don't use BLK_PERM_CONSISTENT_READ for format probing
>   block: Don't request I/O permission with BDRV_O_NO_IO
> 
>  block.c               |  5 ++++-
>  block/block-backend.c | 10 ++++++----
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> -- 
> 2.13.6
> 

Reviewed-by: Fam Zheng <famz@redhat.com>