From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35587) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c7oEw-0000tw-R6 for qemu-devel@nongnu.org; Fri, 18 Nov 2016 13:50:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c7oEv-0008RW-Vp for qemu-devel@nongnu.org; Fri, 18 Nov 2016 13:50:22 -0500 References: <20161118102452.5779-1-olaf@aepfle.de> <42ca6186-ca47-3c63-d2e0-54f2ed9f4be7@redhat.com> <20161118174108.GF5717@aepfle.de> From: Eric Blake Message-ID: Date: Fri, 18 Nov 2016 12:50:12 -0600 MIME-Version: 1.0 In-Reply-To: <20161118174108.GF5717@aepfle.de> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="xqL1kEQQ5w8jlE2Has8QulENeCfdjqfwt" 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: Olaf Hering Cc: qemu-block@nongnu.org, Kevin Wolf , Stefano Stabellini , "open list:All patches CC here" , Max Reitz , "open list:X86" , Anthony Perard This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --xqL1kEQQ5w8jlE2Has8QulENeCfdjqfwt From: Eric Blake To: Olaf Hering Cc: qemu-block@nongnu.org, Kevin Wolf , Stefano Stabellini , "open list:All patches CC here" , Max Reitz , "open list:X86" , Anthony Perard Message-ID: Subject: Re: [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges References: <20161118102452.5779-1-olaf@aepfle.de> <42ca6186-ca47-3c63-d2e0-54f2ed9f4be7@redhat.com> <20161118174108.GF5717@aepfle.de> In-Reply-To: <20161118174108.GF5717@aepfle.de> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/18/2016 11:41 AM, Olaf Hering wrote: > On Fri, Nov 18, Eric Blake wrote: >=20 >> On 11/18/2016 04:24 AM, Olaf Hering wrote: >>> + /* Overflowing byte limit? */ >>> + if ((sec_start + sec_count) > ((INT64_MAX + INT_MAX) >> BDRV_SEC= TOR_BITS)) { >> This is undefined. INT64_MAX + anything non-negative overflows int64,= >=20 > The expanded value used to be stored into a uint64_t before it was used= > here. A "cleanup" introduced this error. Thanks for spotting. >=20 >> If you are trying to detect guests that make a request that would cove= r >> more than INT64_MAX bytes, you can simplify. Besides, for as much >> storage as there is out there, I seriously doubt ANYONE will ever have= >> 2^63 bytes addressable through a single device. Why not just write it= as: >> >> if ((INT64_MAX >> BDRV_SECTOR_BITS) - sec_count < sec_start) { >=20 > That would always be false I think. I will resubmit with this: > if ((sec_start + sec_count) > (INT64_MAX >> BDRV_SECTOR_BITS)) { You're testing whether something overflows, but you don't want to cause overflow as part of the test. So use the commutative law to rewrite it to avoid sec_start+sec_count from overflowing, and you get: if (sec_start > (INT64_MAX >> BDRV_SECTOR_BITS) - sec_count) but that's exactly the expression I wrote above. >=20 > Regarding the cast for ->req, it has type blkif_request_t, but the > pointer needs to be assigned to type blkif_request_discard_t. Then why is the cast to (void*) instead of (blkif_request_discard_t*) ? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --xqL1kEQQ5w8jlE2Has8QulENeCfdjqfwt 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/ iQEcBAEBCAAGBQJYL01kAAoJEKeha0olJ0Nq4JsIAKIz9UGXcUBr/5VlOPLqAX1o IiTrrJ4JkX+C1yuuiw5DwOMRBH0Fb4C17hiJ9EXipyKD5HrlnVChK0r4mT14W6E6 9Obr5hIe6kfZcE57gvKM3Q/mgj/sQ+ylN0VXqdvF0/oW+kVlY8cW6x8z1PaZUSir Kl0iGJnyOgkQDcMHAJa987yz4FZRpWakYT6lkQoa43PBd64SkqOyuFeRqfbzLgd9 fnYm75zZOJNJ2y93wle/LWyaJ5Vd+/OIOFKhF6j+Ce07aXfB+6lqS9jVdH/vaG+X l5/BKHghGLcK9M3CcDtbQRlPkR/297nZJWAkBeQO1vLSqzx/0Hm31HmFzNXYcmA= =Oh7v -----END PGP SIGNATURE----- --xqL1kEQQ5w8jlE2Has8QulENeCfdjqfwt-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Blake Subject: Re: [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges Date: Fri, 18 Nov 2016 12:50:12 -0600 Message-ID: References: <20161118102452.5779-1-olaf@aepfle.de> <42ca6186-ca47-3c63-d2e0-54f2ed9f4be7@redhat.com> <20161118174108.GF5717@aepfle.de> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="xqL1kEQQ5w8jlE2Has8QulENeCfdjqfwt" Return-path: In-Reply-To: <20161118174108.GF5717@aepfle.de> 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: Olaf Hering Cc: Kevin Wolf , "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 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --xqL1kEQQ5w8jlE2Has8QulENeCfdjqfwt Content-Type: multipart/mixed; boundary="7uW93xXCoHsFOKT4aQvtv8VeWcA4ePv9I"; protected-headers="v1" From: Eric Blake To: Olaf Hering Cc: qemu-block@nongnu.org, Kevin Wolf , Stefano Stabellini , "open list:All patches CC here" , Max Reitz , "open list:X86" , Anthony Perard Message-ID: Subject: Re: [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges References: <20161118102452.5779-1-olaf@aepfle.de> <42ca6186-ca47-3c63-d2e0-54f2ed9f4be7@redhat.com> <20161118174108.GF5717@aepfle.de> In-Reply-To: <20161118174108.GF5717@aepfle.de> --7uW93xXCoHsFOKT4aQvtv8VeWcA4ePv9I Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/18/2016 11:41 AM, Olaf Hering wrote: > On Fri, Nov 18, Eric Blake wrote: >=20 >> On 11/18/2016 04:24 AM, Olaf Hering wrote: >>> + /* Overflowing byte limit? */ >>> + if ((sec_start + sec_count) > ((INT64_MAX + INT_MAX) >> BDRV_SEC= TOR_BITS)) { >> This is undefined. INT64_MAX + anything non-negative overflows int64,= >=20 > The expanded value used to be stored into a uint64_t before it was used= > here. A "cleanup" introduced this error. Thanks for spotting. >=20 >> If you are trying to detect guests that make a request that would cove= r >> more than INT64_MAX bytes, you can simplify. Besides, for as much >> storage as there is out there, I seriously doubt ANYONE will ever have= >> 2^63 bytes addressable through a single device. Why not just write it= as: >> >> if ((INT64_MAX >> BDRV_SECTOR_BITS) - sec_count < sec_start) { >=20 > That would always be false I think. I will resubmit with this: > if ((sec_start + sec_count) > (INT64_MAX >> BDRV_SECTOR_BITS)) { You're testing whether something overflows, but you don't want to cause overflow as part of the test. So use the commutative law to rewrite it to avoid sec_start+sec_count from overflowing, and you get: if (sec_start > (INT64_MAX >> BDRV_SECTOR_BITS) - sec_count) but that's exactly the expression I wrote above. >=20 > Regarding the cast for ->req, it has type blkif_request_t, but the > pointer needs to be assigned to type blkif_request_discard_t. Then why is the cast to (void*) instead of (blkif_request_discard_t*) ? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --7uW93xXCoHsFOKT4aQvtv8VeWcA4ePv9I-- --xqL1kEQQ5w8jlE2Has8QulENeCfdjqfwt 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/ iQEcBAEBCAAGBQJYL01kAAoJEKeha0olJ0Nq4JsIAKIz9UGXcUBr/5VlOPLqAX1o IiTrrJ4JkX+C1yuuiw5DwOMRBH0Fb4C17hiJ9EXipyKD5HrlnVChK0r4mT14W6E6 9Obr5hIe6kfZcE57gvKM3Q/mgj/sQ+ylN0VXqdvF0/oW+kVlY8cW6x8z1PaZUSir Kl0iGJnyOgkQDcMHAJa987yz4FZRpWakYT6lkQoa43PBd64SkqOyuFeRqfbzLgd9 fnYm75zZOJNJ2y93wle/LWyaJ5Vd+/OIOFKhF6j+Ce07aXfB+6lqS9jVdH/vaG+X l5/BKHghGLcK9M3CcDtbQRlPkR/297nZJWAkBeQO1vLSqzx/0Hm31HmFzNXYcmA= =Oh7v -----END PGP SIGNATURE----- --xqL1kEQQ5w8jlE2Has8QulENeCfdjqfwt--