Thread (13 messages) 13 messages, 4 authors, 2021-02-23

Re: [PATCH 1/1] xfs: Skip repetitive warnings about mount options

From: Eric Sandeen <hidden>
Date: 2021-02-22 22:20:24

On 2/20/21 4:15 PM, Pavel Reichl wrote:
quoted hunk ↗ jump to hunk
Skip the warnings about mount option being deprecated if we are
remounting and deprecated option state is not changing.

Bug: https://bugzilla.kernel.org/show_bug.cgi?id=211605
Fix-suggested-by: Eric Sandeen [off-list ref]
Signed-off-by: Pavel Reichl <redacted>
---
 fs/xfs/xfs_super.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 813be879a5e5..6724a7018d1f 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1169,6 +1169,13 @@ xfs_fs_parse_param(
 	struct fs_parse_result	result;
 	int			size = 0;
 	int			opt;
+	uint64_t                prev_m_flags = 0; /* Mount flags of prev. mount */
+	bool			remounting = fc->purpose & FS_CONTEXT_FOR_RECONFIGURE;
+
+	/* if reconfiguring then get mount flags of previous flags */
+	if (remounting) {
+		prev_m_flags  = XFS_M(fc->root->d_sb)->m_flags;
I wonder, does mp->m_flags work just as well for this purpose? I do get lost
in how the mount api stashes things. I /think/ that the above is just a
long way of getting to mp->m_flags.
quoted hunk ↗ jump to hunk
+	}
 
 	opt = fs_parse(fc, xfs_fs_parameters, param, &result);
 	if (opt < 0)
@@ -1294,19 +1301,27 @@ xfs_fs_parse_param(
 #endif
 	/* Following mount options will be removed in September 2025 */
 	case Opt_ikeep:
-		xfs_warn(mp, "%s mount option is deprecated.", param->key);
+		if (!remounting ||  !(prev_m_flags & XFS_MOUNT_IKEEP)) {
while we're nitpicking whitespace, ^^ 2 spaces there

as for the prev_m_flags usage, does:

+		if (!remounting || !(mp->m_flags & XFS_MOUNT_IKEEP)) {

work just as well here or no?
+			xfs_warn(mp, "%s mount option is deprecated.", param->key);
+		}
 		mp->m_flags |= XFS_MOUNT_IKEEP;
 		return 0;
 	case Opt_noikeep:
-		xfs_warn(mp, "%s mount option is deprecated.", param->key);
+		if (!remounting || prev_m_flags & XFS_MOUNT_IKEEP) {
and I dunno, I think I'd like parentheses for clarity here i.e.:

+		if (!remounting || (prev_m_flags & XFS_MOUNT_IKEEP)) {

Thanks,
-Eric

+			xfs_warn(mp, "%s mount option is deprecated.", param->key);
+		}
 		mp->m_flags &= ~XFS_MOUNT_IKEEP;
 		return 0;
 	case Opt_attr2:
-		xfs_warn(mp, "%s mount option is deprecated.", param->key);
+		if (!remounting || !(prev_m_flags & XFS_MOUNT_ATTR2)) {
+			xfs_warn(mp, "%s mount option is deprecated.", param->key);
+		}
 		mp->m_flags |= XFS_MOUNT_ATTR2;
 		return 0;
 	case Opt_noattr2:
-		xfs_warn(mp, "%s mount option is deprecated.", param->key);
+		if (!remounting || !(prev_m_flags & XFS_MOUNT_NOATTR2)) {
+			xfs_warn(mp, "%s mount option is deprecated.", param->key);
+		}
 		mp->m_flags &= ~XFS_MOUNT_ATTR2;
 		mp->m_flags |= XFS_MOUNT_NOATTR2;
 		return 0;
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help