diff mbox

[1/9] block: Use BdrvChild callbacks for change_media/resize

Message ID 1458675397-24956-2-git-send-email-kwolf@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kevin Wolf March 22, 2016, 7:36 p.m. UTC 
We want to get rid of BlockDriverState.blk in order to allow multiple
BlockBackends per BDS. Converting the device callbacks in block.c (which
assume a single BlockBackend) to per-child callbacks gets us rid of the
first few instances.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                   | 38 +++++++++++++++++++++++++-------------
 block/block-backend.c     | 15 ++++++++++++++-
 include/block/block_int.h |  4 +++-
 3 files changed, 42 insertions(+), 15 deletions(-)

Comments

Max Reitz March 29, 2016, 5:36 p.m. UTC | #1 
On 22.03.2016 20:36, Kevin Wolf wrote:
> We want to get rid of BlockDriverState.blk in order to allow multiple
> BlockBackends per BDS. Converting the device callbacks in block.c (which
> assume a single BlockBackend) to per-child callbacks gets us rid of the
> first few instances.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c                   | 38 +++++++++++++++++++++++++-------------
>  block/block-backend.c     | 15 ++++++++++++++-
>  include/block/block_int.h |  4 +++-
>  3 files changed, 42 insertions(+), 15 deletions(-)
> 
> diff --git a/block.c b/block.c
> index fb37b91..1fb5dac 100644
> --- a/block.c
> +++ b/block.c
> @@ -1216,6 +1216,27 @@ void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child)
>      bdrv_root_unref_child(child);
>  }
>  
> +
> +static void bdrv_parent_cb_change_media(BlockDriverState *bs, bool load)
> +{
> +    BdrvChild *c;
> +    QLIST_FOREACH(c, &bs->parents, next_parent) {
> +        if (c->role->change_media) {
> +            c->role->change_media(c, load);
> +        }
> +    }
> +}
> +
> +static void bdrv_parent_cb_resize(BlockDriverState *bs)
> +{
> +    BdrvChild *c;
> +    QLIST_FOREACH(c, &bs->parents, next_parent) {
> +        if (c->role->resize) {
> +            c->role->resize(c);
> +        }
> +    }
> +}
> +
>  /*
>   * Sets the backing file link of a BDS. A new reference is created; callers
>   * which don't need their own reference any more must call bdrv_unref().
> @@ -1674,9 +1695,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
>      }
>  
>      if (!bdrv_key_required(bs)) {
> -        if (bs->blk) {
> -            blk_dev_change_media_cb(bs->blk, true);
> -        }
> +        bdrv_parent_cb_change_media(bs, true);
>      } else if (!runstate_check(RUN_STATE_PRELAUNCH)
>                 && !runstate_check(RUN_STATE_INMIGRATE)
>                 && !runstate_check(RUN_STATE_PAUSED)) { /* HACK */
> @@ -2122,9 +2141,7 @@ static void bdrv_close(BlockDriverState *bs)
>      bdrv_release_named_dirty_bitmaps(bs);
>      assert(QLIST_EMPTY(&bs->dirty_bitmaps));
>  
> -    if (bs->blk) {
> -        blk_dev_change_media_cb(bs->blk, false);
> -    }
> +    bdrv_parent_cb_change_media(bs, false);

This is probably unnecessary altogether, but I'm certain about it after
http://lists.nongnu.org/archive/html/qemu-block/2015-11/msg00349.html.
So I'll take a note of removing this then.

