xfs
[Top] [All Lists]

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

To: sandeen@xxxxxxxxxxx
Subject: Re: [PATCH] (and bad attr2 bug) - pack xfs_sb_t for 64-bit arches
From: David Chinner <dgc@xxxxxxx>
Date: Fri, 17 Nov 2006 17:58:10 +1100
Cc: David Chinner <dgc@xxxxxxx>, Timothy Shimmin <tes@xxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <52841.10.0.0.2.1163745285.squirrel@xxxxxxxxxxx>
References: <455CB54F.8080901@xxxxxxxxxxx> <BB70F203E29C2D37A2F727C8@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20061117023946.GN11034@xxxxxxxxxxxxxxxxx> <CEB981736A0E8C7DF9ABD7C8@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20061117055521.GS11034@xxxxxxxxxxxxxxxxx> <52841.10.0.0.2.1163745285.squirrel@xxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.4.2.1i
On Fri, Nov 17, 2006 at 12:34:45AM -0600, sandeen@xxxxxxxxxxx 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


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