From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59836) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aum0K-0002Ow-ML for qemu-devel@nongnu.org; Mon, 25 Apr 2016 15:17:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aum0J-0003Xo-IZ for qemu-devel@nongnu.org; Mon, 25 Apr 2016 15:17:08 -0400 References: <1461368452-10389-1-git-send-email-eblake@redhat.com> <1461368452-10389-45-git-send-email-eblake@redhat.com> <64C7C8AD-6202-476E-A8B5-AC63C06B3357@alex.org.uk> From: Eric Blake Message-ID: <571E6D2B.2040902@redhat.com> Date: Mon, 25 Apr 2016 13:16:59 -0600 MIME-Version: 1.0 In-Reply-To: <64C7C8AD-6202-476E-A8B5-AC63C06B3357@alex.org.uk> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="X7qX8TLGujTPCo4gvCSnTcNTAODhvh3u3" Subject: Re: [Qemu-devel] [PATCH v3 44/44] nbd: Implement NBD_OPT_BLOCK_SIZE on client List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Bligh Cc: "qemu-devel@nongnu.org" , Kevin Wolf , Paolo Bonzini , qemu-block@nongnu.org, Max Reitz This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --X7qX8TLGujTPCo4gvCSnTcNTAODhvh3u3 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 04/25/2016 06:19 AM, Alex Bligh wrote: >=20 > On 23 Apr 2016, at 00:40, Eric Blake wrote: >=20 >> The upstream NBD Protocol has defined a new extension to allow >> the server to advertise block sizes to the client, as well as >> a way for the client to inform the server that it intends to >> obey block sizes. >> >> Pass any received sizes on to the block layer. >> >> Use the minimum block size as the sector size we pass to the >> kernel - which also has the nice effect of cooperating with >> (non-qemu) servers that don't do read-modify-write when exposing >> a block device with 4k sectors; it can also allow us to visit a >> file larger than 2T on a 32-bit kernel. >> >> + be32_to_cpus(&info->max_block); >> + TRACE("Block sizes are 0x%" PRIx32 ", 0x%" PRIx32 ", 0x%"= PRIx32, >> + info->min_block, info->opt_block, info->max_block);= >> + break; >> + >=20 > You should probably check min_block <=3D opt_block <=3D max_block here opt_block > max_block is possible if max_block is clamped to export size (in the degenerate case where you have a small export that is too small for the granularity of a hole or efficient I/O). But yes, some sanity checks that the server isn't horribly broken might be worthwhile. >=20 > Also should there be a check that BDRV_SECTOR_SIZE >=3D min_block? No, because we take the server's min_block and feed it into bs->request_align (which forces the block layer to comply with a minimum alignment larger than 512, using code already tested on physical block drives with 4k sectors), see the hunk in nbd-client.c. >> int nbd_init(int fd, QIOChannelSocket *sioc, NbdExportInfo *info) >> { >> - unsigned long sectors =3D info->size / BDRV_SECTOR_SIZE; >> - if (info->size / BDRV_SECTOR_SIZE !=3D sectors) { >> + unsigned long sector_size =3D MAX(BDRV_SECTOR_SIZE, info->min_blo= ck); >> + unsigned long sectors =3D info->size / sector_size; >> + if (info->size / sector_size !=3D sectors) { >> LOG("Export size %" PRId64 " too large for 32-bit kernel", inf= o->size); >> return -E2BIG; >> } >> @@ -724,18 +784,18 @@ int nbd_init(int fd, QIOChannelSocket *sioc, Nbd= ExportInfo *info) >> return -serrno; >> } >> >> - TRACE("Setting block size to %lu", (unsigned long)BDRV_SECTOR_SIZ= E); >> + TRACE("Setting block size to %lu", sector_size); >> >> - if (ioctl(fd, NBD_SET_BLKSIZE, (unsigned long)BDRV_SECTOR_SIZE) <= 0) { >> + if (ioctl(fd, NBD_SET_BLKSIZE, sector_size) < 0) { We also feed the maximum of 512 or the advertised minimum block size into the kernel when using ioctl() for the kernel to take over transmission phase; although I'm not certain whether the kernel obeys NBD_SET_BLKSIZE as a hint rather than a hard rule - but if that needs patching, it needs patching in the kernel implementation, not in qemu. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --X7qX8TLGujTPCo4gvCSnTcNTAODhvh3u3 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/ iQEcBAEBCAAGBQJXHm0rAAoJEKeha0olJ0NqEpEH/jCJQDA4p08ObAijEgiFP8t/ 5TFT3/AstLXuTibLzB/dIRPslvyfxv6whTsci+BZlIwWnw1e1zgRMNLGamwGESz0 bSIJQBKipni/kftW4ui1t+0fQABB0g8yRYB0tPuCPGQKtt/NpTmKnGqTyrhR6egz tU6dy0y6aW4sHKZSWM/IduAwtPSUDnxiUl8JoZYrzIeqB/Zn6k2IXtwUkd/4XOK6 3Qp4MWW/+uaotQWS7Jj6g4DjN9nnn0Oah3u9bTFJz2LfCBn2VgQQpnqpefkjadkl 0278qdTEYJacea0SrgsQBCmHPTxGTgQqUAGVYXfe5e9SkXBdUg/6fFP7iQ3q2IU= =6JKc -----END PGP SIGNATURE----- --X7qX8TLGujTPCo4gvCSnTcNTAODhvh3u3--