xfs: fix log recovery when unknown rocompat bits are set

Log recovery has always run on read only mounts, even where the primary
superblock advertises unknown rocompat bits.  Due to a misunderstanding
between Eric and Darrick back in 2018, we accidentally changed the
superblock write verifier to shutdown the fs over that exact scenario.
As a result, the log cleaning that occurs at the end of the mounting
process fails if there are unknown rocompat bits set.

As we now allow writing of the superblock if there are unknown rocompat
bits set on a RO mount, we no longer want to turn off RO state to allow
log recovery to succeed on a RO mount.  Hence we also remove all the
(now unnecessary) RO state toggling from the log recovery path.

Fixes: 9e037cb7972f ("xfs: check for unknown v5 feature bits in superblock write verifier"
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 5e174685a77c5ee4f52984e71530a585bf011aff..6264daaab37b06151bd12abc2ef9f41f61fcc675 100644 (file)
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -266,7 +266,8 @@ xfs_validate_sb_write(
                return -EFSCORRUPTED;
        }
 
-       if (xfs_sb_has_ro_compat_feature(sbp, XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) {
+       if (!xfs_is_readonly(mp) &&
+           xfs_sb_has_ro_compat_feature(sbp, XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) {
                xfs_alert(mp,
 "Corruption detected in superblock read-only compatible features (0x%x)!",
                        (sbp->sb_features_ro_compat &

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 79004d193e54dc2b06c796cef994d0bb1ba786eb..51c100c861770f619776a8e3efb921d0b2e55bd8 100644 (file)
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -715,15 +715,7 @@ xfs_log_mount(
         * just worked.
         */
        if (!xfs_has_norecovery(mp)) {
-               /*
-                * log recovery ignores readonly state and so we need to clear
-                * mount-based read only state so it can write to disk.
-                */
-               bool    readonly = test_and_clear_bit(XFS_OPSTATE_READONLY,
-                                               &mp->m_opstate);
                error = xlog_recover(log);
-               if (readonly)
-                       set_bit(XFS_OPSTATE_READONLY, &mp->m_opstate);
                if (error) {
                        xfs_warn(mp, "log mount/recovery failed: error %d",
                                error);
@@ -772,7 +764,6 @@ xfs_log_mount_finish(
        struct xfs_mount        *mp)
 {
        struct xlog             *log = mp->m_log;
-       bool                    readonly;
        int                     error = 0;
 
        if (xfs_has_norecovery(mp)) {
@@ -780,12 +771,6 @@ xfs_log_mount_finish(
                return 0;
        }
 
-       /*
-        * log recovery ignores readonly state and so we need to clear
-        * mount-based read only state so it can write to disk.
-        */
-       readonly = test_and_clear_bit(XFS_OPSTATE_READONLY, &mp->m_opstate);
-
        /*
         * During the second phase of log recovery, we need iget and
         * iput to behave like they do for an active filesystem.
@@ -835,8 +820,6 @@ xfs_log_mount_finish(
        xfs_buftarg_drain(mp->m_ddev_targp);
 
        clear_bit(XLOG_RECOVERY_NEEDED, &log->l_opstate);
-       if (readonly)
-               set_bit(XFS_OPSTATE_READONLY, &mp->m_opstate);
 
        /* Make sure the log is dead if we're returning failure. */
        ASSERT(!error || xlog_is_shutdown(log));