xfs
[Top] [All Lists]

Re: [REVIEW #4] bad_features2 support in userspace

To: Barry Naujok <bnaujok@xxxxxxx>
Subject: Re: [REVIEW #4] bad_features2 support in userspace
From: Lachlan McIlroy <lachlan@xxxxxxx>
Date: Wed, 05 Mar 2008 17:33:30 +1100
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
In-reply-to: <op.t7i0f5123jf8g2@pc-bnaujok.melbourne.sgi.com>
References: <op.t7i0f5123jf8g2@pc-bnaujok.melbourne.sgi.com>
Reply-to: lachlan@xxxxxxx
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.12 (X11/20080213)
Looks good to me.

Barry Naujok wrote:
Due to the issue of mounting filesystem with older kernels and
potentially reading sb_features2 from the wrong location. It
seems the best course of action is to always make sb_features2
and sb_bad_features2 the same. This is pretty important as
new bits in this are supposed to stop older kernels from
mounting filesystems with unsupported features.

If sb_bad_features2 is zero, and the old kernel tries to read
sb_features2 from this location during mount, it will succeed
as it will read zero.

So, this patch changes mkfs.xfs to set sb_bad_features2 to
the same as sb_features2, xfs_check and xfs_repair now also
makes sure they are the same.

Barry.

--


=========================================================================== xfsprogs/db/check.c ===========================================================================

--- a/xfsprogs/db/check.c    2008-03-05 15:30:54.000000000 +1100
+++ b/xfsprogs/db/check.c    2008-03-05 15:28:58.638097511 +1100
@@ -869,6 +869,14 @@ blockget_f(
                 mp->m_sb.sb_frextents, frextents);
         error++;
     }
+    if (mp->m_sb.sb_bad_features2 != mp->m_sb.sb_features2) {
+        if (!sflag)
+            dbprintf("sb_features2 (0x%x) not same as "
+                "sb_bad_features2 (0x%x)\n",
+                mp->m_sb.sb_features2,
+                mp->m_sb.sb_bad_features2);
+        error++;
+    }
     if ((sbversion & XFS_SB_VERSION_ATTRBIT) &&
         !XFS_SB_VERSION_HASATTR(&mp->m_sb)) {
         if (!sflag)

===========================================================================
xfsprogs/db/sb.c
===========================================================================

--- a/xfsprogs/db/sb.c 2008-03-05 15:30:54.000000000 +1100
+++ b/xfsprogs/db/sb.c 2008-02-29 17:16:33.770423296 +1100
@@ -108,6 +108,7 @@ const field_t sb_flds[] = {
{ "logsectsize", FLDT_UINT16D, OI(OFF(logsectsize)), C1, 0, TYP_NONE },
{ "logsunit", FLDT_UINT32D, OI(OFF(logsunit)), C1, 0, TYP_NONE },
{ "features2", FLDT_UINT32X, OI(OFF(features2)), C1, 0, TYP_NONE },
+ { "bad_features2", FLDT_UINT32X, OI(OFF(bad_features2)), C1, 0, TYP_NONE },
{ NULL }
};



=========================================================================== xfsprogs/include/xfs_sb.h ===========================================================================

--- a/xfsprogs/include/xfs_sb.h    2008-03-05 15:30:54.000000000 +1100
+++ b/xfsprogs/include/xfs_sb.h    2008-02-29 17:16:33.814417687 +1100
@@ -151,6 +151,7 @@ typedef struct xfs_sb
     __uint16_t    sb_logsectsize;    /* sector size for the log, bytes */
     __uint32_t    sb_logsunit;    /* stripe unit size for the log */
     __uint32_t    sb_features2;    /* additional feature bits */
+    __uint32_t    sb_bad_features2; /* unusable space */
 } xfs_sb_t;

 /*
@@ -169,7 +170,7 @@ typedef enum {
     XFS_SBS_GQUOTINO, XFS_SBS_QFLAGS, XFS_SBS_FLAGS, XFS_SBS_SHARED_VN,
     XFS_SBS_INOALIGNMT, XFS_SBS_UNIT, XFS_SBS_WIDTH, XFS_SBS_DIRBLKLOG,
     XFS_SBS_LOGSECTLOG, XFS_SBS_LOGSECTSIZE, XFS_SBS_LOGSUNIT,
-    XFS_SBS_FEATURES2,
+    XFS_SBS_FEATURES2, XFS_SBS_BAD_FEATURES2,
     XFS_SBS_FIELDCOUNT
 } xfs_sb_field_t;


=========================================================================== xfsprogs/libxfs/xfs_mount.c ===========================================================================

--- a/xfsprogs/libxfs/xfs_mount.c    2008-03-05 15:30:54.000000000 +1100
+++ b/xfsprogs/libxfs/xfs_mount.c    2008-02-29 17:16:33.834415138 +1100
@@ -140,6 +140,7 @@ static struct {
     { offsetof(xfs_sb_t, sb_logsectsize),0 },
     { offsetof(xfs_sb_t, sb_logsunit),     0 },
     { offsetof(xfs_sb_t, sb_features2),     0 },
+    { offsetof(xfs_sb_t, sb_bad_features2), 0 },
     { sizeof(xfs_sb_t),             0 }
 };


=========================================================================== xfsprogs/mkfs/xfs_mkfs.c ===========================================================================

--- a/xfsprogs/mkfs/xfs_mkfs.c    2008-03-05 15:30:54.000000000 +1100
+++ b/xfsprogs/mkfs/xfs_mkfs.c    2008-03-05 15:27:37.568461787 +1100
@@ -2103,6 +2103,13 @@ an AG size that is one stripe unit small
             dirversion == 2, logversion == 2, attrversion == 1,
             (sectorsize != BBSIZE || lsectorsize != BBSIZE),
             sbp->sb_features2 != 0);
+    /*
+     * Due to a structure alignment issue, sb_features2 ended up in one
+     * of two locations, the second "incorrect" location represented by
+     * the sb_bad_features2 field. To avoid older kernels mounting
+     * filesystems they shouldn't, set both field to the same value.
+     */
+    sbp->sb_bad_features2 = sbp->sb_features2;

     if (force_overwrite)
         zero_old_xfs_structures(&xi, sbp);

===========================================================================
xfsprogs/repair/phase1.c
===========================================================================

--- a/xfsprogs/repair/phase1.c    2008-03-05 15:30:54.000000000 +1100
+++ b/xfsprogs/repair/phase1.c    2008-03-05 15:19:09.513415413 +1100
@@ -91,6 +91,19 @@ phase1(xfs_mount_t *mp)
         primary_sb_modified = 1;
     }

+    /*
+     * Check bad_features2 and make sure features2 the same as
+     * bad_features (ORing the two together). Leave bad_features2
+     * set so older kernels can still use it and not mount unsupported
+     * filesystems when it reads bad_features2.
+     */
+    if (sb->sb_bad_features2 != sb->sb_features2) {
+        sb->sb_features2 |= sb->sb_bad_features2;
+        sb->sb_bad_features2 = sb->sb_features2;
+        primary_sb_modified = 1;
+        do_warn(_("superblock has a features2 mismatch, correcting\n"));
+    }
+
     if (primary_sb_modified)  {
         if (!no_modify)  {
             do_warn(_("writing modified primary superblock\n"));



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