diff mbox series

[RFC,v4,7/9] config: add check to block layer

Message ID 20220712021345.8530-8-faithilikerun@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add support for zoned device | expand

Commit Message

Sam Li July 12, 2022, 2:13 a.m. UTC
Putting zoned/non-zoned BlockDrivers on top of each other is not
allowed.

Signed-off-by: Sam Li <faithilikerun@gmail.com>
---
 block.c                          | 7 +++++++
 block/file-posix.c               | 2 ++
 include/block/block_int-common.h | 5 +++++
 3 files changed, 14 insertions(+)

Comments

Stefan Hajnoczi July 27, 2022, 2:50 p.m. UTC | #1
On Mon, 11 Jul 2022 at 22:22, Sam Li <faithilikerun@gmail.com> wrote:
>
> Putting zoned/non-zoned BlockDrivers on top of each other is not
> allowed.
>
> Signed-off-by: Sam Li <faithilikerun@gmail.com>
> ---
>  block.c                          | 7 +++++++
>  block/file-posix.c               | 2 ++
>  include/block/block_int-common.h | 5 +++++
>  3 files changed, 14 insertions(+)
>
> diff --git a/block.c b/block.c
> index 2c00dddd80..0e24582c7d 100644
> --- a/block.c
> +++ b/block.c
> @@ -7945,6 +7945,13 @@ void bdrv_add_child(BlockDriverState *parent_bs, BlockDriverState *child_bs,
>          return;
>      }
>
> +    if (parent_bs->drv->is_zoned != child_bs->drv->is_zoned) {
> +        error_setg(errp, "Cannot add a %s child to a %s parent",
> +                   child_bs->drv->is_zoned ? "zoned" : "non-zoned",
> +                   parent_bs->drv->is_zoned ? "zoned" : "non-zoned");
> +        return;
> +    }

Please explain the rationale:

/*
 * Non-zoned block drivers do not follow zoned storage constraints
(i.e. sequential writes
 * to zones). Refuse mixing zoned and non-zoned drivers in a graph.
 */

> +
>      if (!QLIST_EMPTY(&child_bs->parents)) {
>          error_setg(errp, "The node %s already has a parent",
>                     child_bs->node_name);
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 42708012ff..e9ad1d8e1e 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -3924,6 +3924,7 @@ static BlockDriver bdrv_host_device = {
>      .format_name        = "host_device",
>      .protocol_name        = "host_device",
>      .instance_size      = sizeof(BDRVRawState),
> +    .is_zoned = false,

In C static struct fields are automatically initialized to 0/false.
This line can be omitted.

>      .bdrv_needs_filename = true,
>      .bdrv_probe_device  = hdev_probe_device,
>      .bdrv_parse_filename = hdev_parse_filename,
> @@ -3971,6 +3972,7 @@ static BlockDriver bdrv_zoned_host_device = {
>          .format_name = "zoned_host_device",
>          .protocol_name = "zoned_host_device",
>          .instance_size = sizeof(BDRVRawState),
> +        .is_zoned = true,
>          .bdrv_needs_filename = true,
>          .bdrv_probe_device  = hdev_probe_device,
>          .bdrv_parse_filename = hdev_parse_filename,
> diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
> index 6037871089..29f1ec9184 100644
> --- a/include/block/block_int-common.h
> +++ b/include/block/block_int-common.h
> @@ -141,6 +141,11 @@ struct BlockDriver {
>       */
>      bool is_format;
>
> +    /*
> +     * Set to true if the BlockDriver is a zoned block driver.
> +     */
> +    bool is_zoned;

This isn't powerful enough to express the constraints.
block/raw-format.c supports both non-zoned and zoned children (after
your patch) but it won't pass the check.

Perhaps add bool supports_zoned_children and change the check to:

if (child_bs->drv->is_zoned &&
!parent_bs->drv->supports_zoned_children) { ...refuse... }

Then raw-format would work on top of zoned_host_device as well as
still working on non-zoned BDSes.

Stefan
diff mbox series

Patch

diff --git a/block.c b/block.c
index 2c00dddd80..0e24582c7d 100644
--- a/block.c
+++ b/block.c
@@ -7945,6 +7945,13 @@  void bdrv_add_child(BlockDriverState *parent_bs, BlockDriverState *child_bs,
         return;
     }
 
+    if (parent_bs->drv->is_zoned != child_bs->drv->is_zoned) {
+        error_setg(errp, "Cannot add a %s child to a %s parent",
+                   child_bs->drv->is_zoned ? "zoned" : "non-zoned",
+                   parent_bs->drv->is_zoned ? "zoned" : "non-zoned");
+        return;
+    }
+
     if (!QLIST_EMPTY(&child_bs->parents)) {
         error_setg(errp, "The node %s already has a parent",
                    child_bs->node_name);
diff --git a/block/file-posix.c b/block/file-posix.c
index 42708012ff..e9ad1d8e1e 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3924,6 +3924,7 @@  static BlockDriver bdrv_host_device = {
     .format_name        = "host_device",
     .protocol_name        = "host_device",
     .instance_size      = sizeof(BDRVRawState),
+    .is_zoned = false,
     .bdrv_needs_filename = true,
     .bdrv_probe_device  = hdev_probe_device,
     .bdrv_parse_filename = hdev_parse_filename,
@@ -3971,6 +3972,7 @@  static BlockDriver bdrv_zoned_host_device = {
         .format_name = "zoned_host_device",
         .protocol_name = "zoned_host_device",
         .instance_size = sizeof(BDRVRawState),
+        .is_zoned = true,
         .bdrv_needs_filename = true,
         .bdrv_probe_device  = hdev_probe_device,
         .bdrv_parse_filename = hdev_parse_filename,
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 6037871089..29f1ec9184 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -141,6 +141,11 @@  struct BlockDriver {
      */
     bool is_format;
 
+    /*
+     * Set to true if the BlockDriver is a zoned block driver.
+     */
+    bool is_zoned;
+
     /*
      * Drivers not implementing bdrv_parse_filename nor bdrv_open should have
      * this field set to true, except ones that are defined only by their