From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40679) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VoZfW-0003Wf-I5 for qemu-devel@nongnu.org; Thu, 05 Dec 2013 09:12:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VoZfQ-0002cl-Ed for qemu-devel@nongnu.org; Thu, 05 Dec 2013 09:12:42 -0500 Received: from nodalink.pck.nerim.net ([62.212.105.220]:44694 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VoZfP-0002cJ-Uv for qemu-devel@nongnu.org; Thu, 05 Dec 2013 09:12:36 -0500 Date: Thu, 5 Dec 2013 15:12:25 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20131205141225.GB2892@irqsave.net> References: <1386077165-19577-1-git-send-email-benoit@irqsave.net> <1386077165-19577-5-git-send-email-benoit@irqsave.net> <529FC115.5050501@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <529FC115.5050501@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC V3 4/7] qmp: Allow block_passwd to manipulate bs graph nodes. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, famz@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, stefanha@redhat.com Le Wednesday 04 Dec 2013 =E0 16:56:05 (-0700), Eric Blake a =E9crit : > On 12/03/2013 06:26 AM, Beno=EEt Canet wrote: > > Signed-off-by: Benoit Canet > > --- > > =20 > > +BlockDriverState * bdrv_lookup_bs(bool has_device, const char * devi= ce, > > + bool has_node_name, const char * n= ode_name, >=20 > Style: no space after * (3 instances) >=20 > > + Error **errp) > > +{ > > + BlockDriverState *bs =3D NULL; > > + > > + if ((has_device && has_node_name) || > > + (!has_device && !has_node_name)) { >=20 > Could be shortened to: >=20 > if (has_device =3D=3D has_node_name) { >=20 > > + error_setg(errp, "Use either device or node-name but not bot= h."); >=20 > We tend to avoid trailing '.' on error messages >=20 > > =20 > > -void qmp_block_passwd(const char *device, const char *password, Erro= r **errp) > > +void qmp_block_passwd(bool has_device, const char * device, > > + bool has_node_name, const char * node_name, > > + const char * password, Error **errp) >=20 > Again, no space after '*' >=20 > > +++ b/include/block/block.h > > @@ -371,6 +371,9 @@ void bdrv_eject(BlockDriverState *bs, bool eject_= flag); > > const char *bdrv_get_format_name(BlockDriverState *bs); > > BlockDriverState *bdrv_find(const char *name); > > BlockDriverState *bdrv_find_node(const char *node_name); > > +BlockDriverState * bdrv_lookup_bs(bool has_device, const char * devi= ce, > > + bool has_node_name, const char * n= ode_name, > > + Error **errp); >=20 > And again >=20 > > +++ b/qapi-schema.json > > @@ -1675,7 +1675,11 @@ > > # determine which ones are encrypted, set the passwords with this co= mmand, and > > # then start the guest with the @cont command. > > # > > -# @device: the name of the device to set the password on > > +# Either @device or @node-name must be set but not both. > > +# > > +# @device: #optional the name of the block backend device to set the= password on > > +# > > +# @node-name: #optional graph node name to set the password on (Sinc= e 1.8) >=20 > 2.0 >=20 > > # > > # @password: the password to use for the device > > # > > @@ -1689,7 +1693,8 @@ > > # > > # Since: 0.14.0 > > ## > > -{ 'command': 'block_passwd', 'data': {'device': 'str', 'password': '= str'} } > > +{ 'command': 'block_passwd', 'data': {'*device': 'str', > > + '*node-name': 'str', 'password= ': 'str'} } >=20 > Seems like a reasonable addition; shouldn't cause any back-compat > problems (older management tools will always provide the now-optional > 'device'). >=20 > Is it intentional that you are not exposing this new functionality in H= MP? Yes, I don't foresee any way to print the graph in HMP so I am limiting t= he changes to QMP. Best regards Beno=EEt >=20 > --=20 > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >=20