From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59614) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxFxS-0008VK-Ix for qemu-devel@nongnu.org; Wed, 18 Jun 2014 09:31:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WxFxM-0003j6-8j for qemu-devel@nongnu.org; Wed, 18 Jun 2014 09:31:22 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:51540 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxFxL-0003id-U9 for qemu-devel@nongnu.org; Wed, 18 Jun 2014 09:31:16 -0400 Date: Wed, 18 Jun 2014 15:31:13 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140618133113.GA4424@irqsave.net> References: <526da734a5f3cffd2eb56accafdb4add38c75270.1403041699.git.jcody@redhat.com> <20140618125315.GA4107@irqsave.net> <20140618131328.GA8856@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20140618131328.GA8856@localhost.localdomain> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v6 for 2.1 01/10] block: Auto-generate node_names for each BDS entry List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: =?iso-8859-1?Q?Beno=EEt?= Canet , kwolf@redhat.com, pkrempa@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com The Wednesday 18 Jun 2014 =E0 09:13:28 (-0400), Jeff Cody wrote : > On Wed, Jun 18, 2014 at 02:53:15PM +0200, Beno=EEt Canet wrote: > > The Tuesday 17 Jun 2014 =E0 17:53:49 (-0400), Jeff Cody wrote : > > > Currently, node_name is only filled in when done so explicitly by t= he > > > user. If no node_name is specified, then the node name field is no= t > > > populated. > > >=20 > > > If node_names are automatically generated when not specified, that = means > > > that all block job operations can be done by reference to the uniqu= e > > > node_name field. This eliminates ambiguity in resolving filenames > > > (relative filenames, or file descriptors, symlinks, mounts, etc..) = that > > > qemu currently needs to deal with. > > >=20 > > > If a node name is specified, then it will not be automatically > > > generated for that BDS entry. > > >=20 > > > 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 > >=20 > > 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 start= ing with > > __qemu. > > >=20 > 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. >=20 > 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. This way is fine for me. >=20 > > >=20 > > > The prefix is to aid in identifying it as a qemu-generated name, th= e > > > numeric portion is to guarantee uniqueness in a given qemu session,= and > > > the random characters are to further avoid any accidental collision= s > > > with user-specified node-names. > > >=20 > > > Reviewed-by: Eric Blake > > > Signed-off-by: Jeff Cody > > > --- > > > block.c | 16 +++++++++++++++- > > > 1 file changed, 15 insertions(+), 1 deletion(-) > > >=20 > > > 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; > > > } > > > =20 > > > +#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 unique= ness */ > > > + > > > + /* 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 =3D strlen(gen_node_name); > > > + while (len < GEN_NODE_NAME_MAX_LEN - 1) { > > > + gen_node_name[len++] =3D g_random_int_range('A', 'Z'); > > > + } > > > + gen_node_name[GEN_NODE_NAME_MAX_LEN - 1] =3D '\0'; > > > + node_name =3D gen_node_name; > > > } > > > =20 > > > /* empty string node name is invalid */ > > > --=20 > > > 1.9.3 > > >=20