From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51040) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XkE5c-00062U-UW for qemu-devel@nongnu.org; Fri, 31 Oct 2014 11:27:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XjxYf-0001lr-VM for qemu-devel@nongnu.org; Thu, 30 Oct 2014 17:47:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37471) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjxYf-0001lX-NX for qemu-devel@nongnu.org; Thu, 30 Oct 2014 17:47:05 -0400 Message-ID: <5452B1D5.9050503@redhat.com> Date: Thu, 30 Oct 2014 15:47:01 -0600 From: Eric Blake MIME-Version: 1.0 References: <1414559044-14501-1-git-send-email-famz@redhat.com> <1414559044-14501-5-git-send-email-famz@redhat.com> In-Reply-To: <1414559044-14501-5-git-send-email-famz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="GGEKfKFsLfEjTjRxNftebTRHBv5tKeLdq" Subject: Re: [Qemu-devel] [PATCH 4/4] qmp: Add optional switch "query-nodes" in query-blockstats List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , Benoit Canet , Markus Armbruster , Max Reitz , Stefan Hajnoczi , Luiz Capitulino This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --GGEKfKFsLfEjTjRxNftebTRHBv5tKeLdq Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/28/2014 11:04 PM, Fam Zheng wrote: > This bool option will allow query all the node names. It iterates all > the BDSes that are assigned a name, also in this case don't query up th= e > backing chain. >=20 > Signed-off-by: Fam Zheng > --- > block/qapi.c | 20 +++++++++++++------- > hmp.c | 2 +- > qapi/block-core.json | 4 +++- > qmp-commands.hx | 2 +- > 4 files changed, 18 insertions(+), 10 deletions(-) >=20 > -BlockStatsList *qmp_query_blockstats(Error **errp) > +BlockStatsList *qmp_query_blockstats(bool has_query_nodes, > + bool query_nodes, > + Error **errp) > { > BlockStatsList *head =3D NULL, **p_next =3D &head; > BlockDriverState *bs =3D NULL; > =20 > - while ((bs =3D bdrv_next(bs))) { > + /* Just to be safe if query_nodes is not always intialized */ s/intialized/initialized/ > + query_nodes =3D query_nodes && has_query_nodes; If things aren't initialized (was true a while ago, but we recently fixed that to ensure 0 initialization, although no one yet really relies on the guarantee), then valgrind could complain of a branch on an uninit memory location. Idiomatically, this is usually written: query_nodes =3D has_query_nodes && query_nodes; to pacify valgrind if we hadn't zero-initialized; although logically, the result is identical, so I don't care if you leave it. > +++ b/qapi/block-core.json > @@ -434,7 +434,9 @@ > # > # Since: 0.14.0 > ## > -{ 'command': 'query-blockstats', 'returns': ['BlockStats'] } > +{ 'command': 'query-blockstats', > + 'data': {'*query-nodes': 'bool' }, > + 'returns': ['BlockStats'] } Max correctly pointed out that this is missing documentation. The idea looks sane; it will visit all named nodes (whether or not those nodes are also reachable from named devices), and omit any unnamed nodes (right now, libvirt would have to be taught to name nodes, or Jeff's patch to auto-name nodes will avoid that problem). Hmm, I wonder - if we are adding an optional parameter that controls what to iterate over, should we also add an optional parameter that says to limit the output to a given input name? Then again, we don't have very many existing query-* commands that filter, and at any rate, adding such a filter should be its own patch. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --GGEKfKFsLfEjTjRxNftebTRHBv5tKeLdq Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg iQEcBAEBCAAGBQJUUrHVAAoJEKeha0olJ0NqwqQH/0UWzxybvmHDArry48d4RvLQ bx+of1EaEt1XgswkGncS0swkk6aKPVVGAsI5jezbmWjEx7ZMQy7UBz5sAfHeKC36 vGHkYH4tKuurtocu5jdzRfq6mZg+LjK/hf5JPl3Kw1yHaIJDrlO4DQjv9dYW6r5c hQzrcDpGEsPY1sg/x0KMqg4/oC8dK0dXqKSSBq3b7qIx3vggRXjcurkpZAlFAP/U fqzOFJxQCBaVYhcceADyOMiEUS5VXcIYcG/VbZhNzSloPe8P2n7XWi6eR5Roj1cu iq5I2QlgomrMv0v7Q9MyZ0eFK6rEcZIbKaDPvsWwFyePVjPhC4slGnbl+uRmzLo= =3MTn -----END PGP SIGNATURE----- --GGEKfKFsLfEjTjRxNftebTRHBv5tKeLdq--