[Qemu-devel] [PATCH 0/6] block: More op blocker fixes

[Qemu-devel] [PATCH 0/6] block: More op blocker fixes

[Kevin Wolf]

Kevin Wolf (6):
  block: Remove check_new_perm from bdrv_replace_child()
  block: Request block status from *file for BDRV_BLOCK_RAW
  commit: Implement bdrv_commit_top.bdrv_co_get_block_status
  block: Refresh filename after changing backing file
  mirror: Implement .bdrv_refresh_filename
  commit: Implement .bdrv_refresh_filename

 block.c        | 23 ++++++++++++++++-------
 block/commit.c | 28 ++++++++++++++++++++++++----
 block/io.c     |  2 +-
 block/mirror.c |  9 +++++++++
 4 files changed, 50 insertions(+), 12 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/6] block: Remove check_new_perm from bdrv_replace_child()

[Kevin Wolf]

All callers pass false now, so the parameter can go away again.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index dd9ded8..756d607 100644
--- a/block.c
+++ b/block.c
@@ -1751,8 +1751,18 @@ static void bdrv_replace_child_noperm(BdrvChild *child,
     }
 }
 
-static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs,
-                               bool check_new_perm)
+/*
+ * Updates @child to change its reference to point to @new_bs, including
+ * checking and applying the necessary permisson updates both to the old node
+ * and to @new_bs.
+ *
+ * NULL is passed as @new_bs for removing the reference before freeing @child.
+ *
+ * If @new_bs is not NULL, bdrv_check_perm() must be called beforehand, as this
+ * function uses bdrv_set_perm() to update the permissions according to the new
+ * reference that @new_bs gets.
+ */
+static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs)
 {
     BlockDriverState *old_bs = child->bs;
     uint64_t perm, shared_perm;
@@ -1770,9 +1780,6 @@ static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs,
 
     if (new_bs) {
         bdrv_get_cumulative_perm(new_bs, &perm, &shared_perm);
-        if (check_new_perm) {
-            bdrv_check_perm(new_bs, perm, shared_perm, NULL, &error_abort);
-        }
         bdrv_set_perm(new_bs, perm, shared_perm);
     }
 }
@@ -1803,7 +1810,7 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
     };
 
     /* This performs the matching bdrv_set_perm() for the above check. */
-    bdrv_replace_child(child, child_bs, false);
+    bdrv_replace_child(child, child_bs);
 
     return child;
 }
@@ -1840,7 +1847,7 @@ static void bdrv_detach_child(BdrvChild *child)
         child->next.le_prev = NULL;
     }
 
-    bdrv_replace_child(child, NULL, false);
+    bdrv_replace_child(child, NULL);
 
     g_free(child->name);
     g_free(child);
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/6] block: Request block status from *file for BDRV_BLOCK_RAW

[Kevin Wolf]

This fixes bdrv_co_get_block_status() for the bdrv_mirror_top block
driver, which must fall through to bs->backing instead of bs->file.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/io.c b/block/io.c
index 8f38d46..2709a70 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1760,7 +1760,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
 
     if (ret & BDRV_BLOCK_RAW) {
         assert(ret & BDRV_BLOCK_OFFSET_VALID);
-        ret = bdrv_get_block_status(bs->file->bs, ret >> BDRV_SECTOR_BITS,
+        ret = bdrv_get_block_status(*file, ret >> BDRV_SECTOR_BITS,
                                     *pnum, pnum, file);
         goto out;
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/6] commit: Implement bdrv_commit_top.bdrv_co_get_block_status

[Kevin Wolf]

In some cases, bdrv_co_get_block_status() is called recursively for the
whole backing chain. The automatically inserted bdrv_commit_top filter
driver must not stop the recursion, so implement a callback that simply
forwards the request to bs->backing.

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

diff --git a/block/commit.c b/block/commit.c
index 9c41988..932d1e6 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -232,6 +232,17 @@ static int coroutine_fn bdrv_commit_top_preadv(BlockDriverState *bs,
     return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
 }
 
+static int64_t coroutine_fn bdrv_commit_top_get_block_status(
+    BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
+    BlockDriverState **file)
+{
+    *pnum = nb_sectors;
+    *file = bs->backing->bs;
+    return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
+           (sector_num << BDRV_SECTOR_BITS);
+}
+
+
 static void bdrv_commit_top_close(BlockDriverState *bs)
 {
 }
@@ -248,10 +259,11 @@ static void bdrv_commit_top_child_perm(BlockDriverState *bs, BdrvChild *c,
 /* Dummy node that provides consistent read to its users without requiring it
  * from its backing file and that allows writes on the backing file chain. */
 static BlockDriver bdrv_commit_top = {
-    .format_name        = "commit_top",
-    .bdrv_co_preadv     = bdrv_commit_top_preadv,
-    .bdrv_close         = bdrv_commit_top_close,
-    .bdrv_child_perm    = bdrv_commit_top_child_perm,
+    .format_name                = "commit_top",
+    .bdrv_co_preadv             = bdrv_commit_top_preadv,
+    .bdrv_co_get_block_status   = bdrv_commit_top_get_block_status,
+    .bdrv_close                 = bdrv_commit_top_close,
+    .bdrv_child_perm            = bdrv_commit_top_child_perm,
 };
 
 void commit_start(const char *job_id, BlockDriverState *bs,
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/6] block: Refresh filename after changing backing file

[Kevin Wolf]

In bdrv_open_inherit(), the filename is refreshed after opening the
backing file, but we neglected to do the same when the backing file
changes later.

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

diff --git a/block.c b/block.c
index 756d607..516cefe 100644
--- a/block.c
+++ b/block.c
@@ -1933,6 +1933,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
         bdrv_unref(backing_hd);
     }
 
+    bdrv_refresh_filename(bs);
+
 out:
     bdrv_refresh_limits(bs, NULL);
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/6] mirror: Implement .bdrv_refresh_filename

