From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50560) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W6iP9-0007sf-VW for qemu-devel@nongnu.org; Fri, 24 Jan 2014 10:10:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W6iP4-0007Yu-Nn for qemu-devel@nongnu.org; Fri, 24 Jan 2014 10:10:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:15003) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W6iP4-0007Vm-9n for qemu-devel@nongnu.org; Fri, 24 Jan 2014 10:10:42 -0500 Date: Fri, 24 Jan 2014 16:10:36 +0100 From: Kevin Wolf Message-ID: <20140124151036.GK3342@dhcp-200-207.str.redhat.com> References: <1390509099-695-1-git-send-email-benoit.canet@irqsave.net> <1390509099-695-3-git-send-email-benoit.canet@irqsave.net> <20140124145111.GB3510@irqsave.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20140124145111.GB3510@irqsave.net> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V6 2/8] block: Allow the user to define "node-name" option both on command line and QMP. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?Beno=EEt?= Canet Cc: armbru@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mreitz@redhat.com Am 24.01.2014 um 15:51 hat Beno=EEt Canet geschrieben: > Le Thursday 23 Jan 2014 =E0 21:31:33 (+0100), Beno=EEt Canet a =E9crit = : > > From: Beno=EEt Canet > >=20 > > Signed-off-by: Benoit Canet > > --- > > block.c | 36 ++++++++++++++++++++++++++++++++++++ > > qapi-schema.json | 2 ++ > > 2 files changed, 38 insertions(+) > >=20 > > diff --git a/block.c b/block.c > > index 60b70bc..d9d02d2 100644 > > --- a/block.c > > +++ b/block.c > > @@ -728,6 +728,33 @@ static int bdrv_open_flags(BlockDriverState *bs,= int flags) > > return open_flags; > > } > > =20 > > +static int bdrv_assign_node_name(BlockDriverState *bs, > > + const char *node_name, > > + Error **errp) > > +{ > > + if (!node_name) { > > + return 0; > > + } > > + > > + /* empty string node name is invalid */ > > + if (node_name[0] =3D=3D '\0') { > > + error_setg(errp, "Empty node name"); > > + return -EINVAL; > > + } > > + > > + /* takes care of avoiding duplicates node names */ > > + if (bdrv_find_node(node_name)) { > > + error_setg(errp, "Duplicate node name"); > > + return -EINVAL; > > + } > > + > > + /* 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); > > + > > + return 0; > > +} > > + > > /* > > * Common part for opening disk images and files > > * > > @@ -738,6 +765,7 @@ static int bdrv_open_common(BlockDriverState *bs,= BlockDriverState *file, > > { > > int ret, open_flags; > > const char *filename; > > + const char *node_name =3D NULL; > > Error *local_err =3D NULL; > > =20 > > assert(drv !=3D NULL); > > @@ -752,6 +780,14 @@ static int bdrv_open_common(BlockDriverState *bs= , BlockDriverState *file, > > =20 > > trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_na= me); > > =20 > > + node_name =3D qdict_get_try_str(options, "node-name"); >=20 > > + qdict_del(options, "node-name"); >=20 > Kevin: I wonder if I delete the option too early hence zeroing node-nam= e ?=20 For the record, we concluded on IRC that I would squash the following patch in: diff --git a/block.c b/block.c index 9dc1216..f043669 100644 --- a/block.c +++ b/block.c @@ -788,12 +788,11 @@ static int bdrv_open_common(BlockDriverState *bs, B= lockDriverState *file, trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name); =20 node_name =3D qdict_get_try_str(options, "node-name"); - qdict_del(options, "node-name"); - ret =3D bdrv_assign_node_name(bs, node_name, errp); if (ret < 0) { return ret; } + qdict_del(options, "node-name"); =20 /* bdrv_open() with directly using a protocol as drv. This layer is = already * opened, so assign it to bs (while file becomes a closed BlockDriv= erState) Kevin > > + > > + ret =3D bdrv_assign_node_name(bs, node_name, errp); > > + if (ret < 0) { > > + return ret; > > + } > > + > > /* bdrv_open() with directly using a protocol as drv. This layer= is already > > * opened, so assign it to bs (while file becomes a closed Block= DriverState) > > * and return immediately. */ > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 35f7b34..04167da 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -4090,6 +4090,7 @@ > > # @id: #optional id by which the new block device can be re= ferred to. > > # This is a required option on the top level of blockd= ev-add, and > > # currently not allowed on any other level. > > +# @node-name: #optional the name of a block driver state node (Sin= ce 2.0) > > # @discard: #optional discard-related options (default: ignore) > > # @cache: #optional cache-related options > > # @aio: #optional AIO backend (default: threads) > > @@ -4105,6 +4106,7 @@ > > { 'type': 'BlockdevOptionsBase', > > 'data': { 'driver': 'str', > > '*id': 'str', > > + '*node-name': 'str', > > '*discard': 'BlockdevDiscardOptions', > > '*cache': 'BlockdevCacheOptions', > > '*aio': 'BlockdevAioOptions', > > --=20 > > 1.8.3.2 > >=20