diff mbox

[9/9] block: Remove BlockDriverState.blk

Message ID 1458675397-24956-10-git-send-email-kwolf@redhat.com
State New
Headers show

Commit Message

Kevin Wolf March 22, 2016, 7:36 p.m. UTC
This patch removes the remaining users of bs->blk, which will allow us
to have multiple BBs on top of a single BDS. All checks that are
currently in place to prevent the user from creating such setups.

Future patches can allow them and e.g. enable users to mirror to a block
device that already has a BlockBackend on it.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                    |  8 --------
 block/block-backend.c      |  8 --------
 block/mirror.c             |  4 ++--
 blockdev.c                 | 19 ++++++++-----------
 include/block/block_int.h  |  2 --
 tests/qemu-iotests/085.out |  6 +++---
 6 files changed, 13 insertions(+), 34 deletions(-)

Comments

Max Reitz March 29, 2016, 6:59 p.m. UTC | #1
On 22.03.2016 20:36, Kevin Wolf wrote:
> This patch removes the remaining users of bs->blk, which will allow us
> to have multiple BBs on top of a single BDS. All checks that are
> currently in place to prevent the user from creating such setups.

I think this sentence is missing a word or two.

> Future patches can allow them and e.g. enable users to mirror to a block
> device that already has a BlockBackend on it.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c                    |  8 --------
>  block/block-backend.c      |  8 --------
>  block/mirror.c             |  4 ++--
>  blockdev.c                 | 19 ++++++++-----------
>  include/block/block_int.h  |  2 --
>  tests/qemu-iotests/085.out |  6 +++---
>  6 files changed, 13 insertions(+), 34 deletions(-)

[...]

