[v3,04/14] block: Add blk_name_taken()

Commit Message

Max Reitz Feb. 16, 2016, 6:08 p.m. UTC

There may be BlockBackends which are not returned by blk_by_name(), but
do exist and have a name. blk_name_taken() allows testing whether a
specific name is in use already, independent of whether the BlockBackend
with that name is accessible through blk_by_name().

Signed-off-by: Max Reitz <[e-mail]>
---
 block.c                        |  4 ++--
 block/block-backend.c          | 19 ++++++++++++++++++-
 include/sysemu/block-backend.h |  1 +
 3 files changed, 21 insertions(+), 3 deletions(-)

Comments

Kevin Wolf Feb. 17, 2016, 10:29 a.m. UTC | #1

Am 16.02.2016 um 19:08 hat Max Reitz geschrieben:
> There may be BlockBackends which are not returned by blk_by_name(), but
> do exist and have a name.

Really? And if so, isn't this a bug?

I expect that a BB is always either visible to the user and has a name
that is resolved to this BB everywhere, or it's entirely internal and
doesn't need a name therefore.

Having a BB that is internal and therefore invisble, but has a name and
prevents the creation of another BB or BDS with the same name, must
certainly be confusing for the user.

> blk_name_taken() allows testing whether a
> specific name is in use already, independent of whether the BlockBackend
> with that name is accessible through blk_by_name().
> 
> Signed-off-by: Max Reitz <[e-mail]>

Kevin

Max Reitz Feb. 17, 2016, 3:36 p.m. UTC | #2

On 17.02.2016 11:29, Kevin Wolf wrote:
> Am 16.02.2016 um 19:08 hat Max Reitz geschrieben:
>> There may be BlockBackends which are not returned by blk_by_name(), but
>> do exist and have a name.
> 
> Really? And if so, isn't this a bug?

Depends on your definition of what the name is. :-)

As you said on IRC, denoting the monitor reference by that name seems
reasonable, so in that case it would be wrong behavior indeed.

> I expect that a BB is always either visible to the user and has a name
> that is resolved to this BB everywhere, or it's entirely internal and
> doesn't need a name therefore.
> 
> Having a BB that is internal and therefore invisble, but has a name and
> prevents the creation of another BB or BDS with the same name, must
> certainly be confusing for the user.

Yep, will change.

Max

>> blk_name_taken() allows testing whether a
>> specific name is in use already, independent of whether the BlockBackend
>> with that name is accessible through blk_by_name().
>>
>> Signed-off-by: Max Reitz <[e-mail]>
> 
> Kevin
>

Patch

diff --git a/block.c b/block.c
index efc3c43..a119840 100644
--- a/block.c
+++ b/block.c
@@ -847,8 +847,8 @@  static void bdrv_assign_node_name(BlockDriverState *bs,
         return;
     }
 
-    /* takes care of avoiding namespaces collisions */
-    if (blk_by_name(node_name)) {
+    /* takes care of avoiding namespace collisions */
+    if (blk_name_taken(node_name)) {
         error_setg(errp, "node-name=%s is conflicting with a device id",
                    node_name);
         goto out;
diff --git a/block/block-backend.c b/block/block-backend.c
index 30decb4..45d4057 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -87,7 +87,7 @@  BlockBackend *blk_new(const char *name, Error **errp)
         error_setg(errp, "Invalid device name");
         return NULL;
     }
-    if (blk_by_name(name)) {
+    if (blk_name_taken(name)) {
         error_setg(errp, "Device with id '%s' already exists", name);
         return NULL;
     }
@@ -291,6 +291,23 @@  BlockBackend *blk_by_name(const char *name)
 }
 
 /*
+ * This function should be used to check whether a certain BlockBackend name is
+ * already taken; blk_by_name() will only search in the list of monitor-owned
+ * BlockBackends which is not necessarily complete.
+ */
+bool blk_name_taken(const char *name)
+{
+    BlockBackend *blk = NULL;
+
+    while ((blk = blk_all_next(blk)) != NULL) {
+        if (!strcmp(name, blk->name)) {
+            return true;
+        }
+    }
+    return false;
+}
+
+/*
  * Return the BlockDriverState attached to @blk if any, else null.
  */
 BlockDriverState *blk_bs(BlockBackend *blk)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index ec30331..3fbf822 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -71,6 +71,7 @@  void blk_unref(BlockBackend *blk);
 void blk_remove_all_bs(void);
 const char *blk_name(BlockBackend *blk);
 BlockBackend *blk_by_name(const char *name);
+bool blk_name_taken(const char *name);
 BlockBackend *blk_next(BlockBackend *blk);
 
 BlockDriverState *blk_bs(BlockBackend *blk);