linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/4] ext4: Make mount option parsing loop more logical
  2012-04-16 12:25 [PATCH 0/4] ext4: Fix regression in mount option parsing and a small cleanup Jan Kara
@ 2012-04-16 12:25 ` Jan Kara
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2012-04-16 12:25 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-ext4, Jan Kara

The loop looking for correct mount option entry is more logical if it is
written rewritten as an empty loop looking for correct option entry and then
code handling the option. It also saves one level of indentation for a lot of
code so we can join a couple of split lines.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/super.c |  224 +++++++++++++++++++++++++++----------------------------
 1 files changed, 111 insertions(+), 113 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 30e26f1..167aaac 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1473,130 +1473,128 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 		return 1;
 	}
 
-	for (m = ext4_mount_opts; m->token != Opt_err; m++) {
-		if (token != m->token)
-			continue;
-		if (args->from && match_int(args, &arg))
+	for (m = ext4_mount_opts; m->token != Opt_err && token != m->token;
+	     m++);
+	if (m->token == Opt_err) {
+		ext4_msg(sb, KERN_ERR, "Unrecognized mount option \"%s\" "
+			 "or missing value", opt);
+		return -1;
+	}
+	
+	if (args->from && match_int(args, &arg))
+		return -1;
+	if (args->from && (m->flags & MOPT_GTE0) && (arg < 0))
+		return -1;
+	if (m->flags & MOPT_EXPLICIT)
+		set_opt2(sb, EXPLICIT_DELALLOC);
+	if (m->flags & MOPT_CLEAR_ERR)
+		clear_opt(sb, ERRORS_MASK);
+	if (token == Opt_noquota && sb_any_quota_loaded(sb)) {
+		ext4_msg(sb, KERN_ERR, "Cannot change quota "
+			 "options when quota turned on");
+		return -1;
+	}
+
+	if (m->flags & MOPT_NOSUPPORT) {
+		ext4_msg(sb, KERN_ERR, "%s option not supported", opt);
+	} else if (token == Opt_commit) {
+		if (arg == 0)
+			arg = JBD2_DEFAULT_MAX_COMMIT_AGE;
+		sbi->s_commit_interval = HZ * arg;
+	} else if (token == Opt_max_batch_time) {
+		if (arg == 0)
+			arg = EXT4_DEF_MAX_BATCH_TIME;
+		sbi->s_max_batch_time = arg;
+	} else if (token == Opt_min_batch_time) {
+		sbi->s_min_batch_time = arg;
+	} else if (token == Opt_inode_readahead_blks) {
+		if (arg > (1 << 30))
 			return -1;
-		if (args->from && (m->flags & MOPT_GTE0) && (arg < 0))
+		if (arg && !is_power_of_2(arg)) {
+			ext4_msg(sb, KERN_ERR,
+				 "EXT4-fs: inode_readahead_blks"
+				 " must be a power of 2");
 			return -1;
-		if (m->flags & MOPT_EXPLICIT)
-			set_opt2(sb, EXPLICIT_DELALLOC);
-		if (m->flags & MOPT_CLEAR_ERR)
-			clear_opt(sb, ERRORS_MASK);
-		if (token == Opt_noquota && sb_any_quota_loaded(sb)) {
-			ext4_msg(sb, KERN_ERR, "Cannot change quota "
-				 "options when quota turned on");
+		}
+		sbi->s_inode_readahead_blks = arg;
+	} else if (token == Opt_init_itable) {
+		set_opt(sb, INIT_INODE_TABLE);
+		if (!args->from)
+			arg = EXT4_DEF_LI_WAIT_MULT;
+		sbi->s_li_wait_mult = arg;
+	} else if (token == Opt_stripe) {
+		sbi->s_stripe = arg;
+	} else if (token == Opt_resuid) {
+		sbi->s_resuid = arg;
+	} else if (token == Opt_resgid) {
+		sbi->s_resgid = arg;
+	} else if (token == Opt_journal_dev) {
+		if (is_remount) {
+			ext4_msg(sb, KERN_ERR,
+				 "Cannot specify journal on remount");
 			return -1;
 		}
-
-		if (m->flags & MOPT_NOSUPPORT) {
-			ext4_msg(sb, KERN_ERR, "%s option not supported", opt);
-		} else if (token == Opt_commit) {
-			if (arg == 0)
-				arg = JBD2_DEFAULT_MAX_COMMIT_AGE;
-			sbi->s_commit_interval = HZ * arg;
-		} else if (token == Opt_max_batch_time) {
-			if (arg == 0)
-				arg = EXT4_DEF_MAX_BATCH_TIME;
-			sbi->s_max_batch_time = arg;
-		} else if (token == Opt_min_batch_time) {
-			sbi->s_min_batch_time = arg;
-		} else if (token == Opt_inode_readahead_blks) {
-			if (arg > (1 << 30))
-				return -1;
-			if (arg && !is_power_of_2(arg)) {
-				ext4_msg(sb, KERN_ERR,
-					 "EXT4-fs: inode_readahead_blks"
-					 " must be a power of 2");
-				return -1;
-			}
-			sbi->s_inode_readahead_blks = arg;
-		} else if (token == Opt_init_itable) {
-			set_opt(sb, INIT_INODE_TABLE);
-			if (!args->from)
-				arg = EXT4_DEF_LI_WAIT_MULT;
-			sbi->s_li_wait_mult = arg;
-		} else if (token == Opt_stripe) {
-			sbi->s_stripe = arg;
-		} else if (token == Opt_resuid) {
-			sbi->s_resuid = arg;
-		} else if (token == Opt_resgid) {
-			sbi->s_resgid = arg;
-		} else if (token == Opt_journal_dev) {
-			if (is_remount) {
-				ext4_msg(sb, KERN_ERR,
-					 "Cannot specify journal on remount");
-				return -1;
-			}
-			*journal_devnum = arg;
-		} else if (token == Opt_journal_ioprio) {
-			if (arg > 7) {
+		*journal_devnum = arg;
+	} else if (token == Opt_journal_ioprio) {
+		if (arg > 7) {
+			ext4_msg(sb, KERN_ERR,
+				 "Invalid journal IO priority (must be 0-7)");
+			return -1;
+		}
+		*journal_ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, arg);
+	} else if (m->flags & MOPT_DATAJ) {
+		if (is_remount) {
+			if (!sbi->s_journal)
+				ext4_msg(sb, KERN_WARNING, "Remounting file system with no journal so ignoring journalled data option");
+			else if (test_opt(sb, DATA_FLAGS) != m->mount_opt) {
 				ext4_msg(sb, KERN_ERR,
-					 "Invalid journal IO priority"
-					 " (must be 0-7)");
+				 "Cannot change data mode on remount");
 				return -1;
 			}
-			*journal_ioprio =
-				IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, arg);
-		} else if (m->flags & MOPT_DATAJ) {
-			if (is_remount) {
-				if (!sbi->s_journal)
-					ext4_msg(sb, KERN_WARNING, "Remounting file system with no journal so ignoring journalled data option");
-				else if (test_opt(sb, DATA_FLAGS) !=
-					 m->mount_opt) {
-					ext4_msg(sb, KERN_ERR,
-					 "Cannot change data mode on remount");
-					return -1;
-				}
-			} else {
-				clear_opt(sb, DATA_FLAGS);
-				sbi->s_mount_opt |= m->mount_opt;
-			}
+		} else {
+			clear_opt(sb, DATA_FLAGS);
+			sbi->s_mount_opt |= m->mount_opt;
+		}
 #ifdef CONFIG_QUOTA
-		} else if (token == Opt_usrjquota) {
-			if (!set_qf_name(sb, USRQUOTA, &args[0]))
-				return -1;
-		} else if (token == Opt_grpjquota) {
-			if (!set_qf_name(sb, GRPQUOTA, &args[0]))
-				return -1;
-		} else if (token == Opt_offusrjquota) {
-			if (!clear_qf_name(sb, USRQUOTA))
-				return -1;
-		} else if (token == Opt_offgrpjquota) {
-			if (!clear_qf_name(sb, GRPQUOTA))
-				return -1;
-		} else if (m->flags & MOPT_QFMT) {
-			if (sb_any_quota_loaded(sb) &&
-			    sbi->s_jquota_fmt != m->mount_opt) {
-				ext4_msg(sb, KERN_ERR, "Cannot "
-					 "change journaled quota options "
-					 "when quota turned on");
-				return -1;
-			}
-			sbi->s_jquota_fmt = m->mount_opt;
+	} else if (token == Opt_usrjquota) {
+		if (!set_qf_name(sb, USRQUOTA, &args[0]))
+			return -1;
+	} else if (token == Opt_grpjquota) {
+		if (!set_qf_name(sb, GRPQUOTA, &args[0]))
+			return -1;
+	} else if (token == Opt_offusrjquota) {
+		if (!clear_qf_name(sb, USRQUOTA))
+			return -1;
+	} else if (token == Opt_offgrpjquota) {
+		if (!clear_qf_name(sb, GRPQUOTA))
+			return -1;
+	} else if (m->flags & MOPT_QFMT) {
+		if (sb_any_quota_loaded(sb) &&
+		    sbi->s_jquota_fmt != m->mount_opt) {
+			ext4_msg(sb, KERN_ERR, "Cannot "
+				 "change journaled quota options "
+				 "when quota turned on");
+			return -1;
+		}
+		sbi->s_jquota_fmt = m->mount_opt;
 #endif
-		} else {
-			if (!args->from)
-				arg = 1;
-			if (m->flags & MOPT_CLEAR)
-				arg = !arg;
-			else if (unlikely(!(m->flags & MOPT_SET))) {
-				ext4_msg(sb, KERN_WARNING,
-					 "buggy handling of option %s", opt);
-				WARN_ON(1);
-				return -1;
-			}
-			if (arg != 0)
-				sbi->s_mount_opt |= m->mount_opt;
-			else
-				sbi->s_mount_opt &= ~m->mount_opt;
+	} else {
+		if (!args->from)
+			arg = 1;
+		if (m->flags & MOPT_CLEAR)
+			arg = !arg;
+		else if (unlikely(!(m->flags & MOPT_SET))) {
+			ext4_msg(sb, KERN_WARNING,
+				 "buggy handling of option %s", opt);
+			WARN_ON(1);
+			return -1;
 		}
-		return 1;
+		if (arg != 0)
+			sbi->s_mount_opt |= m->mount_opt;
+		else
+			sbi->s_mount_opt &= ~m->mount_opt;
 	}
-	ext4_msg(sb, KERN_ERR, "Unrecognized mount option \"%s\" "
-		 "or missing value", opt);
-	return -1;
+	return 1;
 }
 
 static int parse_options(char *options, struct super_block *sb,
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 0/4] Clean up ext4's mount option parsing code
@ 2013-02-03  4:44 Theodore Ts'o
  2013-02-03  4:44 ` [PATCH 1/4] ext4: move several mount options to standard handling loop Theodore Ts'o
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Theodore Ts'o @ 2013-02-03  4:44 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