[Kevin Wolf]

We want query-block to return the right filename, even if a mirror job
put a bdrv_mirror_top on top of the actual image format driver. Let
bdrv_mirror_top.bdrv_refresh_filename get the filename from its backing
file.

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

diff --git a/block/mirror.c b/block/mirror.c
index a5d30ee..4f3a5cb 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "trace.h"
 #include "block/blockjob_int.h"
 #include "block/block_int.h"
@@ -1060,6 +1061,13 @@ static int coroutine_fn bdrv_mirror_top_pdiscard(BlockDriverState *bs,
     return bdrv_co_pdiscard(bs->backing->bs, offset, count);
 }
 
+static void bdrv_mirror_top_refresh_filename(BlockDriverState *bs, QDict *opts)
+{
+    bdrv_refresh_filename(bs->backing->bs);
+    pstrcpy(bs->exact_filename, sizeof(bs->exact_filename),
+            bs->backing->bs->filename);
+}
+
 static void bdrv_mirror_top_close(BlockDriverState *bs)
 {
 }
@@ -1088,6 +1096,7 @@ static BlockDriver bdrv_mirror_top = {
     .bdrv_co_pdiscard           = bdrv_mirror_top_pdiscard,
     .bdrv_co_flush              = bdrv_mirror_top_flush,
     .bdrv_co_get_block_status   = bdrv_mirror_top_get_block_status,
+    .bdrv_refresh_filename      = bdrv_mirror_top_refresh_filename,
     .bdrv_close                 = bdrv_mirror_top_close,
     .bdrv_child_perm            = bdrv_mirror_top_child_perm,
 };
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/6] commit: Implement .bdrv_refresh_filename

[Kevin Wolf]

We want query-block to return the right filename, even if a commit job
put a bdrv_commit_top on top of the actual image format driver. Let
bdrv_commit_top.bdrv_refresh_filename get the filename from its backing
file.

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

diff --git a/block/commit.c b/block/commit.c
index 932d1e6..2832482 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -13,6 +13,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "trace.h"
 #include "block/block_int.h"
 #include "block/blockjob_int.h"
@@ -242,6 +243,12 @@ static int64_t coroutine_fn bdrv_commit_top_get_block_status(
            (sector_num << BDRV_SECTOR_BITS);
 }
 
+static void bdrv_commit_top_refresh_filename(BlockDriverState *bs, QDict *opts)
+{
+    bdrv_refresh_filename(bs->backing->bs);
+    pstrcpy(bs->exact_filename, sizeof(bs->exact_filename),
+            bs->backing->bs->filename);
+}
 
 static void bdrv_commit_top_close(BlockDriverState *bs)
 {
@@ -262,6 +269,7 @@ static BlockDriver bdrv_commit_top = {
     .format_name                = "commit_top",
     .bdrv_co_preadv             = bdrv_commit_top_preadv,
     .bdrv_co_get_block_status   = bdrv_commit_top_get_block_status,
+    .bdrv_refresh_filename      = bdrv_commit_top_refresh_filename,
     .bdrv_close                 = bdrv_commit_top_close,
     .bdrv_child_perm            = bdrv_commit_top_child_perm,
 };
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/6] block: More op blocker fixes

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 783 bytes --]

On 03/09/2017 05:38 AM, Kevin Wolf wrote:
> Kevin Wolf (6):
>   block: Remove check_new_perm from bdrv_replace_child()
>   block: Request block status from *file for BDRV_BLOCK_RAW
>   commit: Implement bdrv_commit_top.bdrv_co_get_block_status
>   block: Refresh filename after changing backing file
>   mirror: Implement .bdrv_refresh_filename
>   commit: Implement .bdrv_refresh_filename
> 

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

>  block.c        | 23 ++++++++++++++++-------
>  block/commit.c | 28 ++++++++++++++++++++++++----
>  block/io.c     |  2 +-
>  block/mirror.c |  9 +++++++++
>  4 files changed, 50 insertions(+), 12 deletions(-)
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 0/6] block: More op blocker fixes

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

Am 09.03.2017 um 15:23 hat Eric Blake geschrieben:
> On 03/09/2017 05:38 AM, Kevin Wolf wrote:
> > Kevin Wolf (6):
> >   block: Remove check_new_perm from bdrv_replace_child()
> >   block: Request block status from *file for BDRV_BLOCK_RAW
> >   commit: Implement bdrv_commit_top.bdrv_co_get_block_status
> >   block: Refresh filename after changing backing file
> >   mirror: Implement .bdrv_refresh_filename
> >   commit: Implement .bdrv_refresh_filename
> > 
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

Applied to the block branch.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]