diff mbox

[v5,01/10] block: Auto-generate node_names for each BDS entry

Message ID 781eb304e07fb4a333e76cea68958a978bb0bfa3.1402683788.git.jcody@redhat.com
State New
Headers show

Commit Message

Jeff Cody June 13, 2014, 6:52 p.m. UTC
Currently, node_name is only filled in when done so explicitly by the
user.  If no node_name is specified, then the node name field is not
populated.

If node_names are automatically generated when not specified, that means
that all block job operations can be done by reference to the unique
node_name field.  This eliminates ambiguity in resolving filenames
(relative filenames, or file descriptors, symlinks, mounts, etc..) that
qemu currently needs to deal with.

If a node name is specified, then it will not be automatically
generated for that BDS entry.

If it is automatically generated, it will be prefaced with "__qemu##",
followed by 8 characters of a unique number, followed by 8 random
ASCII characters in the range of 'A-Z'.  Some sample generated node-name
strings:
    __qemu##00000000IAIYNXXR
    __qemu##00000002METXTRBQ
    __qemu##00000001FMBORDWG

The prefix is to aid in identifying it as a qemu-generated name, the
numeric portion is to guarantee uniqueness in a given qemu session, and
the random characters are to further avoid any accidental collisions
with user-specified node-names.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Benoît Canet June 16, 2014, 2:10 p.m. UTC | #1
The Friday 13 Jun 2014 à 14:52:29 (-0400), Jeff Cody wrote :
> Currently, node_name is only filled in when done so explicitly by the
> user.  If no node_name is specified, then the node name field is not
> populated.
> 
> If node_names are automatically generated when not specified, that means
> that all block job operations can be done by reference to the unique
> node_name field.  This eliminates ambiguity in resolving filenames
> (relative filenames, or file descriptors, symlinks, mounts, etc..) that
> qemu currently needs to deal with.
> 
> If a node name is specified, then it will not be automatically
> generated for that BDS entry.
> 
> If it is automatically generated, it will be prefaced with "__qemu##",
> followed by 8 characters of a unique number, followed by 8 random
> ASCII characters in the range of 'A-Z'.  Some sample generated node-name
> strings:
>     __qemu##00000000IAIYNXXR
>     __qemu##00000002METXTRBQ
>     __qemu##00000001FMBORDWG
> 
> The prefix is to aid in identifying it as a qemu-generated name, the
> numeric portion is to guarantee uniqueness in a given qemu session, and
> the random characters are to further avoid any accidental collisions
> with user-specified node-names.

if the __qemu## determined a different name space no collision resolution would be
needed.

Also Eric you must take care that these node name does not ends up in libvirt
XML since a new qemu process can generate a new different set of node name.

> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 17f763d..fd43016 100644
> --- a/block.c
> +++ b/block.c
> @@ -844,12 +844,26 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
>      return open_flags;
>  }
>  
> +#define GEN_NODE_NAME_PREFIX    "__qemu##"
> +#define GEN_NODE_NAME_MAX_LEN   (sizeof(GEN_NODE_NAME_PREFIX) + 8 + 8)
>  static void bdrv_assign_node_name(BlockDriverState *bs,
>                                    const char *node_name,
>                                    Error **errp)
>  {
> +    char gen_node_name[GEN_NODE_NAME_MAX_LEN];
> +    static uint32_t counter; /* simple counter to guarantee uniqueness */
> +
> +    /* if node_name is NULL, auto-generate a node name */
>      if (!node_name) {
> -        return;
> +        int len;
> +        snprintf(gen_node_name, GEN_NODE_NAME_MAX_LEN,
> +                 "%s%08x", GEN_NODE_NAME_PREFIX, counter++);
> +        len = strlen(gen_node_name);
> +        while (len < GEN_NODE_NAME_MAX_LEN - 1) {
> +            gen_node_name[len++] = g_random_int_range('A', 'Z');
> +        }
> +        gen_node_name[GEN_NODE_NAME_MAX_LEN - 1] = '\0';
> +        node_name = gen_node_name;
>      }
>  
>      /* empty string node name is invalid */
> -- 
> 1.8.3.1
> 
>
Eric Blake June 16, 2014, 2:15 p.m. UTC | #2
On 06/16/2014 08:10 AM, Benoît Canet wrote:

>> The prefix is to aid in identifying it as a qemu-generated name, the
>> numeric portion is to guarantee uniqueness in a given qemu session, and
>> the random characters are to further avoid any accidental collisions
>> with user-specified node-names.
> 
> if the __qemu## determined a different name space no collision resolution would be
> needed.
> 
> Also Eric you must take care that these node name does not ends up in libvirt
> XML since a new qemu process can generate a new different set of node name.

Correct. Either libvirt will always generate its own node names (and the
presence of a qemu-generated name is a bug in libvirt), or libvirt will
query node-names as it runs, and clear them out when the qemu process
goes away.  It will NOT expose qemu names to user-visible XML (although
it should be safe to save the qemu names to internal-only XML used to
cache results across libvirtd restarts while still communicating to the
same qemu process).
Jeff Cody June 16, 2014, 2:18 p.m. UTC | #3
On Mon, Jun 16, 2014 at 04:10:41PM +0200, Benoît Canet wrote:
> The Friday 13 Jun 2014 à 14:52:29 (-0400), Jeff Cody wrote :
> > Currently, node_name is only filled in when done so explicitly by the
> > user.  If no node_name is specified, then the node name field is not
> > populated.
> > 
> > If node_names are automatically generated when not specified, that means
> > that all block job operations can be done by reference to the unique
> > node_name field.  This eliminates ambiguity in resolving filenames
> > (relative filenames, or file descriptors, symlinks, mounts, etc..) that
> > qemu currently needs to deal with.
> > 
> > If a node name is specified, then it will not be automatically
> > generated for that BDS entry.
> > 
> > If it is automatically generated, it will be prefaced with "__qemu##",
> > followed by 8 characters of a unique number, followed by 8 random
> > ASCII characters in the range of 'A-Z'.  Some sample generated node-name
> > strings:
> >     __qemu##00000000IAIYNXXR
> >     __qemu##00000002METXTRBQ
> >     __qemu##00000001FMBORDWG
> > 
> > The prefix is to aid in identifying it as a qemu-generated name, the
> > numeric portion is to guarantee uniqueness in a given qemu session, and
> > the random characters are to further avoid any accidental collisions
> > with user-specified node-names.
> 
> if the __qemu## determined a different name space no collision resolution would be
> needed.
> 

True.  The random string generation was pretty trivial, however, and
has an added benefit: it helps prevent the user / management software
from assuming or attempting to predict node names for images.

> Also Eric you must take care that these node name does not ends up in libvirt
> XML since a new qemu process can generate a new different set of node name.
>

Yes.. the node names are guaranteed to be unique, but are ephemeral in
nature.

> > 
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > Signed-off-by: Jeff Cody <jcody@redhat.com>
> > ---
> >  block.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/block.c b/block.c
> > index 17f763d..fd43016 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -844,12 +844,26 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
> >      return open_flags;
> >  }
> >  
> > +#define GEN_NODE_NAME_PREFIX    "__qemu##"
> > +#define GEN_NODE_NAME_MAX_LEN   (sizeof(GEN_NODE_NAME_PREFIX) + 8 + 8)
> >  static void bdrv_assign_node_name(BlockDriverState *bs,
> >                                    const char *node_name,
> >                                    Error **errp)
> >  {
> > +    char gen_node_name[GEN_NODE_NAME_MAX_LEN];
> > +    static uint32_t counter; /* simple counter to guarantee uniqueness */
> > +
> > +    /* if node_name is NULL, auto-generate a node name */
> >      if (!node_name) {
> > -        return;
> > +        int len;
> > +        snprintf(gen_node_name, GEN_NODE_NAME_MAX_LEN,
> > +                 "%s%08x", GEN_NODE_NAME_PREFIX, counter++);
> > +        len = strlen(gen_node_name);
> > +        while (len < GEN_NODE_NAME_MAX_LEN - 1) {
> > +            gen_node_name[len++] = g_random_int_range('A', 'Z');
> > +        }
> > +        gen_node_name[GEN_NODE_NAME_MAX_LEN - 1] = '\0';
> > +        node_name = gen_node_name;
> >      }
> >  
> >      /* empty string node name is invalid */
> > -- 
> > 1.8.3.1
> > 
> >
diff mbox

Patch

diff --git a/block.c b/block.c
index 17f763d..fd43016 100644
--- a/block.c
+++ b/block.c
@@ -844,12 +844,26 @@  static int bdrv_open_flags(BlockDriverState *bs, int flags)
     return open_flags;
 }
 
+#define GEN_NODE_NAME_PREFIX    "__qemu##"
+#define GEN_NODE_NAME_MAX_LEN   (sizeof(GEN_NODE_NAME_PREFIX) + 8 + 8)
 static void bdrv_assign_node_name(BlockDriverState *bs,
                                   const char *node_name,
                                   Error **errp)
 {
+    char gen_node_name[GEN_NODE_NAME_MAX_LEN];
+    static uint32_t counter; /* simple counter to guarantee uniqueness */
+
+    /* if node_name is NULL, auto-generate a node name */
     if (!node_name) {
-        return;
+        int len;
+        snprintf(gen_node_name, GEN_NODE_NAME_MAX_LEN,
+                 "%s%08x", GEN_NODE_NAME_PREFIX, counter++);
+        len = strlen(gen_node_name);
+        while (len < GEN_NODE_NAME_MAX_LEN - 1) {
+            gen_node_name[len++] = g_random_int_range('A', 'Z');
+        }
+        gen_node_name[GEN_NODE_NAME_MAX_LEN - 1] = '\0';
+        node_name = gen_node_name;
     }
 
     /* empty string node name is invalid */