On Thu, Dec 07, 2017 at 09:16:03AM +1100, Dave Chinner wrote: > On Wed, Dec 06, 2017 at 04:01:58PM -0600, Eric Sandeen wrote: > > On 12/6/17 3:57 PM, Dave Chinner wrote: > > > > > There's a *simple answer* to this problem: fix the new command's > > > output. > > > > > > That is: the user asked for a specific range, so the command itself > > > should trim the map returned by the kernel to only display the exact > > > range the user asked for. Then it doesn't matter if the underlying > > > filesystem trims the extents or not, because the we're going to do > > > that anyway in userspace. > > > > I have a different opinion: > > > > xfs_io is a debugging tool; the fiemap command sends an ioctl to the kernel. > > > > Ranged fiemap queries are a real thing; you put numbers into the kernel, > > and you get numbers out of the kernel. > > > > IMNSO, xfs_io should present to the user /what the kernel returned/, > > and not re-interpret it to fit some other notion of correctness if we > > don't like what the kernel told us. > > I hardly think "trimming to the range the user asked for" is > "re-interpreting what the kernel told us". It's limiting output > range to exactly what the user asked for - the output is still > correct regardless of how it's filtered to match what the user asked > for.... ...except that xfs_io is a tool we use to debug the garbage that FIEMAP sprays back at userspace, so it absolutely should not be hiding things like this from the test suite: $ xfs_io -c "fiemap -va 0k 65k" moofile moo: EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS 0: [0..18446744073709551615]: 1585456720..1585456719 0 0x301 1: [0..129]: hole 130 (Yay ext4. WTF does the extent have length 0?) So.... historically XFS trimmed the FIEMAP results. Fix the test to reflect the XFS behavior, get ext4 (and btrfs) to fix their weird implementations, and leave xfs_io alone. (Don't mind me shifting positions. :P) --D > > If you want to have some user-friendlier behavior where xfs_io layers > > behaviors on top of what the kernel provides, then add a "-t" argument for trim, > > but hiding ioctl inconsistencies by filtering them through xfs_io sounds > > like the wrong approach to me. > > Just filter the last output in the test, then, so it looks like > > 2: [128..XXX] data > > There is absolutely no excuse for creating multiple tests to support > a small difference in trailing extent range output from different > filesystem. > > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
