From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ipmail05.adl6.internode.on.net ([150.101.137.143]:1180 "EHLO ipmail05.adl6.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752647Ab3B1AZh (ORCPT ); Wed, 27 Feb 2013 19:25:37 -0500 Date: Thu, 28 Feb 2013 11:25:32 +1100 From: Dave Chinner To: Wang Sheng-Hui Cc: xfstests , linux-btrfs@vger.kernel.org, list.btrfs@jan-o-sch.net Subject: Re: [PATCH] xfstests 276: fix error 'FIBMAP: Invalid argument' Message-ID: <20130228002532.GJ5551@dastard> References: <512DB284.1090806@gmail.com> <20130227230443.GB5551@dastard> <512E9EAC.9050700@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 In-Reply-To: <512E9EAC.9050700@gmail.com> Sender: linux-btrfs-owner@vger.kernel.org List-ID: 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. -- Dave Chinner david@fromorbit.com From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay.sgi.com (relay2.corp.sgi.com [137.38.102.29]) by oss.sgi.com (Postfix) with ESMTP id 723D07F5D for ; Wed, 27 Feb 2013 18:25:39 -0600 (CST) Received: from cuda.sgi.com (cuda3.sgi.com [192.48.176.15]) by relay2.corp.sgi.com (Postfix) with ESMTP id 60574304051 for ; Wed, 27 Feb 2013 16:25:36 -0800 (PST) Received: from ipmail05.adl6.internode.on.net (ipmail05.adl6.internode.on.net [150.101.137.143]) by cuda.sgi.com with ESMTP id 0aehtPKOuwAaJEQI for ; Wed, 27 Feb 2013 16:25:34 -0800 (PST) Date: Thu, 28 Feb 2013 11:25:32 +1100 From: Dave Chinner Subject: Re: [PATCH] xfstests 276: fix error 'FIBMAP: Invalid argument' Message-ID: <20130228002532.GJ5551@dastard> References: <512DB284.1090806@gmail.com> <20130227230443.GB5551@dastard> <512E9EAC.9050700@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <512E9EAC.9050700@gmail.com> List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: xfs-bounces@oss.sgi.com Sender: xfs-bounces@oss.sgi.com To: Wang Sheng-Hui Cc: list.btrfs@jan-o-sch.net, linux-btrfs@vger.kernel.org, xfstests T24gVGh1LCBGZWIgMjgsIDIwMTMgYXQgMDg6MDI6NTJBTSArMDgwMCwgV2FuZyBTaGVuZy1IdWkg d3JvdGU6Cj4gT24gMjAxM+W5tDAy5pyIMjjml6UgMDc6MDQsIERhdmUgQ2hpbm5lciB3cm90ZToK PiA+T24gV2VkLCBGZWIgMjcsIDIwMTMgYXQgMDM6MTU6MTZQTSArMDgwMCwgV2FuZyBTaGVuZy1I dWkgd3JvdGU6Cj4gPj5CdHJmcyBkb2Vzbid0IHN1cHBvcnQgRklFTUFQX0ZMQUdfWEFUVFIsIHdo aWNoIGlzIGVuYWJsZWQgYnkKPiA+Pi14IG9wdGlvbiBvZiBmaWxlZnJhZywgYW5kIHdpbGwgZmFp bCB3aXRoCj4gPj4JJ0ZJQk1BUDogSW52YWxpZCBhcmd1bWVudCcKPiA+PmZvciAnZmlsZWZyYWcg LXZ4Jy4gJ2ZpbGVmcmFnIC12eCcgZmFpbHMgb24gYnRyZnMgd2l0aAo+ID4+ICAgICAnRklFTUFQ IGZhaWxlZCB3aXRoIHVuc3VwcG9ydGVkIGZsYWdzIDInCj4gPj5SZW1vdmUgdGhlICcteCcgb3B0 aW9uLgo+ID4+Cj4gPj5TaWduZWQtb2ZmLWJ5OiBXYW5nIFNoZW5nLUh1aSA8c2hodWl3QGdtYWls LmNvbT4KPiA+Cj4gPkkgY2FuIHNlZSB0aGF0IHRoaXMgY2hhbmdlcyB3aGF0IGdldHMgZHVtcGVk IGludG8gdGhlICRzZXEuZnVsbAo+ID5maWxlLCBidXQgaXQgc2VlbXMgdG8gbWUgdGhhdCBhbHNv IGNoYW5nZXMgdGhlIGV4dGVudCBsaXN0IHJldHVybmVkCj4gPnRvIHRoZSBjaGVja2luZyBmdW5j dGlvbnMuIFNvIGVpdGhlciB0aGUgdGVzdCBwcmV2aW91c2x5IHdvcmtlZCBhbmQKPiA+bm93IGl0 IGZhaWxzIHdpdGggdGhpcyBjaGFuZ2UsIG9yIHRoZSB0ZXN0IG5ldmVyIHdvcmtlZCBhbmQgbm93 IGl0Cj4gPmRvZXMsIG9yIHBlcmhhcHMgc29tZXRoaW5nIGVsc2U/Cj4gPgo+ID5JT1dzLCBJIGNh bid0IHRlbGwgd2h5IHlvdSB3YW50IHRvIGNoYW5nZSB0aGlzIGZyb20gdGhlIHBhdGNoCj4gPmRl c2NyaXB0aW9uLCBoZW5jZSBJIGRvbid0IGtub3cgaWYgdGhlIG9yaWdpbmFsIGJlaGF2aW91ciB3 YXMKPiA+aW50ZW50aW9uYWwgb3Igbm90LiAgQ2FuIHlvdSBzYXkgZGVzY3JpYmUgd2hhdCB0aGUg b3ZlcmFsbCBlZmZlY3Qgb2YKPiA+dGhlIGNoYW5nZSBpcyBpbiB0aGUgY29tbWl0IGRlc2NyaXB0 aW9uPwo+IAo+IEhpIERhdmUsCj4gCj4gSSBydW4geGZzdGVzdHMgZm9yIGJ0cmZzIGFnYWluc3Qg U0xFUzExU1AyLCBub3QgdXBzdHJlYW0ga2VybmVsLgo+IEluIHRoZSBzZXEuZnVsbCwgSSBjYW4g Z2V0IHRoZSBtZXNzYWdlcwo+IAknRklFTUFQIGZhaWxlZCB3aXRoIHVuc3VwcG9ydGVkIGZsYWdz IDInCj4gCj4gVGhlbiBJIGZvdW5kIHRoYXQgdGhlIHRlc3Qgd2lsbCBydW4gJ2ZpbGVmcmFnIC12 eCcgb24gYnRyZnMsIGFuZAo+ICctdicgd2lsbCBydW4gRklFTUFQX0ZMQUdfWEFUVFIsIHdoaWNo IGlzIG5vdCBzdXBwb3J0ZWQgYnkgYnRyZnMKPiB5ZXQsIGF0IGxlYXN0IGluIDMuOCBrZXJuZWwu CgpTdXJlLCBJIHVuZGVyc3RhbmQgdGhhdC4gSSdtIG5vdCBhc2tpbmcgYWJvdXQgaG93IGl0IGZh aWxzIC0gdGhhdAptdWNoIGlzIG9idmlvdXMuIFdoYXQgSSBhbSBhc2tpbmcgaXM6CgoJLSBkaWQg aXQgZXZlciB3b3JrPwoJLSBpZiBpdCBkaWQsIHdoeSBkaWQgaXQgc3RhcnQgZmFpbGluZz8KCS0g aG93IGxvbmcgaGFzIGl0IGJlZW4gYnJva2VuIGZvcj8KCj4gV2l0aCB0aGUgcGF0Y2gsIEkgY2Fu IHBhc3MgdGhlIHRlc3RjYXNlOgo+ID09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQo+IDI3NiA4cyAuLi4gN3MKPiBSYW46IDI3Ngo+IFBhc3NlZCBhbGwgMSB0ZXN0 cwoKR3JlYXQsIGJ1dCB3aHkgZGlkIGl0IGJyZWFrIGluIHRoZSBmaXJzdCBwbGFjZT8gVGhhdCdz IHRoZQppbmZvcm1hdGlvbiB0aGF0IG5lZWRzIHRvIGJlIGluIHRoZSBjb21taXQgbWVzc2FnZS4g SW5kZWVkLCBFcmljIGhhcwpwb2ludGVkIG91dCB0byB1cyAqZXhhY3RseSogdGhlIGluZm9ybWF0 aW9uIHRoYXQgc2hvdWwgZGJlIGluIHRoZQpjb21taXQgbWVzc2FnZS4gaS5lLiBhIGRlc2NyaXB0 aW9uIGFsb25nIHRoZSBsaW5lcyBvZjoKCiJDb21taXQgMDVkYWRjMCAoIkJ0cmZzOiBhZGQgZmll bWFwJ3MgZmxhZyBjaGVjayIpIGFkZGVkIHZhbGlkaXR5CmNoZWNrcyB0byB0aGUgZmllbWFwIGZs YWdzIGFuZCBoZW5jZSBpbnZhbGlkIGZsYWdzIGFyZSBub3cgYmVpbmcKcmVqZWN0ZWQuIFRlc3Qg Mjc2IHBhc3NlcyBhbiBpbnZhbGlkIGZpZW1hcCBmbGFnIHRvIGJ0cmZzLCBhbmQgc28KdGhhdCB0 ZXN0IGZhaWxzIHNpbmNlIHRoaXMgY29tbWl0IHdhcyBhZGRlZC4iCgpUaGF0IHRlbGxzIHVzIGV4 YWN0bHkgd2h5IHRoZSB0ZXN0IGZhaWxlZCwgd2h5IHRoZSBjaGFuZ2UgaXMKbmVjZXNzYXJ5LCBh bmQgaG93IGxvbmcgdGhlIHRlc3QgaGFzIGJlZW4gYnJva2VuIGZvci4gU2VlbiBhbm90aGVyCndh eSwgaXQgbWFrZXMgbWUgd29uZGVyIGhvdyBvZnRlbiBhbnlvbmUgcnVucyB4ZnN0ZXN0cyBvbiBh IGJ0cmZzCmRldiB0cmVlIG9yIHJlZ3Jlc3Npb24gdGVzdHMgdGhlaXIgb3duIGNoYW5nZXMuCgpD aGVlcnMsCgpEYXZlLgotLSAKRGF2ZSBDaGlubmVyCmRhdmlkQGZyb21vcmJpdC5jb20KCl9fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCnhmcyBtYWlsaW5nIGxp c3QKeGZzQG9zcy5zZ2kuY29tCmh0dHA6Ly9vc3Muc2dpLmNvbS9tYWlsbWFuL2xpc3RpbmZvL3hm cwo=