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 <t...@bakeyournoodle.com> 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 seek_hole >>> checks to determine if the sector is present in the disk image. > > Does this fix the problem or does it just make it less likely that it > becomes apparent? > > If there is a data corruptor, we need to fix it, not just ensure that > only the less common environments are affected. > >>> While we're there modify try_fiemap() to set FIEMAP_FLAG_SYNC. > > That looks like a logically separate change, so it should probably be > a separate patch. > > 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. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
