From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45551) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFU6R-0006A7-PF for qemu-devel@nongnu.org; Tue, 21 Jun 2016 18:25:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bFU6Q-0005br-H7 for qemu-devel@nongnu.org; Tue, 21 Jun 2016 18:25:03 -0400 References: <1465939839-30097-1-git-send-email-eblake@redhat.com> <1465939839-30097-17-git-send-email-eblake@redhat.com> <20160621141211.GI4520@noname.redhat.com> From: Eric Blake Message-ID: <5769BEB6.1090206@redhat.com> Date: Tue, 21 Jun 2016 16:24:54 -0600 MIME-Version: 1.0 In-Reply-To: <20160621141211.GI4520@noname.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="DP05MClElou8TAhPwcOmAGTWgaj1Gv8LQ" Subject: Re: [Qemu-devel] [PATCH v2 16/17] block: Split bdrv_merge_limits() from bdrv_refresh_limits() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Stefan Hajnoczi , Fam Zheng , Max Reitz This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --DP05MClElou8TAhPwcOmAGTWgaj1Gv8LQ From: Eric Blake To: Kevin Wolf Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Stefan Hajnoczi , Fam Zheng , Max Reitz Message-ID: <5769BEB6.1090206@redhat.com> Subject: Re: [PATCH v2 16/17] block: Split bdrv_merge_limits() from bdrv_refresh_limits() References: <1465939839-30097-1-git-send-email-eblake@redhat.com> <1465939839-30097-17-git-send-email-eblake@redhat.com> <20160621141211.GI4520@noname.redhat.com> In-Reply-To: <20160621141211.GI4520@noname.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 06/21/2016 08:12 AM, Kevin Wolf wrote: > Am 14.06.2016 um 23:30 hat Eric Blake geschrieben: >> The raw block driver was blindly copying all limits from bs->file, >> even though: 1. the main bdrv_refresh_limits() already does this >> for many of gthe limits, and 2. blindly copying from the children >> can weaken any stricter limits that were already inherited from >> the backing dhain during the main bdrv_refresh_limits(). Also, >> the next patch is about to move .request_alignment into >> BlockLimits, and that is a limit that should NOT be copied from >> other layers in the BDS chain. >> >> Solve the issue by factoring out a new bdrv_merge_limits(), >> and using that function to properly merge limits when comparing >> two BlockDriverState objects. >> >> Signed-off-by: Eric Blake >> >=20 > Or in fact... >=20 >> diff --git a/block/raw_bsd.c b/block/raw_bsd.c >> index b1d5237..379ce8d 100644 >> --- a/block/raw_bsd.c >> +++ b/block/raw_bsd.c >> @@ -152,7 +152,7 @@ static int raw_get_info(BlockDriverState *bs, Bloc= kDriverInfo *bdi) >> >> static void raw_refresh_limits(BlockDriverState *bs, Error **errp) >> { >> - bs->bl =3D bs->file->bs->bl; >> + bdrv_merge_limits(&bs->bl, &bs->file->bs->bl); >> } The blind copy was not completely redundant, but the argument that we don't want a blind copy but instead want to rely on a merge_limits does indeed mean that... >=20 > ...isn't this completely redundant because bdrv_refresh_limits() alread= y > called bdrv_merge_limits(&bs->bl, &bs->file->bs->bl)? We could simply > remove the .bdrv_refresh_limits implementation from raw_bsd. =2E..this sounds like a better approach. I'll try it for v3. >=20 > And then bdrv_merge_limits() could even be static (I think factoring it= > out is a good idea anyway because it removes some duplication). Yes, even if static, it still gets called twice, and makes for a nicer way to quickly see which direction limits are constrained in (MIN_NON_ZERO vs. MAX), as well as to make any further tweaks in later patches easier to do from one spot (for example, we may want to add logic that clamps an opt limit [which uses MAX logic] to never exceed a max limit [which uses MIN_NON_ZERO logic]). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --DP05MClElou8TAhPwcOmAGTWgaj1Gv8LQ 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 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXab62AAoJEKeha0olJ0NqJoMH/0VGyv0DcyqNj+eVicakTU/h cUU2q18z3M6XmqAin5nbV8M5WyQ0rNLb3+RVPXtViDTBbEIAoK1eKE7PDSqaBVMy Q+aLHaJmItUwq0w4FSQx6Cd6UCS2R4EtGPjrQHElrBQSRlGtjBT8pB/3qG71ssHh 2iohpXqvaABw8Dahm0WlQakigcpmj46S9EYzMtw4U1ehrZbAzZe6f32EQVeBMdJM zOcSJK8MpQu/JyyEp04RFo33VyVhClsTgRJNm4rCVlNYEJKC5a9jU92bbv0no5Fv 00pOAxXTotKe6fOePMbDJ74YxacY3FUKmmorbWk7EjshyJJmsD3SSDzGJXt/7+0= =UuSO -----END PGP SIGNATURE----- --DP05MClElou8TAhPwcOmAGTWgaj1Gv8LQ--