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.
Further down in discrete field checking, there were a lot of
" if (sb->sb_versionnum & XR_PART_SECSB_VNMASK || !do_bzero) { "
checks which seems superfluous for the tests and operations
being performed.
The following patch relies on the bad_features2 patch from the
other week.
--
--- ci.orig/xfsprogs/repair/agheader.c
+++ ci/xfsprogs/repair/agheader.c
@@ -213,82 +213,66 @@ compare_sb(xfs_mount_t *mp, xfs_sb_t *sb
*
* And everything else in the buffer beyond either sb_width,
* sb_dirblklog (v2 dirs), or sb_logsectsize can be zeroed.
- *
- * Note: contrary to the name, this routine is called for all
- * superblocks, not just the secondary superblocks.
*/
-int
-secondary_sb_wack(xfs_mount_t *mp, xfs_buf_t *sbuf, xfs_sb_t *sb,
- xfs_agnumber_t i)
+static int
+sb_whack(
+ xfs_mount_t *mp,
+ xfs_sb_t *sb, /* translated superblock */
+ xfs_buf_t *sbuf, /* disk buffer with superblock */
+ xfs_agnumber_t agno)
{
- int do_bzero;
- int size;
- char *ip;
- int rval;
-
- rval = do_bzero = 0;
+ int rval = 0;
+ int do_zero = 0;
+ int size;
+ char *ip;
/*
- * mkfs's that stamped a feature bit besides the ones in the mask
- * (e.g. were pre-6.5 beta) could leave garbage in the secondary
- * superblock sectors. Anything stamping the shared fs bit or better
- * into the secondaries is ok and should generate clean secondary
- * superblock sectors. so only run the bzero check on the
- * potentially garbaged secondaries.
+ * Check for garbage beyond the last field.
+ * Use field addresses instead so this code will still
+ * work against older filesystems when the superblock
+ * gets rev'ed again with new fields appended.
*/
- if (pre_65_beta ||
- (sb->sb_versionnum & XR_GOOD_SECSB_VNMASK) == 0 ||
- sb->sb_versionnum < XFS_SB_VERSION_4) {
- /*
- * Check for garbage beyond the last field.
- * Use field addresses instead so this code will still
- * work against older filesystems when the superblock
- * gets rev'ed again with new fields appended.
- */
- 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))
+ size = (__psint_t)&sb->sb_logsunit
+ sizeof(sb->sb_logsunit) - (__psint_t)sb;
- else if (XFS_SB_VERSION_HASSECTOR(sb))
- size = (__psint_t)&sb->sb_logsectsize
+ else if (xfs_sb_version_hassector(sb))
+ size = (__psint_t)&sb->sb_logsectsize
+ sizeof(sb->sb_logsectsize) - (__psint_t)sb;
- else if (XFS_SB_VERSION_HASDIRV2(sb))
- size = (__psint_t)&sb->sb_dirblklog
+ else if (xfs_sb_version_hasdirv2(sb))
+ size = (__psint_t)&sb->sb_dirblklog
+ sizeof(sb->sb_dirblklog) - (__psint_t)sb;
- else
- size = (__psint_t)&sb->sb_width
+ else
+ size = (__psint_t)&sb->sb_width
+ sizeof(sb->sb_width) - (__psint_t)sb;
- for (ip = (char *)((__psint_t)sb + size);
- ip < (char *)((__psint_t)sb + mp->m_sb.sb_sectsize);
- ip++) {
- if (*ip) {
- do_bzero = 1;
- break;
- }
- }
cd
- if (do_bzero) {
- rval |= XR_AG_SB_SEC;
- if (!no_modify) {
- do_warn(
- _("zeroing unused portion of %s superblock (AG #%u)\n"),
- !i ? _("primary") : _("secondary"), i);
- bzero((void *)((__psint_t)sb + size),
- mp->m_sb.sb_sectsize - size);
- } else
- do_warn(
- _("would zero unused portion of %s superblock (AG #%u)\n"),
- !i ? _("primary") : _("secondary"), i);
+ for (ip = (char *)XFS_BUF_PTR(sbuf) + size;
+ ip < (char *)XFS_BUF_PTR(sbuf) + mp->m_sb.sb_sectsize; ip++) {
+ if (*ip) {
+ do_zero = 1;
+ break;
}
}
+ if (do_zero) {
+ rval |= XR_AG_SB_SEC;
+ if (!no_modify) {
+ do_warn(_("zeroing unused portion of %s superblock "
+ "(AG #%u)\n"), !agno ? _("primary") :
+ _("secondary"), agno);
+ memset((char *)XFS_BUF_PTR(sbuf) + size, 0,
+ mp->m_sb.sb_sectsize - size);
+ } else
+ do_warn(_("would zero unused portion of %s superblock "
+ "(AG #%u)\n"), !agno ? _("primary") :
+ _("secondary"), agno);
+ }
+
/*
- * now look for the fields we can manipulate directly.
- * if we did a bzero and that bzero could have included
- * the field in question, just silently reset it. otherwise,
- * complain.
+ * now look for the fields we can manipulate directly that
+ * may not have been zeroed above.
*
* for now, just zero the flags field since only
* the readonly flag is used
@@ -296,11 +280,8 @@ secondary_sb_wack(xfs_mount_t *mp, xfs_b
if (sb->sb_flags) {
if (!no_modify)
sb->sb_flags = 0;
- if (sb->sb_versionnum & XR_PART_SECSB_VNMASK || !do_bzero) {
- rval |= XR_AG_SB;
- do_warn(_("bad flags field in superblock %d\n"), i);
- } else
- rval |= XR_AG_SB_SEC;
+ rval |= XR_AG_SB;
+ do_warn(_("bad flags field in superblock %d\n"), agno);
}
/*
@@ -312,38 +293,24 @@ secondary_sb_wack(xfs_mount_t *mp, xfs_b
if (sb->sb_inprogress == 1 && sb->sb_uquotino) {
if (!no_modify)
sb->sb_uquotino = 0;
- if (sb->sb_versionnum & XR_PART_SECSB_VNMASK || !do_bzero) {
- rval |= XR_AG_SB;
- do_warn(
- _("non-null user quota inode field in superblock %d\n"),
- i);
-
- } else
- rval |= XR_AG_SB_SEC;
+ rval |= XR_AG_SB;
+ do_warn(_("non-null user quota inode field in superblock %d\n"),
+ agno);
}
if (sb->sb_inprogress == 1 && sb->sb_gquotino) {
if (!no_modify)
sb->sb_gquotino = 0;
- if (sb->sb_versionnum & XR_PART_SECSB_VNMASK || !do_bzero) {
- rval |= XR_AG_SB;
- do_warn(
- _("non-null group quota inode field in superblock %d\n"),
- i);
-
- } else
- rval |= XR_AG_SB_SEC;
+ rval |= XR_AG_SB;
+ do_warn(_("non-null group quota inode field in superblock
%d\n"),
+ agno);
}
if (sb->sb_inprogress == 1 && sb->sb_qflags) {
if (!no_modify)
sb->sb_qflags = 0;
- if (sb->sb_versionnum & XR_PART_SECSB_VNMASK || !do_bzero) {
- rval |= XR_AG_SB;
- do_warn(_("non-null quota flags in superblock %d\n"),
- i);
- } else
- rval |= XR_AG_SB_SEC;
+ rval |= XR_AG_SB;
+ do_warn(_("non-null quota flags in superblock %d\n"), agno);
}
/*
@@ -352,44 +319,31 @@ secondary_sb_wack(xfs_mount_t *mp, xfs_b
* written at mkfs time (and the corresponding sb version bits
* are set).
*/
- if (!XFS_SB_VERSION_HASSHARED(sb) && sb->sb_shared_vn != 0) {
+ if (!xfs_sb_version_hasshared(sb) && sb->sb_shared_vn) {
if (!no_modify)
sb->sb_shared_vn = 0;
- if (sb->sb_versionnum & XR_PART_SECSB_VNMASK || !do_bzero) {
- rval |= XR_AG_SB;
- do_warn(
- _("bad shared version number in superblock %d\n"),
- i);
- } else
- rval |= XR_AG_SB_SEC;
+ rval |= XR_AG_SB;
+ do_warn(_("bad shared version number in superblock %d\n"),
+ agno);
}
- if (!XFS_SB_VERSION_HASALIGN(sb) && sb->sb_inoalignmt != 0) {
+ if (!xfs_sb_version_hasalign(sb) && sb->sb_inoalignmt) {
if (!no_modify)
sb->sb_inoalignmt = 0;
- if (sb->sb_versionnum & XR_PART_SECSB_VNMASK || !do_bzero) {
- rval |= XR_AG_SB;
- do_warn(
- _("bad inode alignment field in superblock %d\n"),
- i);
- } else
- rval |= XR_AG_SB_SEC;
+ rval |= XR_AG_SB;
+ do_warn(_("bad inode alignment field in superblock %d\n"),
+ agno);
}
- if (!XFS_SB_VERSION_HASDALIGN(sb) &&
- (sb->sb_unit != 0 || sb->sb_width != 0)) {
+ if (!xfs_sb_version_hasdalign(sb) && (sb->sb_unit || sb->sb_width)) {
if (!no_modify)
sb->sb_unit = sb->sb_width = 0;
- if (sb->sb_versionnum & XR_GOOD_SECSB_VNMASK || !do_bzero) {
- rval |= XR_AG_SB;
- do_warn(
- _("bad stripe unit/width fields in superblock %d\n"),
- i);
- } else
- rval |= XR_AG_SB_SEC;
+ rval |= XR_AG_SB;
+ do_warn(_("bad stripe unit/width fields in superblock %d\n"),
+ agno);
}
- if (!XFS_SB_VERSION_HASSECTOR(sb) &&
+ if (!xfs_sb_version_hassector(sb) &&
(sb->sb_sectsize != BBSIZE || sb->sb_sectlog != BBSHIFT ||
sb->sb_logsectsize != 0 || sb->sb_logsectlog != 0)) {
if (!no_modify) {
@@ -398,13 +352,11 @@ secondary_sb_wack(xfs_mount_t *mp, xfs_b
sb->sb_logsectsize = 0;
sb->sb_logsectlog = 0;
}
- if (sb->sb_versionnum & XR_GOOD_SECSB_VNMASK || !do_bzero) {
+ if (!do_zero) {
rval |= XR_AG_SB;
- do_warn(
- _("bad log/data device sector size fields in superblock %d\n"),
- i);
- } else
- rval |= XR_AG_SB_SEC;
+ do_warn(_("bad log/data device sector size fields in "
+ "superblock %d\n"), agno);
+ }
}
return(rval);
@@ -463,7 +415,7 @@ verify_set_agheader(xfs_mount_t *mp, xfs
rval |= XR_AG_SB;
}
- rval |= secondary_sb_wack(mp, sbuf, sb, i);
+ rval |= sb_whack(mp, sb, sbuf, i);
rval |= verify_set_agf(mp, agf, i);
rval |= verify_set_agi(mp, agi, i);
|