>      if (bs->drv) {
>          BdrvChild *child, *next;
> @@ -2605,9 +2622,7 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
>      if (ret == 0) {
>          ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
>          bdrv_dirty_bitmap_truncate(bs);
> -        if (bs->blk) {
> -            blk_dev_resize_cb(bs->blk);
> -        }
> +        bdrv_parent_cb_resize(bs);
>      }
>      return ret;
>  }
> @@ -2718,10 +2733,7 @@ int bdrv_set_key(BlockDriverState *bs, const char *key)
>          bs->valid_key = 0;
>      } else if (!bs->valid_key) {
>          bs->valid_key = 1;
> -        if (bs->blk) {
> -            /* call the change callback now, we skipped it on open */
> -            blk_dev_change_media_cb(bs->blk, true);
> -        }
> +        bdrv_parent_cb_change_media(bs, true);

Not sure why you removed the comment here. In case you send a v2, I'd
keep it.

Whichever way you decide:

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

>      }
>      return ret;
>  }
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 8b4eb1a..9d973ba 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -92,9 +92,15 @@ static void blk_root_inherit_options(int *child_flags, QDict *child_options,
>  }
>  static bool blk_drain_throttling_queue(BdrvChild *child);
>  
> +static void blk_root_change_media(BdrvChild *child, bool load);
> +static void blk_root_resize(BdrvChild *child);
> +
>  static const BdrvChildRole child_root = {
>      .inherit_options    = blk_root_inherit_options,
>  
> +    .change_media       = blk_root_change_media,
> +    .resize             = blk_root_resize,
> +
>      .drain_queue        = blk_drain_throttling_queue,
>  };
>  
> @@ -553,6 +559,11 @@ void blk_dev_change_media_cb(BlockBackend *blk, bool load)
>      }
>  }
>  
> +static void blk_root_change_media(BdrvChild *child, bool load)
> +{
> +    blk_dev_change_media_cb(child->opaque, load);
> +}
> +
>  /*
>   * Does @blk's attached device model have removable media?
>   * %true if no device model is attached.
> @@ -607,8 +618,10 @@ bool blk_dev_is_medium_locked(BlockBackend *blk)
>  /*
>   * Notify @blk's attached device model of a backend size change.
>   */
> -void blk_dev_resize_cb(BlockBackend *blk)
> +static void blk_root_resize(BdrvChild *child)
>  {
> +    BlockBackend *blk = child->opaque;
> +
>      if (blk->dev_ops && blk->dev_ops->resize_cb) {
>          blk->dev_ops->resize_cb(blk->dev_opaque);
>      }
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 000d2c0..f60cb7c 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -356,6 +356,9 @@ struct BdrvChildRole {
>      void (*inherit_options)(int *child_flags, QDict *child_options,
>                              int parent_flags, QDict *parent_options);
>  
> +    void (*change_media)(BdrvChild *child, bool load);
> +    void (*resize)(BdrvChild *child);
> +
>      bool (*drain_queue)(BdrvChild *child);
>  };
>  
> @@ -695,7 +698,6 @@ bool blk_dev_has_tray(BlockBackend *blk);
>  void blk_dev_eject_request(BlockBackend *blk, bool force);
>  bool blk_dev_is_tray_open(BlockBackend *blk);
>  bool blk_dev_is_medium_locked(BlockBackend *blk);
> -void blk_dev_resize_cb(BlockBackend *blk);
>  
>  void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
>  bool bdrv_requests_pending(BlockDriverState *bs);
>
diff mbox

Patch

diff --git a/block.c b/block.c
index fb37b91..1fb5dac 100644
--- a/block.c
+++ b/block.c
@@ -1216,6 +1216,27 @@  void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child)
     bdrv_root_unref_child(child);
 }
 
+
+static void bdrv_parent_cb_change_media(BlockDriverState *bs, bool load)
+{
+    BdrvChild *c;
+    QLIST_FOREACH(c, &bs->parents, next_parent) {
+        if (c->role->change_media) {
+            c->role->change_media(c, load);
+        }
+    }
+}
+
+static void bdrv_parent_cb_resize(BlockDriverState *bs)
+{
+    BdrvChild *c;
+    QLIST_FOREACH(c, &bs->parents, next_parent) {
+        if (c->role->resize) {
+            c->role->resize(c);
+        }
+    }
+}
+
 /*
  * Sets the backing file link of a BDS. A new reference is created; callers
  * which don't need their own reference any more must call bdrv_unref().
@@ -1674,9 +1695,7 @@  static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
     }
 
     if (!bdrv_key_required(bs)) {
-        if (bs->blk) {
-            blk_dev_change_media_cb(bs->blk, true);
-        }
+        bdrv_parent_cb_change_media(bs, true);
     } else if (!runstate_check(RUN_STATE_PRELAUNCH)
                && !runstate_check(RUN_STATE_INMIGRATE)
                && !runstate_check(RUN_STATE_PAUSED)) { /* HACK */
