On 3/18/11 8:48 AM, Chris Mason wrote: ...
> Btrfs before 2.6.38 may have real trouble though, even with the sync. > We were returning overlapping ranges to you, so the destination would > end up bigger than the original. This could be fixed in cp by making > sure to never seek backwards based on the extents returned. > > Example bad results: > > logical: [ 0 ... 1MB ] -> physical [ foo .. foo ] > logical [ 0 ... 2MB ] -> physical [ foo2 .. foo2 ] > > 100% a btrfs bug, but cp would do the 0 .. 1MB bit and then seek back to > zero and do the 0 .. 2MB bit. If cp had not seeked backwards you would > have covered for me. 2.6.38 final fixed this problem. > > I think even XFS was fixed relatively recently, 2.6.36 and later. > Looking at the commit, the simple dd test above wouldn't have triggered > it. Actually, looking at this commit even the sync wouldn't save xfs > before 2.6.36, Eric am I reading this right? Actually I don't think sync helps that either :) But as long as coreutils is looking for the "last extent" flag, I think it works out ok. Without that fix, I -think- we just returned fewer extents than were requested, but without the last/EOF flag returned, so hopefully the application keeps asking for more. I recently found another similar behavior that still exists... and fixed up a tool in xfstests to handle it: http://git.kernel.org/?p=fs/xfs/xfstests-dev.git;a=commitdiff;h=e423b5c584300c738cee95badc13b01bf38c5dc8 I wonder how bad sync will be, in practice. Thinking about my own usecases, I doubt I point "cp" at modified-but-not-written source (source vs. dest I mean) files very often... The testcase in coreutils should definitely remove the fdatasync hack, as that just makes the test pass without worrying about things in the real world. :) A better test suite might be to generate random holey / delalloc files and copy those around. We should write an xfstests test to do the same thing, if we can determine whether we have a "cp" that knows about fiemap? -Eric > So maybe just give in and look for 2.6.38 instead of trying to remember > all the places where individual filesystems didn't suck. Give the user > a cp --sparse=fiemap-im-really-really-sure to override your assumptions > about our bugs. > > commit 9af25465081480a75824fd7a16a37a5cfebeede9 > Author: Tao Ma <tao...@oracle.com> > Date: Mon Aug 30 02:44:03 2010 +0000 > > xfs: Make fiemap work with sparse files > > In xfs_vn_fiemap, we set bvm_count to fi_extent_max + 1 and want > to return fi_extent_max extents, but actually it won't work for > a sparse file. The reason is that in xfs_getbmap we will > calculate holes and set it in 'out', while out is malloced by > bmv_count(fi_extent_max+1) which didn't consider holes. So in the > worst case, if 'out' vector looks like > [hole, extent, hole, extent, hole, ... hole, extent, hole], > we will only return half of fi_extent_max extents. > > This patch add a new parameter BMV_IF_NO_HOLES for bvm_iflags. > So with this flags, we don't use our 'out' in xfs_getbmap for > a hole. The solution is a bit ugly by just don't increasing > index of 'out' vector. I felt that it is not easy to skip it > at the very beginning since we have the complicated check and > some function like xfs_getbmapx_fix_eof_hole to adjust 'out'. > > Cc: Dave Chinner <da...@fromorbit.com> > Signed-off-by: Tao Ma <tao...@oracle.com> > Signed-off-by: Alex Elder <ael...@sgi.com> > > -chris