From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54796) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XX5j9-0003lc-1M for qemu-devel@nongnu.org; Thu, 25 Sep 2014 05:52:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XX5j2-0005sx-SY for qemu-devel@nongnu.org; Thu, 25 Sep 2014 05:52:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:31200) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XX5j2-0005rB-Kr for qemu-devel@nongnu.org; Thu, 25 Sep 2014 05:52:36 -0400 Date: Thu, 25 Sep 2014 11:52:26 +0200 From: Kevin Wolf Message-ID: <20140925095226.GG4667@noname.redhat.com> References: <1411622627-22110-1-git-send-email-tony@bakeyournoodle.com> <87d2akdxug.fsf@blackfin.pond.sub.org> <20140925073030.GA4667@noname.redhat.com> <20140925090041.GE20349@thor.bakeyournoodle.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="r5Pyd7+fXNt84Ff3" Content-Disposition: inline In-Reply-To: <20140925090041.GE20349@thor.bakeyournoodle.com> Subject: Re: [Qemu-devel] [PATCH] block/raw-posix: use seek_hole ahead of fiemap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org, =?iso-8859-1?Q?P=E1draig?= Brady , Michael Steffens , Stefan Hajnoczi , Max Reitz --r5Pyd7+fXNt84Ff3 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 25.09.2014 um 11:00 hat Tony Breeds geschrieben: > On Thu, Sep 25, 2014 at 09:30:30AM +0200, Kevin Wolf wrote: >=20 > > Does this fix the problem or does it just make it less likely that it > > becomes apparent? >=20 > Sorry for not making this clearer in my commit message. >=20 > I haven't been able to reproduce the corruption with the fiemap flag > change. > =20 > > If there is a data corruptor, we need to fix it, not just ensure that > > only the less common environments are affected. >=20 > I agree. I believe that the FIEMAP_FLAG_SYNC flag change fixes the > corrupter and then, as you say, makes that code less commonly executed. > =20 > > That looks like a logically separate change, so it should probably be > > a separate patch. >=20 > Sure I can do that, and be more explicit about the reason in the commit > message. > =20 > > Is this fix for the corruptor? The commit message doesn't make it > > clear. If so and fiemap is safe now, why would we still prefer > > seek_hole? >=20 > The preference for seek_hole was a suggestion from P=E1draig Brady , so > I'll let him defend that :) but as I said above I think it was about=20 > reducing the situations where fiemap was/is called. Okay. I'm not opposed to the change, we just need to split and document it properly. So first we should fix the bug by adding FIEMAP_FLAG_SYNC. Or actually, it might just be a workaround for a kernel bug. Is the expected behaviour documented, and if yes, what is it? We should describe as precisely as possible what the situation is and why we have to add this flag in qemu. You can probably reuse a good part of your current commit message for that and extend it a bit. And then we may or may not make the second change with the preference for seek_hole. If FIEMAP_FLAG_SYNC actually does additional writes or an fsync(), switching is a performance optimisation and might be justified as such. Maybe P=E1draig has some more input on this. Kevin --r5Pyd7+fXNt84Ff3 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJUI+XaAAoJEH8JsnLIjy/WzX8P/iHv4jk77Hp9t7oQ00oT3Mpf 1m5CPoFPzdlSVYOXZ8zB0ZBG9gmiyFRYsww0Poqpfx0IQEEmUMTZiPMsd87vLne+ ksXrlWdDghB1UCf/ZeFW2OIkrnRUGKzg3rLVvoGt5ZpY8EPd39rWUbODEjfRtiBU tiAkEvfNsJrWvkyogZrDP6ieTKz7xjitKYlnWuPeQVofaF1bf7dRJ8dhosfzr2du UYIRoCv1Az/V3AYi6gABfW6b83tow273p4dFqUNw3+T8HW25pCNA+CBjz5tDrsea 2RvE8Jz1rRnyl9sAOvJorklhhlxeKmd8WVsbHNWMgeQfLH6Zjb8LuNKhOYZJfL2r oGmvJ2n78UBRakPluAMeLqUba2bA9FhPkjIUUb/cOMiOZZJygRp9hubORWs+QRx3 AD0mZvWN9i1SKOgKmth9j5w2SIVdfKjuGc/4NX9do6V1XFfnokVkD8zCUhokLOoV VWXL440KrD1d9dCxYy9Fmqv253WK1hB69KJ6a2ZWUPf26ryf2/nI5YTAIuv/Nsk+ W06ereaQuw0yehCAwGb5fTAnziLCA8l8+ic3XsmJ0nlIn6W4mWJU6f+sgJfo0Eyy RxbRrgU8T6JUHCst0uJOSS7pxaM/G0RUcMDICgJ5Zo+4Elh3M8UvEhrp0giXN/12 i5DJtEgsp8hMibW6Y3qk =ES+0 -----END PGP SIGNATURE----- --r5Pyd7+fXNt84Ff3--