@@ -2122,9 +2141,7 @@  static void bdrv_close(BlockDriverState *bs)
     bdrv_release_named_dirty_bitmaps(bs);
     assert(QLIST_EMPTY(&bs->dirty_bitmaps));
 
-    if (bs->blk) {
-        blk_dev_change_media_cb(bs->blk, false);
-    }
+    bdrv_parent_cb_change_media(bs, false);
 
     if (bs->drv) {
         BdrvChild *child, *next;
@@ -2605,9 +2622,7 @@  int bdrv_truncate(BlockDriverState *bs, int64_t offset)
     if (ret == 0) {
         ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
         bdrv_dirty_bitmap_truncate(bs);
-        if (bs->blk) {
-            blk_dev_resize_cb(bs->blk);
-        }
+        bdrv_parent_cb_resize(bs);
     }
     return ret;
 }
@@ -2718,10 +2733,7 @@  int bdrv_set_key(BlockDriverState *bs, const char *key)
         bs->valid_key = 0;
     } else if (!bs->valid_key) {
         bs->valid_key = 1;
-        if (bs->blk) {
-            /* call the change callback now, we skipped it on open */
-            blk_dev_change_media_cb(bs->blk, true);
-        }
+        bdrv_parent_cb_change_media(bs, true);
     }
     return ret;
 }
diff --git a/block/block-backend.c b/block/block-backend.c
index 8b4eb1a..9d973ba 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -92,9 +92,15 @@  static void blk_root_inherit_options(int *child_flags, QDict *child_options,
 }
 static bool blk_drain_throttling_queue(BdrvChild *child);
 
+static void blk_root_change_media(BdrvChild *child, bool load);
+static void blk_root_resize(BdrvChild *child);
+
 static const BdrvChildRole child_root = {
     .inherit_options    = blk_root_inherit_options,
 
+    .change_media       = blk_root_change_media,
+    .resize             = blk_root_resize,
+
     .drain_queue        = blk_drain_throttling_queue,
 };
 
@@ -553,6 +559,11 @@  void blk_dev_change_media_cb(BlockBackend *blk, bool load)
     }
 }
 
+static void blk_root_change_media(BdrvChild *child, bool load)
+{
+    blk_dev_change_media_cb(child->opaque, load);
+}
+
 /*
  * Does @blk's attached device model have removable media?
  * %true if no device model is attached.
@@ -607,8 +618,10 @@  bool blk_dev_is_medium_locked(BlockBackend *blk)
 /*
  * Notify @blk's attached device model of a backend size change.
  */
-void blk_dev_resize_cb(BlockBackend *blk)
+static void blk_root_resize(BdrvChild *child)
 {
+    BlockBackend *blk = child->opaque;
+
     if (blk->dev_ops && blk->dev_ops->resize_cb) {
         blk->dev_ops->resize_cb(blk->dev_opaque);
     }
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 000d2c0..f60cb7c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -356,6 +356,9 @@  struct BdrvChildRole {
     void (*inherit_options)(int *child_flags, QDict *child_options,
                             int parent_flags, QDict *parent_options);
 
+    void (*change_media)(BdrvChild *child, bool load);
+    void (*resize)(BdrvChild *child);
+
     bool (*drain_queue)(BdrvChild *child);
 };
 
@@ -695,7 +698,6 @@  bool blk_dev_has_tray(BlockBackend *blk);
 void blk_dev_eject_request(BlockBackend *blk, bool force);
 bool blk_dev_is_tray_open(BlockBackend *blk);
 bool blk_dev_is_medium_locked(BlockBackend *blk);
-void blk_dev_resize_cb(BlockBackend *blk);
 
 void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
 bool bdrv_requests_pending(BlockDriverState *bs);