The first three patches are from a patch series which Jan Kara submitted
back in April 2012, forward ported to modern kernels.  The last patch
addresses the feature which Carlos suggested, but I've implemented it as
a flag in the mount options parsing, instead of using the mount option
flags.  This allows us to disallow features which don't involve setting
a mount option flags.

						- Ted

Jan Kara (3):
  ext4: move several mount options to standard handling loop
  ext4: make mount option parsing loop more logical
  ext4: print error when argument of inode_readahead_blk is invalid

Theodore Ts'o (1):
  ext4: check incompatible mount options while mounting ext2/3

 fs/ext4/super.c | 261 +++++++++++++++++++++++++++++++-------------------------
 1 file changed, 143 insertions(+), 118 deletions(-)

-- 
1.7.12.rc0.22.gcdd159b


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] ext4: move several mount options to standard handling loop
  2013-02-03  4:44 [PATCH 0/4] Clean up ext4's mount option parsing code Theodore Ts'o
@ 2013-02-03  4:44 ` Theodore Ts'o
  2013-02-03  4:44 ` [PATCH 2/4] ext4: make mount option parsing loop more logical Theodore Ts'o
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2013-02-03  4:44 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Jan Kara, Theodore Ts'o

From: Jan Kara <jack@suse.cz>

Several mount option (resuid, resgid, journal_dev, journal_ioprio) are
currently handled before we enter standard option handling loop. I don't
see a reason for this so move them to normal handling loop to make things
more regular.

Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/super.c | 69 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 38 insertions(+), 31 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index dc20e4d..78c9336 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1447,6 +1447,10 @@ static const struct mount_opts {
 	{Opt_inode_readahead_blks, 0, MOPT_GTE0},
 	{Opt_init_itable, 0, MOPT_GTE0},
 	{Opt_stripe, 0, MOPT_GTE0},
+	{Opt_resuid, 0, MOPT_GTE0},
+	{Opt_resgid, 0, MOPT_GTE0},
+	{Opt_journal_dev, 0, MOPT_GTE0},
+	{Opt_journal_ioprio, 0, MOPT_GTE0},
 	{Opt_data_journal, EXT4_MOUNT_JOURNAL_DATA, MOPT_DATAJ},
 	{Opt_data_ordered, EXT4_MOUNT_ORDERED_DATA, MOPT_DATAJ},
 	{Opt_data_writeback, EXT4_MOUNT_WRITEBACK_DATA, MOPT_DATAJ},
@@ -1499,8 +1503,6 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 	else if (token == Opt_offgrpjquota)
 		return clear_qf_name(sb, GRPQUOTA);
 #endif
-	if (args->from && match_int(args, &arg))
-		return -1;
 	switch (token) {
 	case Opt_noacl:
 	case Opt_nouser_xattr:
@@ -1512,46 +1514,19 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 		ext4_msg(sb, KERN_WARNING,
 			 "Ignoring removed %s option", opt);
 		return 1;
-	case Opt_resuid:
-		uid = make_kuid(current_user_ns(), arg);
-		if (!uid_valid(uid)) {
-			ext4_msg(sb, KERN_ERR, "Invalid uid value %d", arg);
-			return -1;
-		}
-		sbi->s_resuid = uid;
-		return 1;
-	case Opt_resgid:
-		gid = make_kgid(current_user_ns(), arg);
-		if (!gid_valid(gid)) {
-			ext4_msg(sb, KERN_ERR, "Invalid gid value %d", arg);
-			return -1;
-		}
-		sbi->s_resgid = gid;
-		return 1;
 	case Opt_abort:
 		sbi->s_mount_flags |= EXT4_MF_FS_ABORTED;
 		return 1;
 	case Opt_i_version:
 		sb->s_flags |= MS_I_VERSION;
 		return 1;
-	case Opt_journal_dev:
-		if (is_remount) {
-			ext4_msg(sb, KERN_ERR,
-				 "Cannot specify journal on remount");
-			return -1;
-		}
-		*journal_devnum = arg;
-		return 1;
-	case Opt_journal_ioprio:
-		if (arg < 0 || arg > 7)
-			return -1;
-		*journal_ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, arg);
-		return 1;
 	}
 
 	for (m = ext4_mount_opts; m->token != Opt_err; m++) {
 		if (token != m->token)
 			continue;
+		if (args->from && match_int(args, &arg))
+			return -1;
 		if (args->from && (m->flags & MOPT_GTE0) && (arg < 0))
 			return -1;
 		if (m->flags & MOPT_EXPLICIT)
@@ -1595,6 +1570,38 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 			sbi->s_max_dir_size_kb = arg;
 		} else if (token == Opt_stripe) {
 			sbi->s_stripe = arg;
+		} else if (token == Opt_resuid) {
+			uid = make_kuid(current_user_ns(), arg);
+			if (!uid_valid(uid)) {
+				ext4_msg(sb, KERN_ERR,
+					 "Invalid uid value %d", arg);
+				return -1;
+			}
+			sbi->s_resuid = uid;
+		} else if (token == Opt_resgid) {
+			gid = make_kgid(current_user_ns(), arg);
+			if (!gid_valid(gid)) {
+				ext4_msg(sb, KERN_ERR,
+					 "Invalid gid value %d", arg);
+				return -1;
+			}
+			sbi->s_resgid = gid;
+		} else if (token == Opt_journal_dev) {
+			if (is_remount) {
+				ext4_msg(sb, KERN_ERR,
+					 "Cannot specify journal on remount");
+				return -1;
+			}
+			*journal_devnum = arg;
+		} else if (token == Opt_journal_ioprio) {
+			if (arg > 7) {
+				ext4_msg(sb, KERN_ERR,
+					 "Invalid journal IO priority"
+					 " (must be 0-7)");
+				return -1;
+			}
+			*journal_ioprio =
+				IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, arg);
 		} else if (m->flags & MOPT_DATAJ) {
 			if (is_remount) {
 				if (!sbi->s_journal)
-- 
1.7.12.rc0.22.gcdd159b


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] ext4: make mount option parsing loop more logical
  2013-02-03  4:44 [PATCH 0/4] Clean up ext4's mount option parsing code Theodore Ts'o
  2013-02-03  4:44 ` [PATCH 1/4] ext4: move several mount options to standard handling loop Theodore Ts'o
@ 2013-02-03  4:44 ` Theodore Ts'o
  2013-02-03  4:44 ` [PATCH 3/4] ext4: print error when argument of inode_readahead_blk is invalid Theodore Ts'o
  2013-02-03  4:44 ` [PATCH 4/4] ext4: check incompatible mount options while mounting ext2/3 Theodore Ts'o
  3 siblings, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2013-02-03  4:44 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Jan Kara, Theodore Ts'o

From: Jan Kara <jack@suse.cz>

The loop looking for correct mount option entry is more logical if it is
written rewritten as an empty loop looking for correct option entry and then
code handling the option. It also saves one level of indentation for a lot of
code so we can join a couple of split lines.

Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/super.c | 230 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 113 insertions(+), 117 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 78c9336..e8d3c2f 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1511,8 +1511,7 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 	case Opt_sb:
 		return 1;	/* handled by get_sb_block() */
 	case Opt_removed:
-		ext4_msg(sb, KERN_WARNING,
-			 "Ignoring removed %s option", opt);
+		ext4_msg(sb, KERN_WARNING, "Ignoring removed %s option", opt);
 		return 1;
 	case Opt_abort:
 		sbi->s_mount_flags |= EXT4_MF_FS_ABORTED;
@@ -1522,132 +1521,129 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 		return 1;
 	}
 
