On Wed, Jun 18, 2014 at 02:53:15PM +0200, Benoît Canet wrote: > The Tuesday 17 Jun 2014 à 17:53:49 (-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 > > Jeff can't we simply enforce the namespace separation with a check on the > QDict > option content ? > This way we could be sure that the user can't input a node-name starting with > __qemu. >
That still would not stop a user from trying to 'predict' or assuming what a node name would be ("oh, it is the first drive, it is probably __qemu##0000", etc...). Having the combination of the incrementing counter and the random string generation guarantees 2 things: it will always be unique in a qemu session, and it is not predictable by the user. The "__qemu##" just helps to visually identify it as a qemu generated. Although if you are strictly concerned about namespace confusion, we could enforce the namespace as you suggest, so a user could not create a node-name that would look like a qemu-generated node-name. Even in that case, I would still want to keep the sequential number + random string. > > > > 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 <ebl...@redhat.com> > > Signed-off-by: Jeff Cody <jc...@redhat.com> > > --- > > block.c | 16 +++++++++++++++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/block.c b/block.c > > index 43abe96..da32bb0 100644 > > --- a/block.c > > +++ b/block.c > > @@ -843,12 +843,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.9.3 > >