From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41476) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XX90m-0003XL-V7 for qemu-devel@nongnu.org; Thu, 25 Sep 2014 09:23:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XX90h-0003Bl-GX for qemu-devel@nongnu.org; Thu, 25 Sep 2014 09:23:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:4917) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XX90g-00039x-Ra for qemu-devel@nongnu.org; Thu, 25 Sep 2014 09:23:03 -0400 Message-ID: <54240E4D.8020002@redhat.com> Date: Thu, 25 Sep 2014 06:45:01 -0600 From: Eric Blake MIME-Version: 1.0 References: <1411622627-22110-1-git-send-email-tony@bakeyournoodle.com> <87d2akdxug.fsf@blackfin.pond.sub.org> <20140925073030.GA4667@noname.redhat.com> In-Reply-To: <20140925073030.GA4667@noname.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="7GVt7mPxHp95a7bkN3ja9iiRxKKaklDaB" 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: Kevin Wolf , Markus Armbruster Cc: Tony Breeds , qemu-devel@nongnu.org, Max Reitz , =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= , Stefan Hajnoczi , Michael Steffens This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --7GVt7mPxHp95a7bkN3ja9iiRxKKaklDaB Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/25/2014 01:30 AM, Kevin Wolf wrote: > Am 25.09.2014 um 08:21 hat Markus Armbruster geschrieben: >> Please copy Kevin & Stefan for block patches. Doing that for you. I >> also copy Max, who left his fingerprints on commit 4f11aa8. >> >> Tony Breeds writes: >> >>> The command >>> qemu-img convert -O raw inputimage.qcow2 outputimage.raw >>> >>> intermittently creates corrupted output images, when the input image = is not yet fully synchronized to disk. This patch preferese the use of s= eek_hole checks to determine if the sector is present in the disk image. >=20 > Does this fix the problem or does it just make it less likely that it > becomes apparent? >=20 > If there is a data corruptor, we need to fix it, not just ensure that > only the less common environments are affected. >=20 >>> While we're there modify try_fiemap() to set FIEMAP_FLAG_SYNC. >=20 > That looks like a logically separate change, so it should probably be > a separate patch. >=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? My understanding, based on what coreutils learned: fiemap without FIEMAP_FLAG_SYNC is a data corrupter. fiemap _with_ FIEMAP_FLAG_SYNC is a performance killer (noticeably slower, because it forces a sync). SEEK_HOLE is a much simpler interface, and therefore the kernel can optimize it to return correct results faster than it can for fiemap, and it does not suffer from the fiemap on unsynced file data corruption. So coreutils _prefers_ seek_hole first, and when it has to use fiemap, prefers using fiemap with FIEMAP_FLAG_SYNC. I agree with splitting this into two patches; one to add the flag as a data corruption fix but acknowledging that it slows fiemap down, the other to relegate fiemap to the fallback case since seek_hole can be faster when it doesn't have to force a sync. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --7GVt7mPxHp95a7bkN3ja9iiRxKKaklDaB Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg iQEcBAEBCAAGBQJUJA5NAAoJEKeha0olJ0Nq8rcIAIBzNy1716XmAEUvhRZ54Ndt Jtj6ub53m0hZiInLt0XejwHgD1kp98A5iy41x+JoTmxoYak9k8ni4BDPJxFfUuno SSWPoVlE1ncZbXdUbQp5+zSiJseDJ7NUG6T4cG0nxK0tahdmCpvuy6xLVYUWKAbi vYtoBhRns6I3Degg2Jybhq0ah+aqaV1vmW+680AMfjd6pDRgwLxxwbDkZJuHrwzG Mdu+a0WT3g5nFyjyrg0ua50OBR4OhQZBSen+zJKE18IaAqPi3I8VwAViVy5b0lkx AbXFHC8BVQkdC+NeNjCN2T8PibN+RHimzDn0Rsp7a94WBYZmuhuCY4ur68w7NkY= =Cz2K -----END PGP SIGNATURE----- --7GVt7mPxHp95a7bkN3ja9iiRxKKaklDaB--