From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58230) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c7lFl-0000qW-3r for qemu-devel@nongnu.org; Fri, 18 Nov 2016 10:39:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c7lFk-0005Ld-1r for qemu-devel@nongnu.org; Fri, 18 Nov 2016 10:39:01 -0500 Date: Fri, 18 Nov 2016 16:38:50 +0100 From: Kevin Wolf Message-ID: <20161118153850.GB4694@noname.redhat.com> References: <20161118102452.5779-1-olaf@aepfle.de> <5aa0cd45-0c73-b940-81e1-235625a428a2@redhat.com> <37C38B91-CDF7-443F-A517-3D16187B2A1F@aepfle.de> <3b7a53ed-10e1-e733-7454-d5ee38761da0@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="u3/rZRmxL6MmkK24" Content-Disposition: inline In-Reply-To: <3b7a53ed-10e1-e733-7454-d5ee38761da0@redhat.com> Subject: Re: [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Olaf Hering , qemu-block@nongnu.org, Stefano Stabellini , "open list:All patches CC here" , Max Reitz , "open list:X86" , Anthony Perard --u3/rZRmxL6MmkK24 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 18.11.2016 um 15:35 hat Eric Blake geschrieben: > On 11/18/2016 08:19 AM, Olaf Hering wrote: > > Am 18. November 2016 14:43:18 MEZ, schrieb Eric Blake : > >> On 11/18/2016 04:24 AM, Olaf Hering wrote: > >>> The guest sends discard requests as u64 sector/count pairs, but the > >>> block layer operates internally with s64/s32 pairs. The conversion > >>> leads to IO errors in the guest, the discard request is not > >> processed. > >> > >> Doesn't the block layer already split discard requests into 2^31 byte > >> chunks? > >=20 > > How would it do that without valid input? It was wrong before the sect= ors to bytes conversion, and now its even worse given that all the world fi= ts into an int. >=20 > Then it sounds like the real bug is that the block layer > bdrv_co_pdiscard() is buggy for taking 'int count' instead of 'uint64_t > count'. Eventually, I think the entire block layer should be fixed to > allow 64-bit count everywhere, and then auto-fragment it back down to 31 > bits (or even smaller, like NBD's 32M limit or Linux loopback device 64k > limit) as needed, rather than making all the backends reimplement > fragmentation. >=20 > >=20 > > Remember that there is no API to let the guest know about the limitatio= ns of the host.=20 >=20 > Correct. But the goal of the block layer is to hide the quirks, so that > the code handling the guest requests can offload all the common work to > one place. >=20 > Kevin, is it too late for 2.8 for patches that change bdrv_co_pdiscard > to take a 64-bit count? Or would that still be under bug-fix category > because of the xen use case? Given that we're already a few weeks into the freeze, I would very much prefer an isolated patch for xen_disk for 2.8, and then it can be cleaned up during the 2.9 cycle. Kevin --u3/rZRmxL6MmkK24 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJYLyCKAAoJEH8JsnLIjy/WYD0QAMW7Xdg4jmDHJshq9Jq4Oy4O UL+WFqpqNY2IeAq7lVLVwubFucRv+/GAuWTLqHzw6TPEfxx0jq2sVhChkymz5GmR HNvvR6vSvnsmo55BiM9XL24S+JlrSLHWHfHZABEvML0hanneocZpe2RsAI58dnen X4tJ6i5OX4pFuakveapZkpZiDYjVgsQqQmKUaVRcAXnQTGQDP7I0GY6GQM2Z/iP/ a1AWblcq6Y1WahybsASGvP3hKo/EBM+9nguYIx3YX0mnbUJO+9vrLfVUaRB/fHVg 6cUM9Sy4M+/jTr9nWNYNdIvZFA/oZCFkx4Gs1XWZrpwfzpOZxzrkYLL7ofKOun9C jUUlCzFy0c2d1tkfn+lj8rqTJ1e6Un/6E1haCN1mwkSyrYkM+Pghtp9MDfHofAjq vgB7w4ZOATlUcsfR/vFTSz5o42xfKY0dSu/b3MmFoKx6185IjmgySl24z3cTHgYQ /Z3KSGibtLsb/CZVSTzeHpjcXNcukXLY5ZDtrmLPeL1Ff1Yv6Tm7F8A4+SHpEbA2 ZOMc5nmGkrFYLdETxj2gO16/ejy7M8/tOpi0Nv1Orq4ScJ/Lkqtb57hmlDZ6AVQb MnyyNkGlbNMZf0znlA0XXzXfTBYojoxEG4LX53WVFmmr9yudpwEJLio189b7q48v 78h+P1wA7P4FFu4AHmNi =I7UD -----END PGP SIGNATURE----- --u3/rZRmxL6MmkK24-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Wolf Subject: Re: [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges Date: Fri, 18 Nov 2016 16:38:50 +0100 Message-ID: <20161118153850.GB4694@noname.redhat.com> References: <20161118102452.5779-1-olaf@aepfle.de> <5aa0cd45-0c73-b940-81e1-235625a428a2@redhat.com> <37C38B91-CDF7-443F-A517-3D16187B2A1F@aepfle.de> <3b7a53ed-10e1-e733-7454-d5ee38761da0@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="u3/rZRmxL6MmkK24" Return-path: Content-Disposition: inline In-Reply-To: <3b7a53ed-10e1-e733-7454-d5ee38761da0@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-block-bounces+gceqb-qemu-block=m.gmane.org@nongnu.org Sender: "Qemu-block" To: Eric Blake Cc: Olaf Hering , "open list:X86" , qemu-block@nongnu.org, "open list:All patches CC here" , Max Reitz , Stefano Stabellini , Anthony Perard List-Id: xen-devel@lists.xenproject.org --u3/rZRmxL6MmkK24 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 18.11.2016 um 15:35 hat Eric Blake geschrieben: > On 11/18/2016 08:19 AM, Olaf Hering wrote: > > Am 18. November 2016 14:43:18 MEZ, schrieb Eric Blake : > >> On 11/18/2016 04:24 AM, Olaf Hering wrote: > >>> The guest sends discard requests as u64 sector/count pairs, but the > >>> block layer operates internally with s64/s32 pairs. The conversion > >>> leads to IO errors in the guest, the discard request is not > >> processed. > >> > >> Doesn't the block layer already split discard requests into 2^31 byte > >> chunks? > >=20 > > How would it do that without valid input? It was wrong before the sect= ors to bytes conversion, and now its even worse given that all the world fi= ts into an int. >=20 > Then it sounds like the real bug is that the block layer > bdrv_co_pdiscard() is buggy for taking 'int count' instead of 'uint64_t > count'. Eventually, I think the entire block layer should be fixed to > allow 64-bit count everywhere, and then auto-fragment it back down to 31 > bits (or even smaller, like NBD's 32M limit or Linux loopback device 64k > limit) as needed, rather than making all the backends reimplement > fragmentation. >=20 > >=20 > > Remember that there is no API to let the guest know about the limitatio= ns of the host.=20 >=20 > Correct. But the goal of the block layer is to hide the quirks, so that > the code handling the guest requests can offload all the common work to > one place. >=20 > Kevin, is it too late for 2.8 for patches that change bdrv_co_pdiscard > to take a 64-bit count? Or would that still be under bug-fix category > because of the xen use case? Given that we're already a few weeks into the freeze, I would very much prefer an isolated patch for xen_disk for 2.8, and then it can be cleaned up during the 2.9 cycle. Kevin --u3/rZRmxL6MmkK24 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJYLyCKAAoJEH8JsnLIjy/WYD0QAMW7Xdg4jmDHJshq9Jq4Oy4O UL+WFqpqNY2IeAq7lVLVwubFucRv+/GAuWTLqHzw6TPEfxx0jq2sVhChkymz5GmR HNvvR6vSvnsmo55BiM9XL24S+JlrSLHWHfHZABEvML0hanneocZpe2RsAI58dnen X4tJ6i5OX4pFuakveapZkpZiDYjVgsQqQmKUaVRcAXnQTGQDP7I0GY6GQM2Z/iP/ a1AWblcq6Y1WahybsASGvP3hKo/EBM+9nguYIx3YX0mnbUJO+9vrLfVUaRB/fHVg 6cUM9Sy4M+/jTr9nWNYNdIvZFA/oZCFkx4Gs1XWZrpwfzpOZxzrkYLL7ofKOun9C jUUlCzFy0c2d1tkfn+lj8rqTJ1e6Un/6E1haCN1mwkSyrYkM+Pghtp9MDfHofAjq vgB7w4ZOATlUcsfR/vFTSz5o42xfKY0dSu/b3MmFoKx6185IjmgySl24z3cTHgYQ /Z3KSGibtLsb/CZVSTzeHpjcXNcukXLY5ZDtrmLPeL1Ff1Yv6Tm7F8A4+SHpEbA2 ZOMc5nmGkrFYLdETxj2gO16/ejy7M8/tOpi0Nv1Orq4ScJ/Lkqtb57hmlDZ6AVQb MnyyNkGlbNMZf0znlA0XXzXfTBYojoxEG4LX53WVFmmr9yudpwEJLio189b7q48v 78h+P1wA7P4FFu4AHmNi =I7UD -----END PGP SIGNATURE----- --u3/rZRmxL6MmkK24--