From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa0-f41.google.com ([209.85.219.41]:62037 "EHLO mail-oa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750896Ab3B1CGN (ORCPT ); Wed, 27 Feb 2013 21:06:13 -0500 Received: by mail-oa0-f41.google.com with SMTP id i10so2706840oag.14 for ; Wed, 27 Feb 2013 18:06:13 -0800 (PST) Message-ID: <512EBB84.3020301@gmail.com> Date: Thu, 28 Feb 2013 10:05:56 +0800 From: Wang Sheng-Hui MIME-Version: 1.0 To: Dave Chinner CC: xfstests , linux-btrfs@vger.kernel.org, list.btrfs@jan-o-sch.net, sandeen@redhat.com Subject: Re: [PATCH] xfstests 276: fix error 'FIBMAP: Invalid argument' References: <512DB284.1090806@gmail.com> <20130227230443.GB5551@dastard> <512E9EAC.9050700@gmail.com> <20130228002532.GJ5551@dastard> In-Reply-To: <20130228002532.GJ5551@dastard> Content-Type: text/plain; charset=UTF-8; format=flowed Sender: linux-btrfs-owner@vger.kernel.org List-ID: 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 >>> >>> 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 --- 276 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 -- 1.7.10.4 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay.sgi.com (relay1.corp.sgi.com [137.38.102.111]) by oss.sgi.com (Postfix) with ESMTP id D26C87F63 for ; Wed, 27 Feb 2013 20:06:17 -0600 (CST) Received: from cuda.sgi.com (cuda3.sgi.com [192.48.176.15]) by relay1.corp.sgi.com (Postfix) with ESMTP id AF2728F8054 for ; Wed, 27 Feb 2013 18:06:14 -0800 (PST) Received: from mail-oa0-f44.google.com (mail-oa0-f44.google.com [209.85.219.44]) by cuda.sgi.com with ESMTP id dspKEb1sp9qyvS2R (version=TLSv1 cipher=RC4-SHA bits=128 verify=NO) for ; Wed, 27 Feb 2013 18:06:13 -0800 (PST) Received: by mail-oa0-f44.google.com with SMTP id h1so2704452oag.31 for ; Wed, 27 Feb 2013 18:06:13 -0800 (PST) Message-ID: <512EBB84.3020301@gmail.com> Date: Thu, 28 Feb 2013 10:05:56 +0800 From: Wang Sheng-Hui MIME-Version: 1.0 Subject: Re: [PATCH] xfstests 276: fix error 'FIBMAP: Invalid argument' References: <512DB284.1090806@gmail.com> <20130227230443.GB5551@dastard> <512E9EAC.9050700@gmail.com> <20130228002532.GJ5551@dastard> In-Reply-To: <20130228002532.GJ5551@dastard> List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: base64 Content-Type: text/plain; charset="utf-8"; Format="flowed" Errors-To: xfs-bounces@oss.sgi.com Sender: xfs-bounces@oss.sgi.com To: Dave Chinner Cc: sandeen@redhat.com, list.btrfs@jan-o-sch.net, linux-btrfs@vger.kernel.org, xfstests T24gMjAxM+W5tDAy5pyIMjjml6UgMDg6MjUsIERhdmUgQ2hpbm5lciB3cm90ZToKPiBPbiBUaHUs IEZlYiAyOCwgMjAxMyBhdCAwODowMjo1MkFNICswODAwLCBXYW5nIFNoZW5nLUh1aSB3cm90ZToK Pj4gT24gMjAxM+W5tDAy5pyIMjjml6UgMDc6MDQsIERhdmUgQ2hpbm5lciB3cm90ZToKPj4+IE9u IFdlZCwgRmViIDI3LCAyMDEzIGF0IDAzOjE1OjE2UE0gKzA4MDAsIFdhbmcgU2hlbmctSHVpIHdy b3RlOgo+Pj4+IEJ0cmZzIGRvZXNuJ3Qgc3VwcG9ydCBGSUVNQVBfRkxBR19YQVRUUiwgd2hpY2gg aXMgZW5hYmxlZCBieQo+Pj4+IC14IG9wdGlvbiBvZiBmaWxlZnJhZywgYW5kIHdpbGwgZmFpbCB3 aXRoCj4+Pj4gCSdGSUJNQVA6IEludmFsaWQgYXJndW1lbnQnCj4+Pj4gZm9yICdmaWxlZnJhZyAt dngnLiAnZmlsZWZyYWcgLXZ4JyBmYWlscyBvbiBidHJmcyB3aXRoCj4+Pj4gICAgICAnRklFTUFQ IGZhaWxlZCB3aXRoIHVuc3VwcG9ydGVkIGZsYWdzIDInCj4+Pj4gUmVtb3ZlIHRoZSAnLXgnIG9w dGlvbi4KPj4+Pgo+Pj4+IFNpZ25lZC1vZmYtYnk6IFdhbmcgU2hlbmctSHVpIDxzaGh1aXdAZ21h aWwuY29tPgo+Pj4KPj4+IEkgY2FuIHNlZSB0aGF0IHRoaXMgY2hhbmdlcyB3aGF0IGdldHMgZHVt cGVkIGludG8gdGhlICRzZXEuZnVsbAo+Pj4gZmlsZSwgYnV0IGl0IHNlZW1zIHRvIG1lIHRoYXQg YWxzbyBjaGFuZ2VzIHRoZSBleHRlbnQgbGlzdCByZXR1cm5lZAo+Pj4gdG8gdGhlIGNoZWNraW5n IGZ1bmN0aW9ucy4gU28gZWl0aGVyIHRoZSB0ZXN0IHByZXZpb3VzbHkgd29ya2VkIGFuZAo+Pj4g bm93IGl0IGZhaWxzIHdpdGggdGhpcyBjaGFuZ2UsIG9yIHRoZSB0ZXN0IG5ldmVyIHdvcmtlZCBh bmQgbm93IGl0Cj4+PiBkb2VzLCBvciBwZXJoYXBzIHNvbWV0aGluZyBlbHNlPwo+Pj4KPj4+IElP V3MsIEkgY2FuJ3QgdGVsbCB3aHkgeW91IHdhbnQgdG8gY2hhbmdlIHRoaXMgZnJvbSB0aGUgcGF0 Y2gKPj4+IGRlc2NyaXB0aW9uLCBoZW5jZSBJIGRvbid0IGtub3cgaWYgdGhlIG9yaWdpbmFsIGJl aGF2aW91ciB3YXMKPj4+IGludGVudGlvbmFsIG9yIG5vdC4gIENhbiB5b3Ugc2F5IGRlc2NyaWJl IHdoYXQgdGhlIG92ZXJhbGwgZWZmZWN0IG9mCj4+PiB0aGUgY2hhbmdlIGlzIGluIHRoZSBjb21t aXQgZGVzY3JpcHRpb24/Cj4+Cj4+IEhpIERhdmUsCj4+Cj4+IEkgcnVuIHhmc3Rlc3RzIGZvciBi dHJmcyBhZ2FpbnN0IFNMRVMxMVNQMiwgbm90IHVwc3RyZWFtIGtlcm5lbC4KPj4gSW4gdGhlIHNl cS5mdWxsLCBJIGNhbiBnZXQgdGhlIG1lc3NhZ2VzCj4+IAknRklFTUFQIGZhaWxlZCB3aXRoIHVu c3VwcG9ydGVkIGZsYWdzIDInCj4+Cj4+IFRoZW4gSSBmb3VuZCB0aGF0IHRoZSB0ZXN0IHdpbGwg cnVuICdmaWxlZnJhZyAtdngnIG9uIGJ0cmZzLCBhbmQKPj4gJy12JyB3aWxsIHJ1biBGSUVNQVBf RkxBR19YQVRUUiwgd2hpY2ggaXMgbm90IHN1cHBvcnRlZCBieSBidHJmcwo+PiB5ZXQsIGF0IGxl YXN0IGluIDMuOCBrZXJuZWwuCj4KPiBTdXJlLCBJIHVuZGVyc3RhbmQgdGhhdC4gSSdtIG5vdCBh c2tpbmcgYWJvdXQgaG93IGl0IGZhaWxzIC0gdGhhdAo+IG11Y2ggaXMgb2J2aW91cy4gV2hhdCBJ IGFtIGFza2luZyBpczoKPgo+IAktIGRpZCBpdCBldmVyIHdvcms/Cj4gCS0gaWYgaXQgZGlkLCB3 aHkgZGlkIGl0IHN0YXJ0IGZhaWxpbmc/Cj4gCS0gaG93IGxvbmcgaGFzIGl0IGJlZW4gYnJva2Vu IGZvcj8KPgo+PiBXaXRoIHRoZSBwYXRjaCwgSSBjYW4gcGFzcyB0aGUgdGVzdGNhc2U6Cj4+ID09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQo+PiAyNzYgOHMgLi4u IDdzCj4+IFJhbjogMjc2Cj4+IFBhc3NlZCBhbGwgMSB0ZXN0cwo+Cj4gR3JlYXQsIGJ1dCB3aHkg ZGlkIGl0IGJyZWFrIGluIHRoZSBmaXJzdCBwbGFjZT8gVGhhdCdzIHRoZQo+IGluZm9ybWF0aW9u IHRoYXQgbmVlZHMgdG8gYmUgaW4gdGhlIGNvbW1pdCBtZXNzYWdlLiBJbmRlZWQsIEVyaWMgaGFz Cj4gcG9pbnRlZCBvdXQgdG8gdXMgKmV4YWN0bHkqIHRoZSBpbmZvcm1hdGlvbiB0aGF0IHNob3Vs IGRiZSBpbiB0aGUKPiBjb21taXQgbWVzc2FnZS4gaS5lLiBhIGRlc2NyaXB0aW9uIGFsb25nIHRo ZSBsaW5lcyBvZjoKPgo+ICJDb21taXQgMDVkYWRjMCAoIkJ0cmZzOiBhZGQgZmllbWFwJ3MgZmxh ZyBjaGVjayIpIGFkZGVkIHZhbGlkaXR5Cj4gY2hlY2tzIHRvIHRoZSBmaWVtYXAgZmxhZ3MgYW5k IGhlbmNlIGludmFsaWQgZmxhZ3MgYXJlIG5vdyBiZWluZwo+IHJlamVjdGVkLiBUZXN0IDI3NiBw YXNzZXMgYW4gaW52YWxpZCBmaWVtYXAgZmxhZyB0byBidHJmcywgYW5kIHNvCj4gdGhhdCB0ZXN0 IGZhaWxzIHNpbmNlIHRoaXMgY29tbWl0IHdhcyBhZGRlZC4iCj4KPiBUaGF0IHRlbGxzIHVzIGV4 YWN0bHkgd2h5IHRoZSB0ZXN0IGZhaWxlZCwgd2h5IHRoZSBjaGFuZ2UgaXMKPiBuZWNlc3Nhcnks IGFuZCBob3cgbG9uZyB0aGUgdGVzdCBoYXMgYmVlbiBicm9rZW4gZm9yLiBTZWVuIGFub3RoZXIK PiB3YXksIGl0IG1ha2VzIG1lIHdvbmRlciBob3cgb2Z0ZW4gYW55b25lIHJ1bnMgeGZzdGVzdHMg b24gYSBidHJmcwo+IGRldiB0cmVlIG9yIHJlZ3Jlc3Npb24gdGVzdHMgdGhlaXIgb3duIGNoYW5n ZXMuCj4KPiBDaGVlcnMsCj4KPiBEYXZlLgo+CgpNb2RpZnkgdGhlIGNvbW1pdCBtZXNzYWdlLgot LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tCgpQZXIgRXJpYywgQ29tbWl0IDA1ZGFkYzAgKCJCdHJmczogYWRkIGZpZW1hcCdzIGZs YWcgY2hlY2siKSBhZGRlZAp2YWxpZGl0eSBjaGVja3MgdG8gdGhlIGZpZW1hcCBmbGFncyBhbmQg aGVuY2UgaW52YWxpZCBmbGFncyBhcmUgbm93CmJlaW5nIHJlamVjdGVkLiBUZXN0IDI3NiBwYXNz ZXMgYW4gaW52YWxpZCBmaWVtYXAgZmxhZyB0byBidHJmcywgYW5kCnNvIHRoYXQgdGVzdCBmYWls cyBzaW5jZSB0aGlzIGNvbW1pdCB3YXMgYWRkZWQuCgpCdHJmcyBkb2Vzbid0IHN1cHBvcnQgRklF TUFQX0ZMQUdfWEFUVFIsIHdoaWNoIGlzIGVuYWJsZWQgYnkgLXggb3B0aW9uCm9mIGZpbGVmcmFn LCBhbmQgd2lsbCBmYWlsIHdpdGgKCSdGSUJNQVA6IEludmFsaWQgYXJndW1lbnQnCmZvciAnZmls ZWZyYWcgLXZ4Jy4gJ2ZpbGVmcmFnIC12eCcgZmFpbHMgb24gYnRyZnMgd2l0aAogICAgICdGSUVN QVAgZmFpbGVkIHdpdGggdW5zdXBwb3J0ZWQgZmxhZ3MgMicKUmVtb3ZlIHRoZSAnLXgnIG9wdGlv bi4KClNpZ25lZC1vZmYtYnk6IFdhbmcgU2hlbmctSHVpIDxzaGh1aXdAZ21haWwuY29tPgotLS0K ICAyNzYgfCAgICAyICstCiAgMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0aW9uKCspLCAxIGRlbGV0 aW9uKC0pCgpkaWZmIC0tZ2l0IGEvMjc2IGIvMjc2CmluZGV4IDA4MmY5NDMuLmYyYjE3YzkgMTAw NzU1Ci0tLSBhLzI3NgorKysgYi8yNzYKQEAgLTc1LDcgKzc1LDcgQEAgX2ZpbHRlcl9leHRlbnRz KCkKCiAgX2NoZWNrX2ZpbGVfZXh0ZW50cygpCiAgewotCWNtZD0iZmlsZWZyYWcgLXZ4ICQxIgor CWNtZD0iZmlsZWZyYWcgLXYgJDEiCiAgCWVjaG8gIiMgJGNtZCIgPj4gJHNlcS5mdWxsCiAgCW91 dD1gJGNtZCB8IF9maWx0ZXJfZXh0ZW50c2AKICAJaWYgWyAteiAiJG91dCIgXTsgdGhlbgotLSAK MS43LjEwLjQKCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f Xwp4ZnMgbWFpbGluZyBsaXN0Cnhmc0Bvc3Muc2dpLmNvbQpodHRwOi8vb3NzLnNnaS5jb20vbWFp bG1hbi9saXN0aW5mby94ZnMK