On Fri, 28 Mar 2008 10:57:39 +1100, David Chinner <dgc@xxxxxxx> wrote:
On Thu, Mar 27, 2008 at 04:25:33PM +1100, Barry Naujok wrote:
Running XFS QA with a standard HDD with the bad_features2 problem
happening and doing "mkfs.xfs -l version=1", a problem was encounter
where it went to zero the "bad" features2 bit.
Why didn't this happen all the time?
Upon investigation, I updated the behaviour of the "secondary_sb_wack"
function. Now it always zeroes any garbage found beyond the expected
end of the xfs_sb_t structure in the first sector.
.....
- if (XFS_SB_VERSION_HASMOREBITS(sb))
- size = (__psint_t)&sb->sb_features2
- + sizeof(sb->sb_features2) - (__psint_t)sb;
- else if (XFS_SB_VERSION_HASLOGV2(sb))
- size = (__psint_t)&sb->sb_logsunit
+ if (xfs_sb_version_hasmorebits(sb))
+ size = (__psint_t)&sb->sb_bad_features2
+ + sizeof(sb->sb_bad_features2) - (__psint_t)sb;
+ else if (xfs_sb_version_haslogv2(sb))
.....
This is still fragile and requires us to update this function every time
we add a new field to the superblock. Is there some way we can do this
that doesn't require an update for every modification to the superblock
we make?
It sort of seems that the zeroing code of old was only for the early
filesystems before a lot of these features and that later features
were explicitly added and checked:
ie.
if (!xfs_sb_hasfeature(sb) && sb->feature is set), zero it.
Maybe the "mask" (XR_GOOD_SECSB_VNMASK) that was used should be
expanded to the full range of sb_versionnum (0xff00).
If extra features are set that xfs_repair doesn't handle, it doesn't
even get this far to try and zero beyond xfs_sb_t. That does seem to
be the solution (but still requires code for each and every single
new feature added to xfs_sb_t).
So, for CI mode for example,
if (!xfs_sb_hasunicode(sb) && sb->sb_cftino != 0) {
/* handle it */
}
Also -
size = offsetof(sb->sb_bad_features2) +
sizeof(sb->sb_bad_features2);
or in the case of the last entry in the superblock:
size = sizeof(xfs_dsb_t);
Cheers,
Dave.
|