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

David Chinner
2008-02-23T05:00:05Z

[ 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