From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41072) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abAYg-0002V6-Gr for qemu-devel@nongnu.org; Wed, 02 Mar 2016 12:27:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1abAYf-0001Qd-AX for qemu-devel@nongnu.org; Wed, 02 Mar 2016 12:27:34 -0500 Date: Wed, 2 Mar 2016 18:27:26 +0100 From: Kevin Wolf Message-ID: <20160302172726.GH4494@noname.redhat.com> References: <1456518142-9849-1-git-send-email-kwolf@redhat.com> <1456518142-9849-4-git-send-email-kwolf@redhat.com> <56D71E8D.4020309@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="lMM8JwqTlfDpEaS6" Content-Disposition: inline In-Reply-To: <56D71E8D.4020309@redhat.com> Subject: Re: [Qemu-devel] [PATCH 3/3] block/qapi: Include empty drives in query-blockstats List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org --lMM8JwqTlfDpEaS6 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 02.03.2016 um 18:10 hat Max Reitz geschrieben: > On 26.02.2016 21:22, Kevin Wolf wrote: > > Since commit 5ec18f8c, query-blockstats didn't return the statistics of > > drives without media any more because such drives have only a BB now, > > but not a BDS any more. > >=20 > > This patch fixes the regression so that query-blockstats iterates over > > BBs by default and empty drives are displayed again. > >=20 > > Signed-off-by: Kevin Wolf > > --- > > block/qapi.c | 48 +++++++++++++++++++++++++++++++++--------------- > > 1 file changed, 33 insertions(+), 15 deletions(-) > >=20 > > diff --git a/block/qapi.c b/block/qapi.c > > index 31ae879..6a4869a 100644 > > --- a/block/qapi.c > > +++ b/block/qapi.c > > @@ -355,7 +355,8 @@ static void bdrv_query_info(BlockBackend *blk, Bloc= kInfo **p_info, > > qapi_free_BlockInfo(info); > > } > > =20 > > -static BlockStats *bdrv_query_stats(const BlockDriverState *bs, > > +static BlockStats *bdrv_query_stats(BlockBackend *blk, > > + const BlockDriverState *bs, > > bool query_backing); > > =20 > > static void bdrv_query_blk_stats(BlockStats *s, BlockBackend *blk) > > @@ -363,6 +364,9 @@ static void bdrv_query_blk_stats(BlockStats *s, Blo= ckBackend *blk) > > BlockAcctStats *stats =3D blk_get_stats(blk); > > BlockAcctTimedStats *ts =3D NULL; > > =20 > > + s->has_device =3D true; > > + s->device =3D g_strdup(blk_name(blk)); > > + > > s->stats->rd_bytes =3D stats->nr_bytes[BLOCK_ACCT_READ]; > > s->stats->wr_bytes =3D stats->nr_bytes[BLOCK_ACCT_WRITE]; > > s->stats->rd_operations =3D stats->nr_ops[BLOCK_ACCT_READ]; > > @@ -428,11 +432,6 @@ static void bdrv_query_blk_stats(BlockStats *s, Bl= ockBackend *blk) > > static void bdrv_query_bds_stats(BlockStats *s, const BlockDriverState= *bs, > > bool query_backing) > > { > > - if (bdrv_get_device_name(bs)[0]) { > > - s->has_device =3D true; > > - s->device =3D g_strdup(bdrv_get_device_name(bs)); > > - } > > - > > if (bdrv_get_node_name(bs)[0]) { > > s->has_node_name =3D true; > > s->node_name =3D g_strdup(bdrv_get_node_name(bs)); > > @@ -442,17 +441,18 @@ static void bdrv_query_bds_stats(BlockStats *s, c= onst BlockDriverState *bs, > > =20 > > if (bs->file) { > > s->has_parent =3D true; > > - s->parent =3D bdrv_query_stats(bs->file->bs, query_backing); > > + s->parent =3D bdrv_query_stats(NULL, bs->file->bs, query_backi= ng); > > } > > =20 > > if (query_backing && bs->backing) { > > s->has_backing =3D true; > > - s->backing =3D bdrv_query_stats(bs->backing->bs, query_backing= ); > > + s->backing =3D bdrv_query_stats(NULL, bs->backing->bs, query_b= acking); >=20 > This breaks my very important use case of having BBs on backing BDSs!!! >=20 > Kidding aside: Maybe someone actually wants to do this? Should we feel > bad for breaking query-blockstats for those BBs? This is only possible since commit d9b7b057 ('block: Allow references for backing files'), which wasn't in 2.5 yet. Apart from that, you still get the information because that second BB is enumerated as well. It's just not duplicated in the return value of a different BB any more. I think we can consider that a bug fix. Kevin > If no: >=20 > Reviewed-by: Max Reitz >=20 --lMM8JwqTlfDpEaS6 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJW1yJ+AAoJEH8JsnLIjy/WgnwP/i85/O4HuarOzbVNHvLUoacW J3NxIVrOGnE3PM2GlxU/HSwaw7GSabYVhAW70BzBTvOm64iOHvuaoyTvD7r6JU2u PujGrs+nXBAOW1S4YfD2U16T4rzkQ1UJLRu+LdAUWexwttgo0vbwEAujeyJI0JeB SbTvHNE96qILmxapXjH0fi7ot5Fm+xcalZqcqOwKJA3gLuw95fu1nsot0o8uX20V kZZKzeZC3lNncQe/3UV2hLi2MAsujCDZGOFDpLaZfVC88SzXEh+osftogL15nJ4V LqxRdwlGq/YZJ8oeu2YFluenMML8bxck2ANggTH6/vT5tjmV7eCtpQtuQP3trDNr pXd+lsOykQpPk3sAmRvmMDFKdCLcCcx0aW8eoJrXjaL3Qo7QoLo0yWjQFVvSKltx jZqUKJmtAqTFUUVF1wkuJz9hkdeH5NDEKG8sQfZR9Vv+M6oq/77i9aB9Zq1bup3U CFrf61pfnkD/UfQEi5Owgi04tH7LAdfec7s42m5/Dy9NLgb9r3u/pi1i7kGWtZx3 zu5htlUh5QCnWofJzqCTJlWHp2WJB26fYHYKl9q/+Wl5dGumPszvB8jdC2k1IfMM RfXgYrdJqg462d8MIwxcDfRanL+G4zfJ0KOTDO/iaZvxiFpmQg04Sd1QLqxR83ch PN7IvecKiiNqe1rMv3o0 =biAc -----END PGP SIGNATURE----- --lMM8JwqTlfDpEaS6--