From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52165) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bLWgW-0002MY-EA for qemu-devel@nongnu.org; Fri, 08 Jul 2016 10:23:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bLWgV-00023Y-7B for qemu-devel@nongnu.org; Fri, 08 Jul 2016 10:23:16 -0400 References: From: Max Reitz Message-ID: <9589dde4-638e-efe5-5791-a990fc93fe3d@redhat.com> Date: Fri, 8 Jul 2016 16:23:03 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="HdC7mDcIfLvLh6E5m4xfmLSRoXcE7bFkA" Subject: Re: [Qemu-devel] [PATCH 1/2] blockdev: Fix regression with the default naming of throttling groups List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia , qemu-devel@nongnu.org Cc: qemu-block@nongnu.org, Stefan Hajnoczi , qemu-stable@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --HdC7mDcIfLvLh6E5m4xfmLSRoXcE7bFkA From: Max Reitz To: Alberto Garcia , qemu-devel@nongnu.org Cc: qemu-block@nongnu.org, Stefan Hajnoczi , qemu-stable@nongnu.org Message-ID: <9589dde4-638e-efe5-5791-a990fc93fe3d@redhat.com> Subject: Re: [PATCH 1/2] blockdev: Fix regression with the default naming of throttling groups References: In-Reply-To: Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 08.07.2016 16:03, Alberto Garcia wrote: > When I/O limits are set for a block device, the name of the throttling > group is taken from the BlockBackend if the user doesn't specify one. >=20 > Commit efaa7c4eeb7490c6f37f3 moved the naming of the BlockBackend in > blockdev_init() to the end of the function, after I/O limits are set. > The consequence is that the throttling group gets an empty name. >=20 > Signed-off-by: Alberto Garcia > Reported-by: Stefan Hajnoczi > Cc: Max Reitz > Cc: qemu-stable@nongnu.org > --- > blockdev.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) >=20 > diff --git a/blockdev.c b/blockdev.c > index 0f8065c..3ad7d29 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -483,6 +483,7 @@ static BlockBackend *blockdev_init(const char *file= , QDict *bs_opts, > const char *id; > BlockdevDetectZeroesOptions detect_zeroes =3D > BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF; > + const char *blk_id; > const char *throttling_group =3D NULL; > =20 > /* Check common options by copying from bs_opts to opts, all other= options > @@ -512,6 +513,8 @@ static BlockBackend *blockdev_init(const char *file= , QDict *bs_opts, > =20 > writethrough =3D !qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, true)= ; > =20 > + blk_id =3D qemu_opts_id(opts); > + Side note: The "id" variable is supposed to contain the exact same value, but the string it points to is invalidated by the qdict_del(bs_opts, "id") call. So indeed we need to obtain the ID anew here (or we'd have to do g_strdup() before and g_free() after, which is cumbersome). But regarding the variable itself, you could have actually reused "id" (I only noticed that just now). But that's just a minor thing, so: Reviewed-by: Max Reitz > qdict_extract_subqdict(bs_opts, &interval_dict, "stats-intervals."= ); > qdict_array_split(interval_dict, &interval_list); > =20 > @@ -616,7 +619,7 @@ static BlockBackend *blockdev_init(const char *file= , QDict *bs_opts, > /* disk I/O throttling */ > if (throttle_enabled(&cfg)) { > if (!throttling_group) { > - throttling_group =3D blk_name(blk); > + throttling_group =3D blk_id; > } > blk_io_limits_enable(blk, throttling_group); > blk_set_io_limits(blk, &cfg); > @@ -625,7 +628,7 @@ static BlockBackend *blockdev_init(const char *file= , QDict *bs_opts, > blk_set_enable_write_cache(blk, !writethrough); > blk_set_on_error(blk, on_read_error, on_write_error); > =20 > - if (!monitor_add_blk(blk, qemu_opts_id(opts), errp)) { > + if (!monitor_add_blk(blk, blk_id, errp)) { > blk_unref(blk); > blk =3D NULL; > goto err_no_bs_opts; >=20 --HdC7mDcIfLvLh6E5m4xfmLSRoXcE7bFkA Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEvBAEBCAAZBQJXf7dIEhxtcmVpdHpAcmVkaGF0LmNvbQAKCRA7sUIC6DisrcnY B/9jRbe1T/mHz7y384qRTLjVNQTIavTRggg73GlqFvbCucUzjlEqb2ALPYnjdfD0 3caPSL5PPnWeUvCI1eY5Fe7hGyceUBp+nDrPOsnyxWNdynXNk5wekSgEgKXq4i7Z VjSxe6ZLx3nShmzpxpglCVKsA+n7un26i9Mq+yhYQZqAYoLAv4n3DOWLA/bAP23m H3vBwiZWeXaARbwJQwyVwEkxL0ostfvKM03wsWZmOaRcF6XdcFxpIArMFChJOAUD J8prBI5fI4gYrM/dhePWHYnq6VbM6/zEoW1qwvQXvPRxvTUa4E/MefnBbA9+wI9w 9NHiZRuW2ka1Uo81BvXWpY51 =Gjyk -----END PGP SIGNATURE----- --HdC7mDcIfLvLh6E5m4xfmLSRoXcE7bFkA--