Re: [Qemu-devel] [PATCH] block/raw-posix: use seek_hole ahead of fiemap

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2072 bytes --]

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 <tony@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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]