From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46548) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFfBQ-00085Q-8G for qemu-devel@nongnu.org; Wed, 22 Jun 2016 06:14:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bFfBP-0003fK-5v for qemu-devel@nongnu.org; Wed, 22 Jun 2016 06:14:56 -0400 Date: Wed, 22 Jun 2016 12:14:44 +0200 From: Kevin Wolf Message-ID: <20160622101444.GD6134@noname.redhat.com> References: <1465939839-30097-1-git-send-email-eblake@redhat.com> <1465939839-30097-18-git-send-email-eblake@redhat.com> <20160621141600.GJ4520@noname.redhat.com> <5769BF22.8020907@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="NMuMz9nt05w80d4+" Content-Disposition: inline In-Reply-To: <5769BF22.8020907@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 17/17] block: Move request_alignment into BlockLimit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Max Reitz , Stefan Hajnoczi , Fam Zheng , Ronnie Sahlberg , Paolo Bonzini , Peter Lieven --NMuMz9nt05w80d4+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 22.06.2016 um 00:26 hat Eric Blake geschrieben: > On 06/21/2016 08:16 AM, Kevin Wolf wrote: > > Am 14.06.2016 um 23:30 hat Eric Blake geschrieben: > >> It makes more sense to have ALL block size limit constraints > >> in the same struct. Improve the documentation while at it. > >> > >> Signed-off-by: Eric Blake > >> > >> --- >=20 > >> struct BlockLimits { > >> + /* Alignment requirement, in bytes, for offset/length of I/O > >> + * requests. Must be a power of 2 less than INT_MAX. A value of 0 > >> + * defaults to 1 for drivers with modern byte interfaces, and to > >> + * 512 otherwise. */ > >=20 > > No, a value of zero probably crashes qemu. The defaults apply when they > > aren't overridden by the driver, but this field is always non-zero. > >=20 >=20 > Then why does block.c have: >=20 > --- a/block.c > +++ b/block.c > @@ -1016,7 +1016,7 @@ static int bdrv_open_common(BlockDriverState *bs, > BdrvChild *file, >=20 > assert(bdrv_opt_mem_align(bs) !=3D 0); > assert(bdrv_min_mem_align(bs) !=3D 0); > - assert(is_power_of_2(bs->request_alignment) || bdrv_is_sg(bs)); > + assert(is_power_of_2(bs->bl.request_alignment) || bdrv_is_sg(bs)); >=20 > That says that if bdrv_is_sg(bs), we can indeed have request_alignment > be 0. Should I track that down and fix it first, so that > request_alignment is always nonzero? If so, is using '1' for > bdrv_is_sg(bs) the ideal solution? Hm, yes, forgot about this. bdrv_is_sg(bs) means that we never use the I/O functions, so there is no point in specifying any alignment. If we want to say something about 0 in the comment maybe mention bs->sg explicitly and that you can't use read/write functions then. But in fact, I think even sg devices already get a non-zero alignment, so the second part of the assertion might be redundant. Didn't test it, though, just had a quick look at the raw-posix code. Kevin --NMuMz9nt05w80d4+ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJXamUUAAoJEH8JsnLIjy/WM2AQAK0nHvXOiOVuIHf1xrZez5bq AVuogs0T9UzosRZN3c61IjoMfl3CCKgVtMvdKgsdaHGL0c4qr37pqGky1k5cWoUi PXBiJpgCchxERJ4ayBMPUEBgJrGEprO8nL0kjWddEY7hLi2SHQMLdCOmrru1gZ32 W7HAh8vZKE9KlX8Qz0hwjymkgC1rwzseCaB8HmLzy1yyBXSt8YEt9Djsm2K7bCN4 QbcMcvLjo0rdSKhMSv9lKYfctnwEnwu/GZe+boLl7qJ+eYUJXAXgBLxzmDv01G26 zZPes9e132VRlvwAs1udZq4EqEKjy6gBbUkTY7uCIIViHN5OEEjyecN5askpjmrr NzWwAF3sEV44ogzPDFYQl3mCOAWEz7qRDI0nGznqyg0z0TdI7O/e2alqRySDylLv gkUdnhP+0mx8I5uok/q08LO4B3Qrm/oTUDvE5gvZ6627NA83HtN2xYE3T/5kA1oV emxAXrAqYo/ZtyXJeLEfw4HQQv26RTtOJj8x3QKDn+8P0ZfO0jO6jK3LOq70S1S9 918Fks3uIvE/RSLk//UHlREvW1FHXJ2TLbs3DnFebOh7omGAuaiN7o3CvnWk5Yog Ywwwva2zC1j9/X9aV0Kumv3FyGgY53aS5D00RtE9lI/jqbn/y0m0AVn+7y6lyf+N cUv6gYRVi8iRYM9UYrlm =Ye9W -----END PGP SIGNATURE----- --NMuMz9nt05w80d4+--