xfs
[Top] [All Lists]

Re: REVIEW XFSQA test out xfs_attr_shortform_bytesfit attr2/attr1 fix

To: David Chinner <dgc@xxxxxxx>
Subject: Re: REVIEW XFSQA test out xfs_attr_shortform_bytesfit attr2/attr1 fix
From: Timothy Shimmin <tes@xxxxxxx>
Date: Mon, 14 Apr 2008 13:56:46 +1000
Cc: "bnaujok@xxxxxxx via BugWorks" <bnaujok@xxxxxxx>, xfs-dev <xfs-dev@xxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>, asg-qa <asgqa@xxxxxxx>
In-reply-to: <20080414032229.GV103491721@sgi.com>
References: <47FC79FC.5020803@sgi.com> <20080410053601.GK108924158@sgi.com> <4802B4E8.3070807@sgi.com> <20080414032229.GV103491721@sgi.com>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.12 (Macintosh/20080213)
Hi Dave,

David Chinner wrote:
On Mon, Apr 14, 2008 at 11:35:36AM +1000, Timothy Shimmin wrote:
Hi Dave and Barry,

David Chinner wrote:
On Wed, Apr 09, 2008 at 06:10:36PM +1000, Timothy Shimmin wrote:
Hi there,

A test to test out Eric's fix for xfs_attr_shortform_bytesfit
bug when going from attr2 to attr1.

With TOT kernel, without patch, one can see the corrupted inline
dirents. With patch, all is well.

The 186.out _should_ be output'ing ATTR2 for the db version
command but I'm awaiting Barry's xfsprogs checkin to fix that one -
and then I will regenerate it.
Really? I'm seeing it fail with ATTR2 in the xfs_db output...

That's what I used to see until I updated to TOT kernel.
That's weird.

It's likely due to Eric's change to make sb_badfeatures2 = sb_features2. That will make userspace pick up the attr2 feature properly regardless of where it is. Perhaps xfs_db is b0rked w.r.t. reporting attr vs attr2....

Cheers,

Dave.

I'm not sure, but with Barry's latest checkins, all seems well and I'm now getting the attr2 output as expected.

Which just leaves the ATTR and ATTR2 output when I add an EA
to a file on an ATTR2 file-system.
It is naturally the kernel that is doing this.
The existing behaviour prior to ATTR2, was to
turn on ATTR as soon as one added an EA to a file (added by doucette).

xfs_bmap_add_attrfork
....
        if (!xfs_sb_version_hasattr(&mp->m_sb) ||
           (!xfs_sb_version_hasattr2(&mp->m_sb) && version == 2)) {
                __int64_t sbfields = 0;

                spin_lock(&mp->m_sb_lock);
                if (!xfs_sb_version_hasattr(&mp->m_sb)) {
                        xfs_sb_version_addattr(&mp->m_sb);
                        sbfields |= XFS_SB_VERSIONNUM;
                }
                if (!xfs_sb_version_hasattr2(&mp->m_sb) && version == 2) {
                        xfs_sb_version_addattr2(&mp->m_sb);
                        sbfields |= (XFS_SB_VERSIONNUM | XFS_SB_FEATURES2);
                }

So I would say it is intentional and we are preserving the behaviour of knowing if we have EAs in use or not. I guess it seems a bit redundant but it may well cause less problems in doing so and doesn't hurt.

--Tim


<Prev in Thread] Current Thread [Next in Thread>