qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 22/34] block: Exclude nested options only for ch


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH 22/34] block: Exclude nested options only for children in append_open_options()
Date: Wed, 13 May 2015 14:50:04 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0

On 13.05.2015 14:49, Max Reitz wrote:
On 08.05.2015 19:21, Kevin Wolf wrote:
Some drivers have nested options (e.g. blkdebug rule arrays), which
don't belong to a child node and shouldn't be removed. Don't remove all
options with "." in their name, but check for the complete prefixes of
actually existing child nodes.

Signed-off-by: Kevin Wolf <address@hidden>
---
  block.c                   | 30 +++++++++++++++++++-----------
  include/block/block_int.h |  1 +
  2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/block.c b/block.c
index e329a47..e9a1d76 100644
--- a/block.c
+++ b/block.c
@@ -81,7 +81,7 @@ static QLIST_HEAD(, BlockDriver) bdrv_drivers =
static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, const char *reference, QDict *options, int flags,
-                             BlockDriverState* parent,
+ BlockDriverState* parent, const char *child_name,
                               const BdrvChildRole *child_role,
                               BlockDriver *drv, Error **errp);
@@ -1174,8 +1174,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
      backing_hd = NULL;
      ret = bdrv_open_inherit(&backing_hd,
*backing_filename ? backing_filename : NULL,
-                            reference, options, 0, bs, &child_backing,
-                            NULL, &local_err);
+                            reference, options, 0, bs, "backing",
+                            &child_backing, NULL, &local_err);
      if (ret < 0) {
          bs->open_flags |= BDRV_O_NO_BACKING;
          error_setg(errp, "Could not open backing file: %s",
@@ -1238,7 +1238,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
      }
ret = bdrv_open_inherit(pbs, filename, reference, image_options, 0,
-                            parent, child_role, NULL, errp);
+                            parent, bdref_key, child_role, NULL, errp);
    done:
      qdict_del(options, bdref_key);
@@ -1312,11 +1312,13 @@ out:
    static void bdrv_attach_child(BlockDriverState *parent_bs,
                                BlockDriverState *child_bs,
+                              const char *child_name,
                                const BdrvChildRole *child_role)
  {
      BdrvChild *child = g_new(BdrvChild, 1);
      *child = (BdrvChild) {
          .bs     = child_bs,
+        .name   = child_name,
          .role   = child_role,
      };
@@ -1340,7 +1342,7 @@ static void bdrv_attach_child(BlockDriverState *parent_bs,
   */
static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, const char *reference, QDict *options, int flags,
-                             BlockDriverState* parent,
+ BlockDriverState* parent, const char *child_name,
                               const BdrvChildRole *child_role,
                               BlockDriver *drv, Error **errp)
  {
@@ -1376,7 +1378,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
          }
          bdrv_ref(bs);
          if (child_role) {
-            bdrv_attach_child(parent, bs, child_role);
+            bdrv_attach_child(parent, bs, child_name, child_role);
          }
          *pbs = bs;
          return 0;
@@ -1519,7 +1521,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
      }
        if (child_role) {
-        bdrv_attach_child(parent, bs, child_role);
+        bdrv_attach_child(parent, bs, child_name, child_role);
      }
        QDECREF(options);
@@ -1563,7 +1565,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
                BlockDriver *drv, Error **errp)
  {
return bdrv_open_inherit(pbs, filename, reference, options, flags, NULL,
-                             NULL, drv, errp);
+                             NULL, NULL, drv, errp);
  }
    typedef struct BlockReopenQueueEntry {
@@ -2093,7 +2095,7 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
      /* The contents of 'tmp' will become bs_top, as we are
       * swapping bs_new and bs_top contents. */
      bdrv_set_backing_hd(bs_top, bs_new);
-    bdrv_attach_child(bs_top, bs_new, &child_backing);
+    bdrv_attach_child(bs_top, bs_new, "backing", &child_backing);
  }
    static void bdrv_delete(BlockDriverState *bs)
@@ -4037,13 +4039,19 @@ static bool append_open_options(QDict *d, BlockDriverState *bs)
  {
      const QDictEntry *entry;
      QemuOptDesc *desc;
+    BdrvChild *child;
      bool found_any = false;
        for (entry = qdict_first(bs->options); entry;
           entry = qdict_next(bs->options, entry))
      {
-        /* Only take options for this level */
-        if (strchr(qdict_entry_key(entry), '.')) {
+        /* Exclude options for children */
+        QLIST_FOREACH(child, &bs->children, next) {
+            if (strstart(qdict_entry_key(entry), child->name, NULL)) {

I think the prefix should be "#{child->name}.", tested like "if (strstart(..., &ptr) && *ptr == '.')".

Make that "(*ptr == '.' || !*ptr)", I think.

It doesn't matter now, so I'm not sure whether I can give an R-b anyway... I guess when in doubt, back out. So I won't. :-)

Max

+                break;
+            }
+        }
+        if (child) {
              continue;
          }
  diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2fad5f8..90da3f7 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -336,6 +336,7 @@ extern const BdrvChildRole child_format;
    typedef struct BdrvChild {
      BlockDriverState *bs;
+    const char *name;
      const BdrvChildRole *role;
      QLIST_ENTRY(BdrvChild) next;
  } BdrvChild;





reply via email to

[Prev in Thread] Current Thread [Next in Thread]