Message ID | 781eb304e07fb4a333e76cea68958a978bb0bfa3.1402683788.git.jcody@redhat.com |
---|---|
State | New |
Headers | show |
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 > >
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).
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 --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 */