-	for (m = ext4_mount_opts; m->token != Opt_err; m++) {
-		if (token != m->token)
-			continue;
-		if (args->from && match_int(args, &arg))
+	for (m = ext4_mount_opts; m->token != Opt_err; m++)
+		if (token == m->token)
+			break;
+
+	if (m->token == Opt_err) {
+		ext4_msg(sb, KERN_ERR, "Unrecognized mount option \"%s\" "
+			 "or missing value", opt);
+		return -1;
+	}
+
+	if (args->from && match_int(args, &arg))
+		return -1;
+	if (args->from && (m->flags & MOPT_GTE0) && (arg < 0))
+		return -1;
+	if (m->flags & MOPT_EXPLICIT)
+		set_opt2(sb, EXPLICIT_DELALLOC);
+	if (m->flags & MOPT_CLEAR_ERR)
+		clear_opt(sb, ERRORS_MASK);
+	if (token == Opt_noquota && sb_any_quota_loaded(sb)) {
+		ext4_msg(sb, KERN_ERR, "Cannot change quota "
+			 "options when quota turned on");
+		return -1;
+	}
+
+	if (m->flags & MOPT_NOSUPPORT) {
+		ext4_msg(sb, KERN_ERR, "%s option not supported", opt);
+	} else if (token == Opt_commit) {
+		if (arg == 0)
+			arg = JBD2_DEFAULT_MAX_COMMIT_AGE;
+		sbi->s_commit_interval = HZ * arg;
+	} else if (token == Opt_max_batch_time) {
+		if (arg == 0)
+			arg = EXT4_DEF_MAX_BATCH_TIME;
+		sbi->s_max_batch_time = arg;
+	} else if (token == Opt_min_batch_time) {
+		sbi->s_min_batch_time = arg;
+	} else if (token == Opt_inode_readahead_blks) {
+		if (arg > (1 << 30))
 			return -1;
-		if (args->from && (m->flags & MOPT_GTE0) && (arg < 0))
+		if (arg && !is_power_of_2(arg)) {
+			ext4_msg(sb, KERN_ERR, "EXT4-fs: inode_readahead_blks"
+				 " must be a power of 2");
 			return -1;
-		if (m->flags & MOPT_EXPLICIT)
-			set_opt2(sb, EXPLICIT_DELALLOC);
-		if (m->flags & MOPT_CLEAR_ERR)
-			clear_opt(sb, ERRORS_MASK);
-		if (token == Opt_noquota && sb_any_quota_loaded(sb)) {
-			ext4_msg(sb, KERN_ERR, "Cannot change quota "
-				 "options when quota turned on");
+		}
+		sbi->s_inode_readahead_blks = arg;
+	} else if (token == Opt_init_itable) {
+		set_opt(sb, INIT_INODE_TABLE);
+		if (!args->from)
+			arg = EXT4_DEF_LI_WAIT_MULT;
+		sbi->s_li_wait_mult = arg;
+	} else if (token == Opt_max_dir_size_kb) {
+		sbi->s_max_dir_size_kb = arg;
+	} else if (token == Opt_stripe) {
+		sbi->s_stripe = arg;
+	} else if (token == Opt_resuid) {
+		uid = make_kuid(current_user_ns(), arg);
+		if (!uid_valid(uid)) {
+			ext4_msg(sb, KERN_ERR, "Invalid uid value %d", arg);
 			return -1;
 		}
-
-		if (m->flags & MOPT_NOSUPPORT) {
-			ext4_msg(sb, KERN_ERR, "%s option not supported", opt);
-		} else if (token == Opt_commit) {
-			if (arg == 0)
-				arg = JBD2_DEFAULT_MAX_COMMIT_AGE;
-			sbi->s_commit_interval = HZ * arg;
-		} else if (token == Opt_max_batch_time) {
-			if (arg == 0)
-				arg = EXT4_DEF_MAX_BATCH_TIME;
-			sbi->s_max_batch_time = arg;
-		} else if (token == Opt_min_batch_time) {
-			sbi->s_min_batch_time = arg;
-		} else if (token == Opt_inode_readahead_blks) {
-			if (arg > (1 << 30))
-				return -1;
-			if (arg && !is_power_of_2(arg)) {
-				ext4_msg(sb, KERN_ERR,
-					 "EXT4-fs: inode_readahead_blks"
-					 " must be a power of 2");
-				return -1;
-			}
-			sbi->s_inode_readahead_blks = arg;
-		} else if (token == Opt_init_itable) {
-			set_opt(sb, INIT_INODE_TABLE);
-			if (!args->from)
-				arg = EXT4_DEF_LI_WAIT_MULT;
-			sbi->s_li_wait_mult = arg;
-		} else if (token == Opt_max_dir_size_kb) {
-			sbi->s_max_dir_size_kb = arg;
-		} else if (token == Opt_stripe) {
-			sbi->s_stripe = arg;
-		} else if (token == Opt_resuid) {
-			uid = make_kuid(current_user_ns(), arg);
-			if (!uid_valid(uid)) {
-				ext4_msg(sb, KERN_ERR,
-					 "Invalid uid value %d", arg);
-				return -1;
-			}
-			sbi->s_resuid = uid;
-		} else if (token == Opt_resgid) {
-			gid = make_kgid(current_user_ns(), arg);
-			if (!gid_valid(gid)) {
-				ext4_msg(sb, KERN_ERR,
-					 "Invalid gid value %d", arg);
-				return -1;
-			}
-			sbi->s_resgid = gid;
-		} else if (token == Opt_journal_dev) {
-			if (is_remount) {
-				ext4_msg(sb, KERN_ERR,
-					 "Cannot specify journal on remount");
-				return -1;
-			}
-			*journal_devnum = arg;
-		} else if (token == Opt_journal_ioprio) {
-			if (arg > 7) {
+		sbi->s_resuid = uid;
+	} else if (token == Opt_resgid) {
+		gid = make_kgid(current_user_ns(), arg);
+		if (!gid_valid(gid)) {
+			ext4_msg(sb, KERN_ERR, "Invalid gid value %d", arg);
+			return -1;
+		}
+		sbi->s_resgid = gid;
+	} else if (token == Opt_journal_dev) {
+		if (is_remount) {
+			ext4_msg(sb, KERN_ERR,
+				 "Cannot specify journal on remount");
+			return -1;
+		}
+		*journal_devnum = arg;
+	} else if (token == Opt_journal_ioprio) {
+		if (arg > 7) {
+			ext4_msg(sb, KERN_ERR, "Invalid journal IO priority"
+				 " (must be 0-7)");
+			return -1;
+		}
+		*journal_ioprio =
+			IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, arg);
+	} else if (m->flags & MOPT_DATAJ) {
+		if (is_remount) {
+			if (!sbi->s_journal)
+				ext4_msg(sb, KERN_WARNING, "Remounting file system with no journal so ignoring journalled data option");
+			else if (test_opt(sb, DATA_FLAGS) != m->mount_opt) {
 				ext4_msg(sb, KERN_ERR,
-					 "Invalid journal IO priority"
-					 " (must be 0-7)");
-				return -1;
-			}
-			*journal_ioprio =
-				IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, arg);
-		} else if (m->flags & MOPT_DATAJ) {
-			if (is_remount) {
-				if (!sbi->s_journal)
-					ext4_msg(sb, KERN_WARNING, "Remounting file system with no journal so ignoring journalled data option");
-				else if (test_opt(sb, DATA_FLAGS) !=
-					 m->mount_opt) {
-					ext4_msg(sb, KERN_ERR,
 					 "Cannot change data mode on remount");
-					return -1;
-				}
-			} else {
-				clear_opt(sb, DATA_FLAGS);
-				sbi->s_mount_opt |= m->mount_opt;
-			}
-#ifdef CONFIG_QUOTA
-		} else if (m->flags & MOPT_QFMT) {
-			if (sb_any_quota_loaded(sb) &&
-			    sbi->s_jquota_fmt != m->mount_opt) {
-				ext4_msg(sb, KERN_ERR, "Cannot "
-					 "change journaled quota options "
-					 "when quota turned on");
 				return -1;
 			}
-			sbi->s_jquota_fmt = m->mount_opt;
-#endif
 		} else {
-			if (!args->from)
-				arg = 1;
-			if (m->flags & MOPT_CLEAR)
-				arg = !arg;
-			else if (unlikely(!(m->flags & MOPT_SET))) {
-				ext4_msg(sb, KERN_WARNING,
-					 "buggy handling of option %s", opt);
-				WARN_ON(1);
-				return -1;
-			}
-			if (arg != 0)
-				sbi->s_mount_opt |= m->mount_opt;
-			else
-				sbi->s_mount_opt &= ~m->mount_opt;
+			clear_opt(sb, DATA_FLAGS);
+			sbi->s_mount_opt |= m->mount_opt;
 		}
-		return 1;
+#ifdef CONFIG_QUOTA
+	} else if (m->flags & MOPT_QFMT) {
+		if (sb_any_quota_loaded(sb) &&
+		    sbi->s_jquota_fmt != m->mount_opt) {
+			ext4_msg(sb, KERN_ERR, "Cannot change journaled "
+				 "quota options when quota turned on");
+			return -1;
+		}
+		sbi->s_jquota_fmt = m->mount_opt;
+#endif
+	} else {
+		if (!args->from)
+			arg = 1;
+		if (m->flags & MOPT_CLEAR)
+			arg = !arg;
+		else if (unlikely(!(m->flags & MOPT_SET))) {
+			ext4_msg(sb, KERN_WARNING,
+				 "buggy handling of option %s", opt);
+			WARN_ON(1);
+			return -1;
+		}
+		if (arg != 0)
+			sbi->s_mount_opt |= m->mount_opt;
+		else
+			sbi->s_mount_opt &= ~m->mount_opt;
 	}
-	ext4_msg(sb, KERN_ERR, "Unrecognized mount option \"%s\" "
-		 "or missing value", opt);
-	return -1;
+	return 1;
 }
 
 static int parse_options(char *options, struct super_block *sb,
-- 
1.7.12.rc0.22.gcdd159b


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] ext4: print error when argument of inode_readahead_blk is invalid
  2013-02-03  4:44 [PATCH 0/4] Clean up ext4's mount option parsing code Theodore Ts'o
  2013-02-03  4:44 ` [PATCH 1/4] ext4: move several mount options to standard handling loop Theodore Ts'o
  2013-02-03  4:44 ` [PATCH 2/4] ext4: make mount option parsing loop more logical Theodore Ts'o
@ 2013-02-03  4:44 ` Theodore Ts'o
  2013-02-03  4:44 ` [PATCH 4/4] ext4: check incompatible mount options while mounting ext2/3 Theodore Ts'o
  3 siblings, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2013-02-03  4:44 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Jan Kara, Theodore Ts'o

From: Jan Kara <jack@suse.cz>

If argument of inode_readahead_blk is too big, we just bail out
without printing any error. Fix this since it could confuse users.

Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/super.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e8d3c2f..bcd5f12 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1558,11 +1558,10 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 	} else if (token == Opt_min_batch_time) {
 		sbi->s_min_batch_time = arg;
 	} else if (token == Opt_inode_readahead_blks) {
-		if (arg > (1 << 30))
-			return -1;
-		if (arg && !is_power_of_2(arg)) {
-			ext4_msg(sb, KERN_ERR, "EXT4-fs: inode_readahead_blks"
-				 " must be a power of 2");
+		if (arg && (arg > (1 << 30) || !is_power_of_2(arg))) {
+			ext4_msg(sb, KERN_ERR,
+				 "EXT4-fs: inode_readahead_blks must be "
+				 "0 or a power of 2 smaller than 2^31");
 			return -1;
 		}
 		sbi->s_inode_readahead_blks = arg;
-- 
1.7.12.rc0.22.gcdd159b


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] ext4: check incompatible mount options while mounting ext2/3
  2013-02-03  4:44 [PATCH 0/4] Clean up ext4's mount option parsing code Theodore Ts'o
                   ` (2 preceding siblings ...)
  2013-02-03  4:44 ` [PATCH 3/4] ext4: print error when argument of inode_readahead_blk is invalid Theodore Ts'o
@ 2013-02-03  4:44 ` Theodore Ts'o
  3 siblings, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2013-02-03  4:44 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Check for incompatible mount options when using the ext4 file system
driver to mount ext2 or ext3 file systems.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/super.c | 47 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index bcd5f12..b132df5 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1409,6 +1409,9 @@ static int clear_qf_name(struct super_block *sb, int qtype)
 #define MOPT_QFMT	MOPT_NOSUPPORT
 #endif
 #define MOPT_DATAJ	0x0080
+#define MOPT_NO_EXT2	0x0100
+#define MOPT_NO_EXT3	0x0200
+#define MOPT_EXT4_ONLY	(MOPT_NO_EXT2 | MOPT_NO_EXT3)
 
 static const struct mount_opts {
 	int	token;
@@ -1421,21 +1424,29 @@ static const struct mount_opts {
 	{Opt_nogrpid, EXT4_MOUNT_GRPID, MOPT_CLEAR},
 	{Opt_block_validity, EXT4_MOUNT_BLOCK_VALIDITY, MOPT_SET},
 	{Opt_noblock_validity, EXT4_MOUNT_BLOCK_VALIDITY, MOPT_CLEAR},
-	{Opt_dioread_nolock, EXT4_MOUNT_DIOREAD_NOLOCK, MOPT_SET},
-	{Opt_dioread_lock, EXT4_MOUNT_DIOREAD_NOLOCK, MOPT_CLEAR},
+	{Opt_dioread_nolock, EXT4_MOUNT_DIOREAD_NOLOCK,
+	 MOPT_EXT4_ONLY | MOPT_SET},
+	{Opt_dioread_lock, EXT4_MOUNT_DIOREAD_NOLOCK,
+	 MOPT_EXT4_ONLY | MOPT_CLEAR},
 	{Opt_discard, EXT4_MOUNT_DISCARD, MOPT_SET},
 	{Opt_nodiscard, EXT4_MOUNT_DISCARD, MOPT_CLEAR},
-	{Opt_delalloc, EXT4_MOUNT_DELALLOC, MOPT_SET | MOPT_EXPLICIT},
-	{Opt_nodelalloc, EXT4_MOUNT_DELALLOC, MOPT_CLEAR | MOPT_EXPLICIT},
-	{Opt_journal_checksum, EXT4_MOUNT_JOURNAL_CHECKSUM, MOPT_SET},
+	{Opt_delalloc, EXT4_MOUNT_DELALLOC,
+	 MOPT_EXT4_ONLY | MOPT_SET | MOPT_EXPLICIT},
+	{Opt_nodelalloc, EXT4_MOUNT_DELALLOC,
+	 MOPT_EXT4_ONLY | MOPT_CLEAR | MOPT_EXPLICIT},
+	{Opt_journal_checksum, EXT4_MOUNT_JOURNAL_CHECKSUM,
+	 MOPT_EXT4_ONLY | MOPT_SET},
 	{Opt_journal_async_commit, (EXT4_MOUNT_JOURNAL_ASYNC_COMMIT |
-				    EXT4_MOUNT_JOURNAL_CHECKSUM), MOPT_SET},
-	{Opt_noload, EXT4_MOUNT_NOLOAD, MOPT_SET},
+				    EXT4_MOUNT_JOURNAL_CHECKSUM),
+	 MOPT_EXT4_ONLY | MOPT_SET},
+	{Opt_noload, EXT4_MOUNT_NOLOAD, MOPT_NO_EXT2 | MOPT_SET},
 	{Opt_err_panic, EXT4_MOUNT_ERRORS_PANIC, MOPT_SET | MOPT_CLEAR_ERR},
 	{Opt_err_ro, EXT4_MOUNT_ERRORS_RO, MOPT_SET | MOPT_CLEAR_ERR},
 	{Opt_err_cont, EXT4_MOUNT_ERRORS_CONT, MOPT_SET | MOPT_CLEAR_ERR},
-	{Opt_data_err_abort, EXT4_MOUNT_DATA_ERR_ABORT, MOPT_SET},
-	{Opt_data_err_ignore, EXT4_MOUNT_DATA_ERR_ABORT, MOPT_CLEAR},
+	{Opt_data_err_abort, EXT4_MOUNT_DATA_ERR_ABORT,
+	 MOPT_NO_EXT2 | MOPT_SET},
+	{Opt_data_err_ignore, EXT4_MOUNT_DATA_ERR_ABORT,
+	 MOPT_NO_EXT2 | MOPT_CLEAR},
 	{Opt_barrier, EXT4_MOUNT_BARRIER, MOPT_SET},
 	{Opt_nobarrier, EXT4_MOUNT_BARRIER, MOPT_CLEAR},
 	{Opt_noauto_da_alloc, EXT4_MOUNT_NO_AUTO_DA_ALLOC, MOPT_SET},
@@ -1451,9 +1462,10 @@ static const struct mount_opts {
 	{Opt_resgid, 0, MOPT_GTE0},
 	{Opt_journal_dev, 0, MOPT_GTE0},
 	{Opt_journal_ioprio, 0, MOPT_GTE0},
-	{Opt_data_journal, EXT4_MOUNT_JOURNAL_DATA, MOPT_DATAJ},
-	{Opt_data_ordered, EXT4_MOUNT_ORDERED_DATA, MOPT_DATAJ},
-	{Opt_data_writeback, EXT4_MOUNT_WRITEBACK_DATA, MOPT_DATAJ},
+	{Opt_data_journal, EXT4_MOUNT_JOURNAL_DATA, MOPT_NO_EXT2 | MOPT_DATAJ},
+	{Opt_data_ordered, EXT4_MOUNT_ORDERED_DATA, MOPT_NO_EXT2 | MOPT_DATAJ},
+	{Opt_data_writeback, EXT4_MOUNT_WRITEBACK_DATA,
+	 MOPT_NO_EXT2 | MOPT_DATAJ},
 	{Opt_user_xattr, EXT4_MOUNT_XATTR_USER, MOPT_SET},
 	{Opt_nouser_xattr, EXT4_MOUNT_XATTR_USER, MOPT_CLEAR},
 #ifdef CONFIG_EXT4_FS_POSIX_ACL
@@ -1531,6 +1543,17 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 		return -1;
 	}
 
+	if ((m->flags & MOPT_NO_EXT2) && IS_EXT2_SB(sb)) {
+		ext4_msg(sb, KERN_ERR,
+			 "Mount option \"%s\" incompatible with ext2", opt);
+		return -1;
+	}
+	if ((m->flags & MOPT_NO_EXT3) && IS_EXT3_SB(sb)) {
+		ext4_msg(sb, KERN_ERR,
+			 "Mount option \"%s\" incompatible with ext3", opt);
+		return -1;
+	}
+
 	if (args->from && match_int(args, &arg))
 		return -1;
 	if (args->from && (m->flags & MOPT_GTE0) && (arg < 0))
-- 
1.7.12.rc0.22.gcdd159b


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-02-03  4:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-03  4:44 [PATCH 0/4] Clean up ext4's mount option parsing code Theodore Ts'o
2013-02-03  4:44 ` [PATCH 1/4] ext4: move several mount options to standard handling loop Theodore Ts'o
2013-02-03  4:44 ` [PATCH 2/4] ext4: make mount option parsing loop more logical Theodore Ts'o
2013-02-03  4:44 ` [PATCH 3/4] ext4: print error when argument of inode_readahead_blk is invalid Theodore Ts'o
2013-02-03  4:44 ` [PATCH 4/4] ext4: check incompatible mount options while mounting ext2/3 Theodore Ts'o
  -- strict thread matches above, loose matches on Subject: below --
2012-04-16 12:25 [PATCH 0/4] ext4: Fix regression in mount option parsing and a small cleanup Jan Kara
2012-04-16 12:25 ` [PATCH 2/4] ext4: Make mount option parsing loop more logical Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).