Re: [REVIEW] User-space support for bad_features2 patch

[David Chinner <dgc@sgi.com>]

[ please inline patches so they are easy to quote for review. ]

On Fri, Feb 22, 2008 at 06:52:43PM +1100, Barry Naujok wrote:
> The attached patch fixes mkfs.xfs writing the bad features2 in the first  
> place (the change to xfs_sb.h does this).
> 
> Next xfs_db support printing of this superblock field and xfs_check can  
> report the bad_features2 field is set.
> 
> xfs_repair can correct the error in the same fashion that David Chinner's  
> mount code does it.

Actually, it doesn't:

        /*
+        * Check bad_features2, if set and features2 is zero, copy
+        * bad_features2 to features2 and zero bad_features2.
+        */
+       if (sb->sb_bad_features2 != 0) {
+               if (sb->sb_features2 == 0)
+                       sb->sb_features2 = sb->sb_bad_features2;

This simply copies the bad features over the features field if
the sb_features2 field is zero. This ignores the fact that we may
have set something into the sb_features2 field before detecting the
problem (e.g. attr2 can be turned on dynamically).  The patch I posted
OR'd the two fields together to ensure no feature bits were lost.
This needs to be done here as well.

Cheers,

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