diff mbox

[v2,2/2] block: auto-generated node-names

Message ID f9b451fde3aa9b57372f29ae2bb8e7bbecf81a35.1441140367.git.jcody@redhat.com
State New
Headers show

Commit Message

Jeff Cody Sept. 1, 2015, 10:30 p.m. UTC
If a node-name is not specified, automatically generate the node-name.

Generated node-names will use the "block" sub-system identifier.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

Comments

Markus Armbruster Sept. 18, 2015, 1:03 p.m. UTC | #1
Jeff Cody <jcody@redhat.com> writes:

> If a node-name is not specified, automatically generate the node-name.
>
> Generated node-names will use the "block" sub-system identifier.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: John Snow <jsnow@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/block.c b/block.c
> index d088ee0..892e127 100644
> --- a/block.c
> +++ b/block.c
> @@ -771,32 +771,39 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
>                                    const char *node_name,
>                                    Error **errp)
>  {
> +    char *gen_node_name = NULL;
> +
>      if (!node_name) {
> -        return;
> -    }
> -
> -    /* Check for empty string or invalid characters */
> -    if (!id_wellformed(node_name)) {
> -        error_setg(errp, "Invalid node name");
> -        return;
> +        gen_node_name = id_generate(ID_BLOCK);
> +        node_name = gen_node_name;

I'd do

        node_name = gen_node_name = id_generate(ID_BLOCK);

but that's clearly a matter of taste.

> +    } else {
> +        /* Check for empty string or invalid characters, but not if it is
> +         * generated (generated names use characters not available to the user)
> +         * */

Untidy comment.  Make it:

        /*
         * Check for empty string or invalid characters, but not if it
         * is generated (generated names use characters not available to
         * the user)
         */

> +        if (!id_wellformed(node_name)) {
> +            error_setg(errp, "Invalid node name");
> +            return;
> +        }
>      }

I'd probably do an else if here.  Additional benefit: smaller diff.

>  
>      /* takes care of avoiding namespaces collisions */
>      if (blk_by_name(node_name)) {
>          error_setg(errp, "node-name=%s is conflicting with a device id",
>                     node_name);
> -        return;
> +        goto out;
>      }
>  
>      /* takes care of avoiding duplicates node names */
>      if (bdrv_find_node(node_name)) {
>          error_setg(errp, "Duplicate node name");
> -        return;
> +        goto out;
>      }
>  
>      /* copy node name into the bs and insert it into the graph list */
>      pstrcpy(bs->node_name, sizeof(bs->node_name), node_name);
>      QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs, node_list);
> +out:
> +    g_free(gen_node_name);
>  }
>  
>  static QemuOptsList bdrv_runtime_opts = {
diff mbox

Patch

diff --git a/block.c b/block.c
index d088ee0..892e127 100644
--- a/block.c
+++ b/block.c
@@ -771,32 +771,39 @@  static void bdrv_assign_node_name(BlockDriverState *bs,
                                   const char *node_name,
                                   Error **errp)
 {
+    char *gen_node_name = NULL;
+
     if (!node_name) {
-        return;
-    }
-
-    /* Check for empty string or invalid characters */
-    if (!id_wellformed(node_name)) {
-        error_setg(errp, "Invalid node name");
-        return;
+        gen_node_name = id_generate(ID_BLOCK);
+        node_name = gen_node_name;
+    } else {
+        /* Check for empty string or invalid characters, but not if it is
+         * generated (generated names use characters not available to the user)
+         * */
+        if (!id_wellformed(node_name)) {
+            error_setg(errp, "Invalid node name");
+            return;
+        }
     }
 
     /* takes care of avoiding namespaces collisions */
     if (blk_by_name(node_name)) {
         error_setg(errp, "node-name=%s is conflicting with a device id",
                    node_name);
-        return;
+        goto out;
     }
 
     /* takes care of avoiding duplicates node names */
     if (bdrv_find_node(node_name)) {
         error_setg(errp, "Duplicate node name");
-        return;
+        goto out;
     }
 
     /* copy node name into the bs and insert it into the graph list */
     pstrcpy(bs->node_name, sizeof(bs->node_name), node_name);
     QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs, node_list);
+out:
+    g_free(gen_node_name);
 }
 
 static QemuOptsList bdrv_runtime_opts = {