From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43343) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VoZqy-0007SY-5d for qemu-devel@nongnu.org; Thu, 05 Dec 2013 09:24:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VoZqs-00068P-1w for qemu-devel@nongnu.org; Thu, 05 Dec 2013 09:24:32 -0500 Received: from nodalink.pck.nerim.net ([62.212.105.220]:44706 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VoZqr-000684-GK for qemu-devel@nongnu.org; Thu, 05 Dec 2013 09:24:25 -0500 Date: Thu, 5 Dec 2013 15:24:16 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20131205142416.GD2892@irqsave.net> References: <1386077165-19577-1-git-send-email-benoit@irqsave.net> <1386077165-19577-4-git-send-email-benoit@irqsave.net> <529FBEDA.9050409@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <529FBEDA.9050409@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC V3 3/7] qapi: Add skeletton of command to query a drive bs graph. 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:46:34 (-0700), Eric Blake a =E9crit : > On 12/03/2013 06:26 AM, Beno=EEt Canet wrote: >=20 > In addition to Fam's review, >=20 > s/skeletton/skeleton/ in subject >=20 > > --- > > blockdev.c | 8 ++++++++ > > qapi-schema.json | 32 ++++++++++++++++++++++++++++++++ > > 2 files changed, 40 insertions(+) > >=20 > > diff --git a/blockdev.c b/blockdev.c > > index a474bb5..824e718 100644 > > --- a/blockdev.c > > +++ b/blockdev.c > > @@ -1940,6 +1940,14 @@ void qmp_drive_backup(const char *device, cons= t char *target, > > } > > } > > =20 > > +BlockGraphNode * qmp_query_drive_graph(const char *device, Error **e= rrp) >=20 > Style: no space after * >=20 > > +{ > > + /* the implementation of this function would recurse through the > > + * BlockDriverState graph to build it's result > > + */ > > + return NULL; >=20 > Shouldn't you set errp when returning failure? >=20 > > +++ b/qapi-schema.json > > @@ -2008,6 +2008,38 @@ > > { 'command': 'drive-backup', 'data': 'DriveBackup' } > > =20 > > ## > > +# @BlockGraphNode > > +# > > +# Information about a node of the block driver state graph > > +# > > +# @node-name: the name of the node in the graph > > +# > > +# @drv: the name of the block format used by this node as described = in > > +# @BlockDeviceInfo. >=20 > It would be nice if BlockDeviceInfo and BlockGraphNode used an enum > rather than an open-coded string for this field. >=20 > > +# > > +# @children: a list of @BlockGraphNode being the children of this no= de >=20 > s/being/that are/ >=20 > > +## > > +# @query-drive-graph > > +# > > +# Get the block driver states graph for a given drive > > +# > > +# @device: the name of the device to get the graph from > > +# > > +# Returns: the root @BlockGraphNode > > +# > > +# Since 1.8 > > +## > > +{ 'command': 'query-drive-graph', > > + 'data': { 'device': 'str' }, > > + 'returns': 'BlockGraphNode' } >=20 > Am I correct that it will be possible to have named nodes that are not > currently associated with any device? If so, how do we learn about > those nodes? Would it be better to have a command that returns an arra= y > of structs for all known node roots, with an optional member describing > which device owns that node root? Something like: The code have a list of all named nodes but not a list of named nodes roo= ts. Also it's difficult to get the device name for a named node because the b= ses don't have any backward pointers to their parents. It could be done by recursing into all the blockbackend bs but it's twist= ed. In fact I am wondering if we really need something to spit out the named = nodes topology in QMP for the simple reason that the names of the nodes are giv= en by the management so the management should already know the topology. Best regards Beno=EEt >=20 > # Represent a root of a block graph > # @root: a named node forming a root of a node graph > # @device: #optional device name that owns this root > { 'type': 'BlockGraphRoot', > 'data': { 'root': 'BlockGraphNode', > '*device': 'str' } } >=20 > # @query_drive-graphs > # Returns an array of all node graph roots > { 'command': 'query-drive-graphs', > 'returns': [ 'BlockGraphRoot' ] } >=20 > possibly with 'data':{'*device':'str'} to allow filtering to just a > 1-element array based on the device name (although I'm not sure if > providing the complexity of filtering is worth it). >=20 > --=20 > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >=20