Skip to contentSkip to conversation

Fix 'qemu-img info' on mirror target

PATCH In review Kevin Wolf 3 participants 5 messages Nov 20, 2017 Block layer

lore
Share & tools
lore

Kevin Wolf

author
#0 lore raw

cover Fix 'qemu-img info' on mirror target

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(-)
Signature · 1 line
2.13.6
author
#1 lore raw

PATCH 1/2 block: Don't use BLK_PERM_CONSISTENT_READ for format probing

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 <[email protected]>
---
 block.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
block.c +4 -1
index 6c8ef98dfa..68b724206d 100644
@@ -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) {
Signature · 1 line
2.13.6
author
#2 lore raw

PATCH 2/2 block: Don't request I/O permission with BDRV_O_NO_IO

In reply to 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 <[email protected]>
---
 block/block-backend.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
block/block-backend.c +6 -4
index 5836cb3087..baef8e7abc 100644
@@ -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;
Signature · 1 line
2.13.6
Reviewed-by on v1 2/2
#3 lore raw

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

Quoted 10 lines · 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 <[email protected]>
Reviewed-by Alberto Garcia <[email protected]>

Berto

Reviewed-by on the v1 series
#4 lore raw
In reply to Kevin Wolf
Quoted 7 lines · 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

Quoted 11 lines

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(-)
Signature · 1 line
2.13.6
Reviewed-by Fam Zheng <[email protected]>