> diff --git a/blockdev.c b/blockdev.c
> index c59cf3e..a658869 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1774,9 +1774,8 @@ static void external_snapshot_prepare(BlkActionState *common,
>          return;
>      }
>  
> -    if (state->new_bs->blk != NULL) {
> -        error_setg(errp, "The snapshot is already in use by %s",
> -                   blk_name(state->new_bs->blk));
> +    if (bdrv_has_blk(state->new_bs)) {
> +        error_setg(errp, "The snapshot is already in use");

You did add bdrv_get_parent_name(), so you could use it here...

>          return;
>      }
>  
> @@ -2492,9 +2491,8 @@ void qmp_x_blockdev_insert_medium(const char *device, const char *node_name,
>          return;
>      }
>  
> -    if (bs->blk) {
> -        error_setg(errp, "Node '%s' is already in use by '%s'", node_name,
> -                   blk_name(bs->blk));
> +    if (bdrv_has_blk(bs)) {
> +        error_setg(errp, "Node '%s' is already in use", node_name);

...and here...

>          return;
>      }
>  
> @@ -3439,7 +3437,7 @@ static void blockdev_mirror_common(BlockDriverState *bs,
>      if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_MIRROR_TARGET, errp)) {
>          return;
>      }
> -    if (target->blk) {
> +    if (bdrv_has_blk(target)) {
>          error_setg(errp, "Cannot mirror to an attached block device");
>          return;
>      }
> @@ -4023,15 +4021,14 @@ void qmp_x_blockdev_del(bool has_id, const char *id,
>          bs = blk_bs(blk);
>          aio_context = blk_get_aio_context(blk);
>      } else {
> +        blk = NULL;
>          bs = bdrv_find_node(node_name);
>          if (!bs) {
>              error_setg(errp, "Cannot find node %s", node_name);
>              return;
>          }
> -        blk = bs->blk;
> -        if (blk) {
> -            error_setg(errp, "Node %s is in use by %s",
> -                       node_name, blk_name(blk));
> +        if (bdrv_has_blk(bs)) {
> +            error_setg(errp, "Node %s is in use", node_name);

...and here.

If you consciously decide not to use it there, and after you have fixed
the commit message:

Reviewed-by: Max Reitz <mreitz@redhat.com>

>              return;
>          }
>          aio_context = bdrv_get_aio_context(bs);

[...]
diff mbox

Patch

diff --git a/block.c b/block.c
index eefbcf3..05f9ad4 100644
--- a/block.c
+++ b/block.c
@@ -2227,14 +2227,6 @@  static void change_parent_backing_link(BlockDriverState *from,
 {
     BdrvChild *c, *next;
 
-    if (from->blk) {
-        /* FIXME We bypass blk_set_bs(), so we need to make these updates
-         * manually. The root problem is not in this change function, but the
-         * existence of BlockDriverState.blk. */
-        to->blk = from->blk;
-        from->blk = NULL;
-    }
-
     QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) {
         assert(c->role != &child_backing);
         c->bs = to;
diff --git a/block/block-backend.c b/block/block-backend.c
index b71b822..42f95a6 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -190,7 +190,6 @@  BlockBackend *blk_new_with_bs(Error **errp)
     bs = bdrv_new_root();
     blk->root = bdrv_root_attach_child(bs, "root", &child_root);
     blk->root->opaque = blk;
-    bs->blk = blk;
     return blk;
 }
 
@@ -453,12 +452,10 @@  bool bdrv_has_blk(BlockDriverState *bs)
     BdrvChild *child;
     QLIST_FOREACH(child, &bs->parents, next_parent) {
         if (child->role == &child_root) {
-            assert(bs->blk);
             return true;
         }
     }
 
-    assert(!bs->blk);
     return false;
 }
 
@@ -518,8 +515,6 @@  BlockBackend *blk_by_public(BlockBackendPublic *public)
  */
 void blk_remove_bs(BlockBackend *blk)
 {
-    assert(blk->root->bs->blk == blk);
-
     notifier_list_notify(&blk->remove_bs_notifiers, blk);
     if (blk->public.throttle_state) {
         throttle_timers_detach_aio_context(&blk->public.throttle_timers);
@@ -527,7 +522,6 @@  void blk_remove_bs(BlockBackend *blk)
 
     blk_update_root_state(blk);
 
-    blk->root->bs->blk = NULL;
     bdrv_root_unref_child(blk->root);
     blk->root = NULL;
 }
@@ -537,11 +531,9 @@  void blk_remove_bs(BlockBackend *blk)
  */
 void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
 {
-    assert(!blk->root && !bs->blk);
     bdrv_ref(bs);
     blk->root = bdrv_root_attach_child(bs, "root", &child_root);
     blk->root->opaque = blk;
-    bs->blk = blk;
 
     notifier_list_notify(&blk->insert_bs_notifiers, blk);
     if (blk->public.throttle_state) {
diff --git a/block/mirror.c b/block/mirror.c
index b9a93fd..8b32271 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -450,7 +450,7 @@  static void mirror_exit(BlockJob *job, void *opaque)
 
         /* This was checked in mirror_start_job(), but meanwhile one of the
          * nodes could have been newly attached to a BlockBackend. */
-        if (to_replace->blk && s->target->blk) {
+        if (bdrv_has_blk(to_replace) && bdrv_has_blk(s->target)) {
             error_report("block job: Can't create node with two BlockBackends");
             data->ret = -EINVAL;
             goto out;
@@ -825,7 +825,7 @@  static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
     } else {
         replaced_bs = bs;
     }
-    if (replaced_bs->blk && target->blk) {
+    if (bdrv_has_blk(replaced_bs) && bdrv_has_blk(target)) {
         error_setg(errp, "Can't create node with two BlockBackends");
         return;
     }
diff --git a/blockdev.c b/blockdev.c
index c59cf3e..a658869 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1774,9 +1774,8 @@  static void external_snapshot_prepare(BlkActionState *common,
         return;
     }
 
-    if (state->new_bs->blk != NULL) {
-        error_setg(errp, "The snapshot is already in use by %s",
-                   blk_name(state->new_bs->blk));
+    if (bdrv_has_blk(state->new_bs)) {
+        error_setg(errp, "The snapshot is already in use");
         return;
     }
 
@@ -2492,9 +2491,8 @@  void qmp_x_blockdev_insert_medium(const char *device, const char *node_name,
         return;
     }
 
-    if (bs->blk) {
-        error_setg(errp, "Node '%s' is already in use by '%s'", node_name,
-                   blk_name(bs->blk));
+    if (bdrv_has_blk(bs)) {
+        error_setg(errp, "Node '%s' is already in use", node_name);
         return;
     }
 
@@ -3439,7 +3437,7 @@  static void blockdev_mirror_common(BlockDriverState *bs,
     if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_MIRROR_TARGET, errp)) {
         return;
     }
-    if (target->blk) {
+    if (bdrv_has_blk(target)) {
         error_setg(errp, "Cannot mirror to an attached block device");
         return;
     }
@@ -4023,15 +4021,14 @@  void qmp_x_blockdev_del(bool has_id, const char *id,
         bs = blk_bs(blk);
         aio_context = blk_get_aio_context(blk);
     } else {
+        blk = NULL;
         bs = bdrv_find_node(node_name);
         if (!bs) {
             error_setg(errp, "Cannot find node %s", node_name);
             return;
         }
-        blk = bs->blk;
-        if (blk) {
-            error_setg(errp, "Node %s is in use by %s",
-                       node_name, blk_name(blk));
+        if (bdrv_has_blk(bs)) {
+            error_setg(errp, "Node %s is in use", node_name);
             return;
         }
         aio_context = bdrv_get_aio_context(bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 9358949..2bbca78 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -406,8 +406,6 @@  struct BlockDriverState {
     BlockDriver *drv; /* NULL means no media */
     void *opaque;
 
-    BlockBackend *blk;          /* owning backend, if any */
-
     AioContext *aio_context; /* event loop used for fd handlers, timers, etc */
     /* long-running tasks intended to always use the same AioContext as this
      * BDS may register themselves in this list to be notified of changes
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index 01c78d6..08e4bb7 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -68,9 +68,9 @@  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/
 
 === Invalid command - snapshot node used as active layer ===
 
-{"error": {"class": "GenericError", "desc": "The snapshot is already in use by virtio0"}}
-{"error": {"class": "GenericError", "desc": "The snapshot is already in use by virtio0"}}
-{"error": {"class": "GenericError", "desc": "The snapshot is already in use by virtio1"}}
+{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}
+{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}
+{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}
 
 === Invalid command - snapshot node used as backing hd ===