xfstests 276: fix error 'FIBMAP: Invalid argument'

Commit Message

Wang Sheng-Hui Feb. 28, 2013, 2:05 a.m. UTC

On 2013?02?28? 08:25, Dave Chinner wrote:
> On Thu, Feb 28, 2013 at 08:02:52AM +0800, Wang Sheng-Hui wrote:
>> On 2013?02?28? 07:04, Dave Chinner wrote:
>>> On Wed, Feb 27, 2013 at 03:15:16PM +0800, Wang Sheng-Hui wrote:
>>>> Btrfs doesn't support FIEMAP_FLAG_XATTR, which is enabled by
>>>> -x option of filefrag, and will fail with
>>>> 	'FIBMAP: Invalid argument'
>>>> for 'filefrag -vx'. 'filefrag -vx' fails on btrfs with
>>>>      'FIEMAP failed with unsupported flags 2'
>>>> Remove the '-x' option.
>>>>
>>>> Signed-off-by: Wang Sheng-Hui <[e-mail]>
>>>
>>> I can see that this changes what gets dumped into the $seq.full
>>> file, but it seems to me that also changes the extent list returned
>>> to the checking functions. So either the test previously worked and
>>> now it fails with this change, or the test never worked and now it
>>> does, or perhaps something else?
>>>
>>> IOWs, I can't tell why you want to change this from the patch
>>> description, hence I don't know if the original behaviour was
>>> intentional or not.  Can you say describe what the overall effect of
>>> the change is in the commit description?
>>
>> Hi Dave,
>>
>> I run xfstests for btrfs against SLES11SP2, not upstream kernel.
>> In the seq.full, I can get the messages
>> 	'FIEMAP failed with unsupported flags 2'
>>
>> Then I found that the test will run 'filefrag -vx' on btrfs, and
>> '-v' will run FIEMAP_FLAG_XATTR, which is not supported by btrfs
>> yet, at least in 3.8 kernel.
>
> Sure, I understand that. I'm not asking about how it fails - that
> much is obvious. What I am asking is:
>
> 	- did it ever work?
> 	- if it did, why did it start failing?
> 	- how long has it been broken for?
>
>> With the patch, I can pass the testcase:
>> =============================================
>> 276 8s ... 7s
>> Ran: 276
>> Passed all 1 tests
>
> Great, but why did it break in the first place? That's the
> information that needs to be in the commit message. Indeed, Eric has
> pointed out to us *exactly* the information that shoul dbe in the
> commit message. i.e. a description along the lines of:
>
> "Commit 05dadc0 ("Btrfs: add fiemap's flag check") added validity
> checks to the fiemap flags and hence invalid flags are now being
> rejected. Test 276 passes an invalid fiemap flag to btrfs, and so
> that test fails since this commit was added."
>
> That tells us exactly why the test failed, why the change is
> necessary, and how long the test has been broken for. Seen another
> way, it makes me wonder how often anyone runs xfstests on a btrfs
> dev tree or regression tests their own changes.
>
> Cheers,
>
> Dave.
>

Modify the commit message.
----------------------------------------------------------------

Per Eric, Commit 05dadc0 ("Btrfs: add fiemap's flag check") added
validity checks to the fiemap flags and hence invalid flags are now
being rejected. Test 276 passes an invalid fiemap flag to btrfs, and
so that test fails since this commit was added.

Btrfs doesn't support FIEMAP_FLAG_XATTR, which is enabled by -x option
of filefrag, and will fail with
	'FIBMAP: Invalid argument'
for 'filefrag -vx'. 'filefrag -vx' fails on btrfs with
     'FIEMAP failed with unsupported flags 2'
Remove the '-x' option.

Signed-off-by: Wang Sheng-Hui <[e-mail]>
---
  276 |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Patch

diff --git a/276 b/276
index 082f943..f2b17c9 100755
--- a/276
+++ b/276
@@ -75,7 +75,7 @@  _filter_extents()

  _check_file_extents()
  {
-	cmd="filefrag -vx $1"
+	cmd="filefrag -v $1"
  	echo "# $cmd" >> $seq.full
  	out=`$cmd | _filter_extents`
  	if [ -z "$out" ]; then