Re: [PATCH] (and bad attr2 bug) - pack xfs_sb_t for 64-bit arches

[David Chinner <dgc@sgi.com>]

On Fri, Nov 17, 2006 at 12:34:45AM -0600, sandeen@sandeen.net wrote:
> > On Fri, Nov 17, 2006 at 02:11:12PM +1000, Timothy Shimmin wrote:
> 
> > Whenever you add to the table, you now need to modify both the new
> > entry and the terminator to get it right.
> >
> > Nor (IMO) is it obvious that it is a terminator or why it is
> > different to all the other entries in the structure. A field such as
> > sb_dummy or sb_pad before the terminator is fairly obvious, and it
> > means that you don't need to modify the table terminator every time
> > the superblock gets extended.
> >
> > That way the code stays more consistent over time, diffs are smaller
> > and neater, and you can see at a simple diff just how the features
> > have been added over time (like I did this morning).....
> 
> nothing in the code is terribly obvious.. please add comments however you
> decide to fix it :)

*nod*

> and really, now that this is out in the wild, maybe sb_features3 instead
> of padding is appropriate, and check both for the attr2 bit...? :(

I'm not sure that this is a good idea, especially as past history of
introducing new feature bits is anything to go by (I think this makes bug #6
that the features2 field has been responsible for). I'd much prefer to
fix the bug, blacklist the bad 4 bytes in the superblock, and then either:

	- modify xfs_admin/repair to detect a busted superblock and have them
	  fix it; or
	- put code in the mount path that detects this and corrects it
	  automatically (which we do for some other superblock fields).

> i'm trying to figure out what the kernel upgrade path is for fc6 users who
> have an extra-padded-flipped features2/attr2 filesystem.  :(

Depends on what we do to fix it, right? Do you have any preferences?

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group