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

[Pádraig Brady]

On 09/25/2014 10:00 AM, Tony Breeds wrote:
> On Thu, Sep 25, 2014 at 09:30:30AM +0200, Kevin Wolf wrote:
> 
>> Does this fix the problem or does it just make it less likely that it
>> becomes apparent?
> 
> Sorry for not making this clearer in my commit message.
> 
> I haven't been able to reproduce the corruption with the fiemap flag
> change.
>  
>> If there is a data corruptor, we need to fix it, not just ensure that
>> only the less common environments are affected.
> 
> 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.
>  
>> That looks like a logically separate change, so it should probably be
>> a separate patch.
> 
> Sure I can do that, and be more explicit about the reason in the commit
> message.
>  
>> 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?

syncing a file has performance side effects,
so best go try the (more portable) seek hole interface.

> The preference for seek_hole was a suggestion from Pádraig Brady , so
> I'll let him defend that :) but as I said above I think it was about 
> reducing the situations where fiemap was/is called.

IMHO it's a kernel bug that fiemap() needs a sync to be useful.
This was being fixed up in various file systems but Dave Chiner
pushed back on XFS adjustments, and thus changes were stalled.
The current situation is it's best to avoid fiemap().

thanks,
Pádraig.