linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] E2fsprogs: tune2fs: use an ioctl to update mounted fs
@ 2025-09-17  3:28 Theodore Ts'o
  2025-09-17  3:28 ` [PATCH 1/3] tune2fs: reorganize command-line flag handling Theodore Ts'o
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Theodore Ts'o @ 2025-09-17  3:28 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Teach tune2fs to try use the new EXT4_IOC_SET_TUNE_SB_PARAM ioctl
interface to update mounted file systems.  This will allow us to
disallow read/write access to the block device while the file system
is mounted, once we are sure the updated e2fsprogs is in use.

Theodore Ts'o (3):
  tune2fs: reorganize command-line flag handling
  tune2fs: rework parse_extended_opts() so it only parses the option
    string
  tune2fs: try to use the SET_TUNE_SB_PARAM ioctl on mounted file
    systems

 misc/tune2fs.c | 763 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 496 insertions(+), 267 deletions(-)

-- 
2.51.0


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

* [PATCH 1/3] tune2fs: reorganize command-line flag handling
  2025-09-17  3:28 [PATCH 0/3] E2fsprogs: tune2fs: use an ioctl to update mounted fs Theodore Ts'o
@ 2025-09-17  3:28 ` Theodore Ts'o
  2025-09-17  3:28 ` [PATCH 2/3] tune2fs: rework parse_extended_opts() so it only parses the option string Theodore Ts'o
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Theodore Ts'o @ 2025-09-17  3:28 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Instead of using individual ad-hoc variables indicating whether a
particular superblock value has been requested to be changed (e.g.,
c_flag, C_flag, et.al) use an array of booleans with indexes that are
defined with more human-readable #define's (e.g., OPT_MAX_MOUNTCOUNT).

There should be no behavioral changes from this code restructuring.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 misc/tune2fs.c | 212 +++++++++++++++++++++++++++----------------------
 1 file changed, 117 insertions(+), 95 deletions(-)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 3db57632..1b3716e1 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -103,16 +103,37 @@ struct fsuuid {
 
 extern int ask_yn(const char *string, int def);
 
+#define OPT_MAX_MOUNTCOUNT	 1
+#define OPT_MOUNTCOUNT		 2
+#define OPT_CHECKINTERVAL	 3
+#define OPT_CHECKTIME		 4
+#define OPT_ERROR_BEHAVIOR	 5
+#define OPT_RESUID		 6
+#define OPT_RESGID		 7
+#define OPT_RESERVED_BLOCKS	 8
+#define OPT_RESERVED_RATIO	 9
+#define OPT_INODE_SIZE	 	10
+#define OPT_LABEL		11
+#define OPT_PRINT_LABEL		12
+#define OPT_UUID		13
+#define OPT_LAST_MOUNTED	14
+#define OPT_SPARSE_SUPER	15
+#define OPT_QUOTA		16
+#define OPT_JOURNAL_SIZE	17
+#define OPT_JOURNAL_OPTS	18
+#define OPT_MNTOPTS		19
+#define OPT_FEATURES		20
+#define OPT_EXTENDED_CMD	21
+#define MAX_OPTS		22
+static bool opts[MAX_OPTS];
+
 const char *program_name = "tune2fs";
 char *device_name;
 char *new_label, *new_last_mounted, *requested_uuid;
 char *io_options;
-static int c_flag, C_flag, e_flag, f_flag, g_flag, i_flag, l_flag, L_flag;
-static int m_flag, M_flag, Q_flag, r_flag, s_flag = -1, u_flag, U_flag, T_flag;
-static int I_flag;
+static int force, do_list_super, sparse_value = -1;
 static int clear_mmp;
 static time_t last_check_time;
-static int print_label;
 static int max_mount_count, mount_count, mount_flags;
 static unsigned long interval;
 static blk64_t reserved_blocks;
@@ -237,6 +258,19 @@ static __u32 clear_ok_features[3] = {
 		EXT4_FEATURE_RO_COMPAT_READONLY
 };
 
+/**
+ * Return true if there is at least one superblock change requested
+ */
+static bool tune_opts_requested()
+{
+	int	i;
+
+	for (i = 0; i < MAX_OPTS; i++)
+		if (opts[i])
+			return true;
+	return false;
+}
+
 /**
  * Try to get journal super block if any
  */
@@ -293,7 +327,7 @@ static int remove_journal_device(ext2_filsys fs)
 	int		commit_remove_journal = 0;
 	io_manager	io_ptr;
 
-	if (f_flag)
+	if (force)
 		commit_remove_journal = 1; /* force removal even if error */
 
 	uuid_unparse(fs->super->s_journal_uuid, buf);
@@ -1204,7 +1238,7 @@ static int update_feature_set(ext2_filsys fs, char *features)
 			return 1;
 		}
 		if (ext2fs_has_feature_journal_needs_recovery(sb) &&
-		    f_flag < 2) {
+		    force < 2) {
 			fputs(_("The needs_recovery flag is set.  "
 				"Please run e2fsck before clearing\n"
 				"the has_journal flag.\n"), stderr);
@@ -1228,7 +1262,7 @@ static int update_feature_set(ext2_filsys fs, char *features)
 				"when the filesystem is unmounted.\n"), stderr);
 			return 1;
 		}
-		if (ext2fs_has_feature_orphan_present(sb) && f_flag < 2) {
+		if (ext2fs_has_feature_orphan_present(sb) && force < 2) {
 			fputs(_("The orphan_present feature is set. Please "
 				"run e2fsck before clearing orphan_file "
 				"feature.\n"),
@@ -1551,8 +1585,8 @@ mmp_error:
 		 * Set the Q_flag here and handle the quota options in the code
 		 * below.
 		 */
-		if (!Q_flag) {
-			Q_flag = 1;
+		if (!opts[OPT_QUOTA]) {
+			opts[OPT_QUOTA] = 1;
 			/* Enable usr/grp quota by default */
 			for (qtype = 0; qtype < MAXQUOTAS; qtype++) {
 				if (qtype != PRJQUOTA)
@@ -1571,26 +1605,26 @@ mmp_error:
 					  "inode size too small.\n"));
 			return 1;
 		}
-		Q_flag = 1;
+		opts[OPT_QUOTA] = true;
 		quota_enable[PRJQUOTA] = QOPT_ENABLE;
 	}
 
 	if (FEATURE_OFF(E2P_FEATURE_RO_INCOMPAT,
 			EXT4_FEATURE_RO_COMPAT_PROJECT)) {
-		Q_flag = 1;
+		opts[OPT_QUOTA] = true;
 		quota_enable[PRJQUOTA] = QOPT_DISABLE;
 	}
 
 	if (FEATURE_OFF(E2P_FEATURE_RO_INCOMPAT,
 				EXT4_FEATURE_RO_COMPAT_QUOTA)) {
 		/*
-		 * Set the Q_flag here and handle the quota options in the code
-		 * below.
+		 * Set the quota flag here and handle the quota
+		 * options in the code below.
 		 */
-		if (Q_flag)
+		if (opts[OPT_QUOTA])
 			fputs(_("\nWarning: '^quota' option overrides '-Q'"
 				"arguments.\n"), stderr);
-		Q_flag = 1;
+	        opts[OPT_QUOTA] = true;
 		/* Disable all quota by default */
 		for (qtype = 0; qtype < MAXQUOTAS; qtype++)
 			quota_enable[qtype] = QOPT_DISABLE;
@@ -1940,10 +1974,10 @@ static void parse_e2label_options(int argc, char ** argv)
 	open_flag = EXT2_FLAG_JOURNAL_DEV_OK | EXT2_FLAG_SUPER_ONLY;
 	if (argc == 3) {
 		open_flag |= EXT2_FLAG_RW;
-		L_flag = 1;
+		opts[OPT_LABEL] = true;
 		new_label = argv[2];
 	} else
-		print_label++;
+		opts[OPT_PRINT_LABEL] = true;
 }
 
 static time_t parse_time(char *str)
@@ -1990,8 +2024,7 @@ static void parse_tune2fs_options(int argc, char **argv)
 	while ((c = getopt(argc, argv, optstring)) != EOF)
 		switch (c) {
 		case 'c':
-			open_flag = EXT2_FLAG_RW;
-			c_flag = 1;
+			opts[OPT_MAX_MOUNTCOUNT] = true;
 			if (strcmp(optarg, "random") == 0) {
 				max_mount_count = 65536;
 				break;
@@ -2008,6 +2041,7 @@ static void parse_tune2fs_options(int argc, char **argv)
 				max_mount_count = -1;
 			break;
 		case 'C':
+			opts[OPT_MOUNTCOUNT] = true;
 			mount_count = strtoul(optarg, &tmp, 0);
 			if (*tmp || mount_count > 16000) {
 				com_err(program_name, 0,
@@ -2015,10 +2049,9 @@ static void parse_tune2fs_options(int argc, char **argv)
 					optarg);
 				usage();
 			}
-			C_flag = 1;
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'e':
+			opts[OPT_ERROR_BEHAVIOR] = true;
 			if (strcmp(optarg, "continue") == 0)
 				errors = EXT2_ERRORS_CONTINUE;
 			else if (strcmp(optarg, "remount-ro") == 0)
@@ -2031,17 +2064,16 @@ static void parse_tune2fs_options(int argc, char **argv)
 					optarg);
 				usage();
 			}
-			e_flag = 1;
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'E':
+			opts[OPT_EXTENDED_CMD] = true;
 			extended_cmd = optarg;
-			open_flag |= EXT2_FLAG_RW;
 			break;
 		case 'f': /* Force */
-			f_flag++;
+			force++;
 			break;
 		case 'g':
+			opts[OPT_RESGID] = true;
 			resgid = strtoul(optarg, &tmp, 0);
 			if (*tmp) {
 				gr = getgrnam(optarg);
@@ -2058,10 +2090,9 @@ static void parse_tune2fs_options(int argc, char **argv)
 					optarg);
 				usage();
 			}
-			g_flag = 1;
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'i':
+			opts[OPT_CHECKINTERVAL] = 1;
 			interval = strtoul(optarg, &tmp, 0);
 			switch (*tmp) {
 			case 's':
@@ -2090,28 +2121,25 @@ static void parse_tune2fs_options(int argc, char **argv)
 					_("bad interval - %s"), optarg);
 				usage();
 			}
-			i_flag = 1;
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'j':
+			opts[OPT_JOURNAL_SIZE] = true;
 			if (!journal_size)
 				journal_size = -1;
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'J':
+			opts[OPT_JOURNAL_OPTS] = true;
 			parse_journal_opts(optarg);
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'l':
-			l_flag = 1;
+			do_list_super = 1;
 			break;
 		case 'L':
+			opts[OPT_LABEL] = true;
 			new_label = optarg;
-			L_flag = 1;
-			open_flag |= EXT2_FLAG_RW |
-				EXT2_FLAG_JOURNAL_DEV_OK;
 			break;
 		case 'm':
+			opts[OPT_RESERVED_RATIO] = true;
 			reserved_ratio = strtod(optarg, &tmp);
 			if (*tmp || reserved_ratio > 50 ||
 			    reserved_ratio < 0) {
@@ -2120,40 +2148,37 @@ static void parse_tune2fs_options(int argc, char **argv)
 					optarg);
 				usage();
 			}
-			m_flag = 1;
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'M':
+			opts[OPT_LAST_MOUNTED] = true;
 			new_last_mounted = optarg;
-			M_flag = 1;
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'o':
+			opts[OPT_MNTOPTS] = true;
 			if (mntopts_cmd) {
 				com_err(program_name, 0, "%s",
 					_("-o may only be specified once"));
 				usage();
 			}
 			mntopts_cmd = optarg;
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'O':
+			opts[OPT_FEATURES] = true;
 			if (features_cmd) {
 				com_err(program_name, 0, "%s",
 					_("-O may only be specified once"));
 				usage();
 			}
 			features_cmd = optarg;
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'Q':
-			Q_flag = 1;
+			opts[OPT_QUOTA] = true;
 			ret = parse_quota_opts(optarg, option_handle_function);
 			if (ret)
 				exit(1);
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'r':
+			opts[OPT_RESERVED_BLOCKS] = true;
 			reserved_blocks = strtoul(optarg, &tmp, 0);
 			if (*tmp) {
 				com_err(program_name, 0,
@@ -2161,45 +2186,39 @@ static void parse_tune2fs_options(int argc, char **argv)
 					optarg);
 				usage();
 			}
-			r_flag = 1;
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 's': /* Deprecated */
-			s_flag = atoi(optarg);
-			open_flag = EXT2_FLAG_RW;
+			opts[OPT_SPARSE_SUPER] = true;
+			sparse_value = atoi(optarg);
 			break;
 		case 'T':
-			T_flag = 1;
+			opts[OPT_CHECKTIME] = true;
 			last_check_time = parse_time(optarg);
-			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'u':
-				resuid = strtoul(optarg, &tmp, 0);
-				if (*tmp) {
-					pw = getpwnam(optarg);
-					if (pw == NULL)
-						tmp = optarg;
-					else {
-						resuid = pw->pw_uid;
-						*tmp = 0;
-					}
+			opts[OPT_RESUID] = true;
+			resuid = strtoul(optarg, &tmp, 0);
+			if (*tmp) {
+				pw = getpwnam(optarg);
+				if (pw == NULL)
+					tmp = optarg;
+				else {
+					resuid = pw->pw_uid;
+					*tmp = 0;
 				}
-				if (*tmp) {
-					com_err(program_name, 0,
-						_("bad uid/user name - %s"),
-						optarg);
+			}
+			if (*tmp) {
+				com_err(program_name, 0,
+					_("bad uid/user name - %s"), optarg);
 					usage();
-				}
-				u_flag = 1;
-				open_flag = EXT2_FLAG_RW;
-				break;
+			}
+			break;
 		case 'U':
+			opts[OPT_UUID] = true;
 			requested_uuid = optarg;
-			U_flag = 1;
-			open_flag = EXT2_FLAG_RW |
-				EXT2_FLAG_JOURNAL_DEV_OK;
 			break;
 		case 'I':
+			opts[OPT_INODE_SIZE] = true;
 			new_inode_size = strtoul(optarg, &tmp, 0);
 			if (*tmp) {
 				com_err(program_name, 0,
@@ -2215,8 +2234,6 @@ static void parse_tune2fs_options(int argc, char **argv)
 					optarg);
 				usage();
 			}
-			open_flag = EXT2_FLAG_RW;
-			I_flag = 1;
 			break;
 		case 'z':
 			undo_file = optarg;
@@ -2226,8 +2243,13 @@ static void parse_tune2fs_options(int argc, char **argv)
 		}
 	if (optind < argc - 1 || optind == argc)
 		usage();
-	if (!open_flag && !l_flag)
+	if (tune_opts_requested()) {
+		open_flag = EXT2_FLAG_RW;
+		if (opts[OPT_LABEL] || opts[OPT_UUID])
+			open_flag |= EXT2_FLAG_JOURNAL_DEV_OK;
+	} else {
 		usage();
+	}
 	io_options = strchr(argv[optind], '?');
 	if (io_options)
 		*io_options++ = 0;
@@ -3221,8 +3243,8 @@ int tune2fs_main(int argc, char **argv)
 	 * Try the get/set fs label using ioctls before we even attempt
 	 * to open the file system.
 	 */
-	if (L_flag || print_label) {
-		rc = handle_fslabel(L_flag);
+	if (opts[OPT_LABEL] || opts[OPT_PRINT_LABEL]) {
+		rc = handle_fslabel(opts[OPT_LABEL]);
 		if (rc != -1) {
 #ifndef BUILD_AS_LIB
 			exit(rc);
@@ -3233,7 +3255,7 @@ int tune2fs_main(int argc, char **argv)
 	}
 
 retry_open:
-	if ((open_flag & EXT2_FLAG_RW) == 0 || f_flag)
+	if ((open_flag & EXT2_FLAG_RW) == 0 || force)
 		open_flag |= EXT2_FLAG_SKIP_MMP;
 
 	open_flag |= EXT2_FLAG_64BITS | EXT2_FLAG_THREADS |
@@ -3276,7 +3298,7 @@ retry_open:
 	}
 	fs->default_bitmap_type = EXT2FS_BMAP64_RBTREE;
 
-	if (I_flag) {
+	if (opts[OPT_INODE_SIZE]) {
 		/*
 		 * Check the inode size is right so we can issue an
 		 * error message and bail before setting up the tdb
@@ -3328,7 +3350,7 @@ retry_open:
 	sb = fs->super;
 	fs->flags &= ~EXT2_FLAG_MASTER_SB_ONLY;
 
-	if (print_label) {
+	if (opts[OPT_PRINT_LABEL]) {
 		/* For e2label emulation */
 		printf("%.*s\n", EXT2_LEN_STR(sb->s_volume_name));
 		remove_error_table(&et_ext2_error_table);
@@ -3378,7 +3400,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 	/* Normally we only need to write out the superblock */
 	fs->flags |= EXT2_FLAG_SUPER_ONLY;
 
-	if (c_flag) {
+	if (opts[OPT_MAX_MOUNTCOUNT]) {
 		if (max_mount_count == 65536)
 			max_mount_count = EXT2_DFL_MAX_MNT_COUNT +
 				(random() % EXT2_DFL_MAX_MNT_COUNT);
@@ -3387,17 +3409,17 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 		printf(_("Setting maximal mount count to %d\n"),
 		       max_mount_count);
 	}
-	if (C_flag) {
+	if (opts[OPT_MOUNTCOUNT]) {
 		sb->s_mnt_count = mount_count;
 		ext2fs_mark_super_dirty(fs);
 		printf(_("Setting current mount count to %d\n"), mount_count);
 	}
-	if (e_flag) {
+	if (opts[OPT_ERROR_BEHAVIOR]) {
 		sb->s_errors = errors;
 		ext2fs_mark_super_dirty(fs);
 		printf(_("Setting error behavior to %d\n"), errors);
 	}
-	if (g_flag) {
+	if (opts[OPT_RESGID]) {
 		if (sb->s_def_resgid != resgid) {
 			sb->s_def_resgid = resgid;
 			ext2fs_mark_super_dirty(fs);
@@ -3406,7 +3428,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 			printf(_("Reserved blocks gid already set to %lu\n"), resgid);
 		}
 	}
-	if (i_flag) {
+	if (opts[OPT_CHECKINTERVAL]) {
 		if ((unsigned long long)interval >= (1ULL << 32)) {
 			com_err(program_name, 0,
 				_("interval between checks is too big (%lu)"),
@@ -3419,7 +3441,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 		printf(_("Setting interval between checks to %lu seconds\n"),
 		       interval);
 	}
-	if (m_flag) {
+	if (opts[OPT_RESERVED_RATIO]) {
 		ext2fs_r_blocks_count_set(sb, reserved_ratio *
 					  ext2fs_blocks_count(sb) / 100.0);
 		ext2fs_mark_super_dirty(fs);
@@ -3427,7 +3449,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 			reserved_ratio,
 			(unsigned long long) ext2fs_r_blocks_count(sb));
 	}
-	if (r_flag) {
+	if (opts[OPT_RESERVED_BLOCKS]) {
 		if (reserved_blocks > ext2fs_blocks_count(sb)/2) {
 			com_err(program_name, 0,
 				_("reserved blocks count is too big (%llu)"),
@@ -3440,7 +3462,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 		printf(_("Setting reserved blocks count to %llu\n"),
 		       (unsigned long long) reserved_blocks);
 	}
-	if (s_flag == 1) {
+	if (sparse_value == 1) {
 		if (ext2fs_has_feature_sparse_super(sb)) {
 			fputs(_("\nThe filesystem already has sparse "
 				"superblocks.\n"), stderr);
@@ -3459,24 +3481,24 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 			       _(please_fsck));
 		}
 	}
-	if (s_flag == 0) {
+	if (sparse_value == 0) {
 		fputs(_("\nClearing the sparse superblock flag not supported.\n"),
 		      stderr);
 		rc = 1;
 		goto closefs;
 	}
-	if (T_flag) {
+	if (opts[OPT_CHECKTIME]) {
 		ext2fs_set_tstamp(sb, s_lastcheck, last_check_time);
 		ext2fs_mark_super_dirty(fs);
 		printf(_("Setting time filesystem last checked to %s\n"),
 		       ctime(&last_check_time));
 	}
-	if (u_flag) {
+	if (opts[OPT_RESUID]) {
 		sb->s_def_resuid = resuid;
 		ext2fs_mark_super_dirty(fs);
 		printf(_("Setting reserved blocks uid to %lu\n"), resuid);
 	}
-	if (L_flag) {
+	if (opts[OPT_LABEL]) {
 		if (strlen(new_label) > sizeof(sb->s_volume_name))
 			fputs(_("Warning: label too long, truncating.\n"),
 			      stderr);
@@ -3485,7 +3507,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 			sizeof(sb->s_volume_name));
 		ext2fs_mark_super_dirty(fs);
 	}
-	if (M_flag) {
+	if (opts[OPT_LAST_MOUNTED]) {
 		memset(sb->s_last_mounted, 0, sizeof(sb->s_last_mounted));
 		strncpy((char *)sb->s_last_mounted, new_last_mounted,
 			sizeof(sb->s_last_mounted));
@@ -3505,7 +3527,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 		rc = parse_extended_opts(fs, extended_cmd);
 		if (rc)
 			goto closefs;
-		if (clear_mmp && !f_flag) {
+		if (clear_mmp && !force) {
 			fputs(_("Error in using clear_mmp. "
 				"It must be used with -f\n"),
 			      stderr);
@@ -3541,7 +3563,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 		}
 	}
 
-	if (Q_flag) {
+	if (opts[OPT_QUOTA]) {
 		if (mount_flags & (EXT2_MF_BUSY | EXT2_MF_MOUNTED)) {
 			fputs(_("The quota feature may only be changed when "
 				"the filesystem is unmounted and not in use.\n"), stderr);
@@ -3553,7 +3575,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 			goto closefs;
 	}
 
-	if (U_flag) {
+	if (opts[OPT_UUID]) {
 		int set_csum = 0;
 		dgrp_t i;
 		char buf[SUPERBLOCK_SIZE] __attribute__ ((aligned(8)));
@@ -3694,7 +3716,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 		}
 	}
 
-	if (I_flag) {
+	if (opts[OPT_INODE_SIZE]) {
 		if (mount_flags & (EXT2_MF_BUSY | EXT2_MF_MOUNTED)) {
 			fputs(_("The inode size may only be "
 				"changed when the filesystem is "
@@ -3740,7 +3762,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 		}
 	}
 
-	if (l_flag)
+	if (do_list_super)
 		list_super(sb);
 	if (stride_set) {
 		sb->s_raid_stride = stride;
-- 
2.51.0


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

* [PATCH 2/3] tune2fs: rework parse_extended_opts() so it only parses the option string
  2025-09-17  3:28 [PATCH 0/3] E2fsprogs: tune2fs: use an ioctl to update mounted fs Theodore Ts'o
  2025-09-17  3:28 ` [PATCH 1/3] tune2fs: reorganize command-line flag handling Theodore Ts'o
@ 2025-09-17  3:28 ` Theodore Ts'o
  2025-09-17  6:15   ` Andreas Dilger
  2025-09-17  3:28 ` [PATCH 3/3] tune2fs: try to use the SET_TUNE_SB_PARAM ioctl on mounted file systems Theodore Ts'o
  2025-09-17  6:26 ` [PATCH 0/3] E2fsprogs: tune2fs: use an ioctl to update mounted fs Andreas Dilger
  3 siblings, 1 reply; 9+ messages in thread
From: Theodore Ts'o @ 2025-09-17  3:28 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

The parse_extended_opts() was doing two things: interpreting the
string passed into the command line and modifying the file system's
superblock.  Separate out the file system modification and move it out
from parse_extended_opts().

This allows the user to specify more than one -E command-line option,
and it also allows some of the file system changes to be modified via
an ioctl for a mounted file system.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 misc/tune2fs.c | 211 +++++++++++++++++++++++++++----------------------
 1 file changed, 118 insertions(+), 93 deletions(-)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 1b3716e1..e752c328 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -123,8 +123,19 @@ extern int ask_yn(const char *string, int def);
 #define OPT_JOURNAL_OPTS	18
 #define OPT_MNTOPTS		19
 #define OPT_FEATURES		20
-#define OPT_EXTENDED_CMD	21
-#define MAX_OPTS		22
+#define OPT_CLEAR_MMP		21
+#define OPT_MMP_INTERVAL	22
+#define OPT_FORCE_FSCK		23
+#define OPT_TEST_FS		24
+#define OPT_CLEAR_TEST_FS	25
+#define OPT_RAID_STRIDE		26
+#define OPT_RAID_STRIPE_WIDTH	27
+#define OPT_HASH_ALG		28
+#define OPT_MOUNT_OPTS		29
+#define OPT_ENCODING		30
+#define OPT_ENCODING_FLAGS	31
+#define OPT_ORPHAN_FILE_SIZE	32
+#define MAX_OPTS		33
 static bool opts[MAX_OPTS];
 
 const char *program_name = "tune2fs";
@@ -132,7 +143,6 @@ char *device_name;
 char *new_label, *new_last_mounted, *requested_uuid;
 char *io_options;
 static int force, do_list_super, sparse_value = -1;
-static int clear_mmp;
 static time_t last_check_time;
 static int max_mount_count, mount_count, mount_flags;
 static unsigned long interval;
@@ -140,12 +150,16 @@ static blk64_t reserved_blocks;
 static double reserved_ratio;
 static unsigned long resgid, resuid;
 static unsigned short errors;
+static unsigned long mmp_interval;
+static int hash_alg;
+static char *hash_alg_str;
+static int encoding;
+static __u16 encoding_flags;
+static char *encoding_str, *encoding_flags_str;
 static int open_flag;
 static char *features_cmd;
 static char *mntopts_cmd;
 static int stride, stripe_width;
-static int stride_set, stripe_width_set;
-static char *extended_cmd;
 static unsigned long new_inode_size;
 static char *ext_mount_opts;
 static int quota_enable[MAXQUOTAS];
@@ -153,7 +167,6 @@ static int rewrite_checksums;
 static int feature_64bit;
 static int fsck_requested;
 static char *undo_file;
-int enabling_casefold;
 
 int journal_size, journal_fc_size, journal_flags;
 char *journal_device;
@@ -184,6 +197,8 @@ void do_findfs(int argc, char **argv);
 int journal_enable_debug = -1;
 #endif
 
+static int parse_extended_opts(const char *ext_opts);
+
 static void usage(void)
 {
 	fprintf(stderr,
@@ -1645,7 +1660,6 @@ mmp_error:
 		}
 		fs->super->s_encoding = EXT4_ENC_UTF8_12_1;
 		fs->super->s_encoding_flags = e2p_get_encoding_flags(EXT4_ENC_UTF8_12_1);
-		enabling_casefold = 1;
 	}
 
 	if (FEATURE_OFF(E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_CASEFOLD)) {
@@ -1661,7 +1675,6 @@ mmp_error:
 		}
 		fs->super->s_encoding = 0;
 		fs->super->s_encoding_flags = 0;
-		enabling_casefold = 0;
 	}
 
 	if (FEATURE_ON(E2P_FEATURE_INCOMPAT,
@@ -2066,8 +2079,8 @@ static void parse_tune2fs_options(int argc, char **argv)
 			}
 			break;
 		case 'E':
-			opts[OPT_EXTENDED_CMD] = true;
-			extended_cmd = optarg;
+			if (parse_extended_opts(optarg))
+				exit(1);
 			break;
 		case 'f': /* Force */
 			force++;
@@ -2259,6 +2272,11 @@ static void parse_tune2fs_options(int argc, char **argv)
 			argv[optind]);
 		exit(1);
 	}
+	if (opts[OPT_ENCODING_FLAGS] && !opts[OPT_ENCODING]) {
+		fprintf(stderr, _("error: An encoding must be explicitly "
+				  "specified when passing encoding-flags\n"));
+		exit(1);
+	}
 }
 
 #ifdef CONFIG_BUILD_FINDFS
@@ -2282,23 +2300,22 @@ void do_findfs(int argc, char **argv)
 }
 #endif
 
-static int parse_extended_opts(ext2_filsys fs, const char *opts)
+#define member_size(type, member) (sizeof( ((type *)0)->member ))
+
+static int parse_extended_opts(const char *ext_opts)
 {
-	struct ext2_super_block *sb = fs->super;
 	char	*buf, *token, *next, *p, *arg;
-	int	len, hash_alg;
+	int	len;
 	int	r_usage = 0;
-	int encoding = 0;
-	char	*encoding_flags = NULL;
 
-	len = strlen(opts);
+	len = strlen(ext_opts);
 	buf = malloc(len+1);
 	if (!buf) {
 		fprintf(stderr, "%s",
 			_("Couldn't allocate memory to parse options!\n"));
 		return 1;
 	}
-	strcpy(buf, opts);
+	strcpy(buf, ext_opts);
 	for (token = buf; token && *token; token = next) {
 		p = strchr(token, ',');
 		next = 0;
@@ -2313,14 +2330,13 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
 		}
 		if (strcmp(token, "clear-mmp") == 0 ||
 		    strcmp(token, "clear_mmp") == 0) {
-			clear_mmp = 1;
+			opts[OPT_CLEAR_MMP] = true;
 		} else if (strcmp(token, "mmp_update_interval") == 0) {
-			unsigned long intv;
 			if (!arg) {
 				r_usage++;
 				continue;
 			}
-			intv = strtoul(arg, &p, 0);
+			mmp_interval = strtoul(arg, &p, 0);
 			if (*p) {
 				fprintf(stderr,
 					_("Invalid mmp_update_interval: %s\n"),
@@ -2328,34 +2344,22 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
 				r_usage++;
 				continue;
 			}
-			if (intv == 0) {
-				intv = EXT4_MMP_UPDATE_INTERVAL;
-			} else if (intv > EXT4_MMP_MAX_UPDATE_INTERVAL) {
+			if (mmp_interval == 0) {
+				mmp_interval = EXT4_MMP_UPDATE_INTERVAL;
+			} else if (mmp_interval > EXT4_MMP_MAX_UPDATE_INTERVAL) {
 				fprintf(stderr,
 					_("mmp_update_interval too big: %lu\n"),
-					intv);
+					mmp_interval);
 				r_usage++;
 				continue;
 			}
-			printf(P_("Setting multiple mount protection update "
-				  "interval to %lu second\n",
-				  "Setting multiple mount protection update "
-				  "interval to %lu seconds\n", intv),
-			       intv);
-			sb->s_mmp_update_interval = intv;
-			ext2fs_mark_super_dirty(fs);
+			opts[OPT_MMP_INTERVAL] = true;
 		} else if (!strcmp(token, "force_fsck")) {
-			sb->s_state |= EXT2_ERROR_FS;
-			printf(_("Setting filesystem error flag to force fsck.\n"));
-			ext2fs_mark_super_dirty(fs);
+			opts[OPT_FORCE_FSCK] = true;
 		} else if (!strcmp(token, "test_fs")) {
-			sb->s_flags |= EXT2_FLAGS_TEST_FILESYS;
-			printf("Setting test filesystem flag\n");
-			ext2fs_mark_super_dirty(fs);
+			opts[OPT_TEST_FS] = true;
 		} else if (!strcmp(token, "^test_fs")) {
-			sb->s_flags &= ~EXT2_FLAGS_TEST_FILESYS;
-			printf("Clearing test filesystem flag\n");
-			ext2fs_mark_super_dirty(fs);
+			opts[OPT_CLEAR_TEST_FS] = true;
 		} else if (strcmp(token, "stride") == 0) {
 			if (!arg) {
 				r_usage++;
@@ -2369,7 +2373,7 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
 				r_usage++;
 				continue;
 			}
-			stride_set = 1;
+			opts[OPT_RAID_STRIDE] = true;
 		} else if (strcmp(token, "stripe-width") == 0 ||
 			   strcmp(token, "stripe_width") == 0) {
 			if (!arg) {
@@ -2384,7 +2388,7 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
 				r_usage++;
 				continue;
 			}
-			stripe_width_set = 1;
+			opts[OPT_RAID_STRIPE_WIDTH] = true;
 		} else if (strcmp(token, "hash_alg") == 0 ||
 			   strcmp(token, "hash-alg") == 0) {
 			if (!arg) {
@@ -2399,21 +2403,21 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
 				r_usage++;
 				continue;
 			}
-			sb->s_def_hash_version = hash_alg;
-			printf(_("Setting default hash algorithm "
-				 "to %s (%d)\n"),
-			       arg, hash_alg);
-			ext2fs_mark_super_dirty(fs);
+			hash_alg_str = strdup(arg);
+			opts[OPT_HASH_ALG] = true;
 		} else if (!strcmp(token, "mount_opts")) {
 			if (!arg) {
 				r_usage++;
 				continue;
 			}
-			if (strlen(arg) >= sizeof(fs->super->s_mount_opts)) {
+			if (strlen(arg) >=
+			    member_size(struct ext2_super_block,
+					s_mount_opts)) {
 				fprintf(stderr,
 					"Extended mount options too long\n");
 				continue;
 			}
+			opts[OPT_MOUNT_OPTS] = true;
 			ext_mount_opts = strdup(arg);
 		} else if (!strcmp(token, "encoding")) {
 			if (!arg) {
@@ -2426,36 +2430,33 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
 				r_usage++;
 				continue;
 			}
-			if (ext2fs_has_feature_casefold(sb) && !enabling_casefold) {
-				fprintf(stderr, _("Cannot alter existing encoding\n"));
-				r_usage++;
-				continue;
-			}
 			encoding = e2p_str2encoding(arg);
 			if (encoding < 0) {
 				fprintf(stderr, _("Invalid encoding: %s\n"), arg);
 				r_usage++;
 				continue;
 			}
-			enabling_casefold = 1;
-			sb->s_encoding = encoding;
-			printf(_("Setting encoding to '%s'\n"), arg);
-			sb->s_encoding_flags =
-				e2p_get_encoding_flags(sb->s_encoding);
+			encoding_str = strdup(arg);
+			opts[OPT_ENCODING] = true;
 		} else if (!strcmp(token, "encoding_flags")) {
 			if (!arg) {
 				r_usage++;
 				continue;
 			}
-			encoding_flags = arg;
+			if (e2p_str2encoding_flags(EXT4_ENC_UTF8_12_1,
+						   arg, &encoding_flags)) {
+				fprintf(stderr,
+		_("error: Invalid encoding flag: %s\n"), arg);
+				r_usage++;
+			}
+			encoding_flags_str = strdup(arg);
+			opts[OPT_ENCODING_FLAGS] = true;
 		} else if (!strcmp(token, "orphan_file_size")) {
 			if (!arg) {
 				r_usage++;
 				continue;
 			}
-			orphan_file_blocks = parse_num_blocks2(arg,
-						 fs->super->s_log_block_size);
-
+			orphan_file_blocks = parse_num_blocks2(arg, 0);
 			if (orphan_file_blocks < 1) {
 				fprintf(stderr,
 					_("Invalid size of orphan file %s\n"),
@@ -2463,30 +2464,10 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
 				r_usage++;
 				continue;
 			}
+			opts[OPT_ORPHAN_FILE_SIZE] = true;
 		} else
 			r_usage++;
 	}
-
-	if (encoding > 0 && !r_usage) {
-		sb->s_encoding_flags =
-			e2p_get_encoding_flags(sb->s_encoding);
-
-		if (encoding_flags &&
-		    e2p_str2encoding_flags(sb->s_encoding, encoding_flags,
-					   &sb->s_encoding_flags)) {
-			fprintf(stderr, _("error: Invalid encoding flag: %s\n"),
-					encoding_flags);
-			r_usage++;
-		} else if (encoding_flags)
-			printf(_("Setting encoding_flags to '%s'\n"),
-				 encoding_flags);
-		ext2fs_set_feature_casefold(sb);
-		ext2fs_mark_super_dirty(fs);
-	} else if (encoding_flags && !r_usage) {
-		fprintf(stderr, _("error: An encoding must be explicitly "
-				  "specified when passing encoding-flags\n"));
-		r_usage++;
-	}
 	if (r_usage) {
 		fprintf(stderr, "%s", _("\nBad options specified.\n\n"
 			"Extended options are separated by commas, "
@@ -3518,27 +3499,64 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 		if (rc)
 			goto closefs;
 	}
+	if (ext2fs_has_feature_casefold(sb) && opts[OPT_ENCODING]) {
+		fprintf(stderr, _("Cannot alter existing encoding\n"));
+		rc = 1;
+		goto closefs;
+	}
 	if (features_cmd) {
 		rc = update_feature_set(fs, features_cmd);
 		if (rc)
 			goto closefs;
 	}
-	if (extended_cmd) {
-		rc = parse_extended_opts(fs, extended_cmd);
-		if (rc)
-			goto closefs;
-		if (clear_mmp && !force) {
+	if (opts[OPT_CLEAR_MMP]) {
+		if (!force) {
 			fputs(_("Error in using clear_mmp. "
 				"It must be used with -f\n"),
 			      stderr);
 			rc = 1;
 			goto closefs;
 		}
-	}
-	if (clear_mmp) {
 		rc = ext2fs_mmp_clear(fs);
 		goto closefs;
 	}
+	if (opts[OPT_MMP_INTERVAL]) {
+		printf(P_("Setting multiple mount protection update "
+			  "interval to %lu second\n",
+			  "Setting multiple mount protection update "
+			  "interval to %lu seconds\n", mmp_interval),
+		       mmp_interval);
+		sb->s_mmp_update_interval = mmp_interval;
+		ext2fs_mark_super_dirty(fs);
+	}
+	if (opts[OPT_FORCE_FSCK]) {
+		sb->s_state |= EXT2_ERROR_FS;
+		printf(_("Setting filesystem error flag to force fsck.\n"));
+		ext2fs_mark_super_dirty(fs);
+	}
+	if (opts[OPT_TEST_FS]) {
+		sb->s_flags |= EXT2_FLAGS_TEST_FILESYS;
+		printf("Setting test filesystem flag\n");
+		ext2fs_mark_super_dirty(fs);
+	}
+	if (opts[OPT_CLEAR_TEST_FS]) {
+		sb->s_flags &= ~EXT2_FLAGS_TEST_FILESYS;
+		printf("Clearing test filesystem flag\n");
+		ext2fs_mark_super_dirty(fs);
+	}
+	if (opts[OPT_ENCODING]) {
+		ext2fs_set_feature_casefold(sb);
+		sb->s_encoding = encoding;
+		printf(_("Setting encoding to '%s'\n"), encoding_str);
+		if (opts[OPT_ENCODING_FLAGS]) {
+			sb->s_encoding_flags = encoding_flags;
+			printf(_("Setting encoding_flags to '%s'\n"),
+			       encoding_flags_str);
+		} else
+			sb->s_encoding_flags =
+				e2p_get_encoding_flags(sb->s_encoding);
+			ext2fs_mark_super_dirty(fs);
+	}
 	if (journal_size || journal_device) {
 		rc = add_journal(fs);
 		if (rc)
@@ -3554,6 +3572,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 			rc = 1;
 			goto closefs;
 		}
+		orphan_file_blocks >>= fs->super->s_log_block_size;
 		err = ext2fs_create_orphan_file(fs, orphan_file_blocks);
 		if (err) {
 			com_err(program_name, err, "%s",
@@ -3764,17 +3783,23 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 
 	if (do_list_super)
 		list_super(sb);
-	if (stride_set) {
+	if (opts[OPT_RAID_STRIDE]) {
 		sb->s_raid_stride = stride;
 		ext2fs_mark_super_dirty(fs);
 		printf(_("Setting stride size to %d\n"), stride);
 	}
-	if (stripe_width_set) {
+	if (opts[OPT_RAID_STRIPE_WIDTH]) {
 		sb->s_raid_stripe_width = stripe_width;
 		ext2fs_mark_super_dirty(fs);
 		printf(_("Setting stripe width to %d\n"), stripe_width);
 	}
-	if (ext_mount_opts) {
+	if (opts[OPT_HASH_ALG]) {
+		sb->s_def_hash_version = hash_alg;
+		printf(_("Setting default hash algorithm to %s (%d)\n"),
+		       hash_alg_str, hash_alg);
+		ext2fs_mark_super_dirty(fs);
+	}
+	if (opts[OPT_MOUNT_OPTS]) {
 		strncpy((char *)(fs->super->s_mount_opts), ext_mount_opts,
 			sizeof(fs->super->s_mount_opts));
 		fs->super->s_mount_opts[sizeof(fs->super->s_mount_opts)-1] = 0;
-- 
2.51.0


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

* [PATCH 3/3] tune2fs: try to use the SET_TUNE_SB_PARAM ioctl on mounted file systems
  2025-09-17  3:28 [PATCH 0/3] E2fsprogs: tune2fs: use an ioctl to update mounted fs Theodore Ts'o
  2025-09-17  3:28 ` [PATCH 1/3] tune2fs: reorganize command-line flag handling Theodore Ts'o
  2025-09-17  3:28 ` [PATCH 2/3] tune2fs: rework parse_extended_opts() so it only parses the option string Theodore Ts'o
@ 2025-09-17  3:28 ` Theodore Ts'o
  2025-09-18 17:47   ` Darrick J. Wong
  2025-09-17  6:26 ` [PATCH 0/3] E2fsprogs: tune2fs: use an ioctl to update mounted fs Andreas Dilger
  3 siblings, 1 reply; 9+ messages in thread
From: Theodore Ts'o @ 2025-09-17  3:28 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Try to uuse the new EXT4_IOC_GET_TUNE_SB_PARAM ioctl to update the
superblock if the file system is mounted.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 misc/tune2fs.c | 352 +++++++++++++++++++++++++++++++++++++------------
 1 file changed, 267 insertions(+), 85 deletions(-)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index e752c328..b1ec3991 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -101,6 +101,64 @@ struct fsuuid {
 #define EXT4_IOC_SETFSUUID	_IOW('f', 44, struct fsuuid)
 #endif
 
+#if (!defined(EXT4_IOC_GET_TUNE_SB_PARAM) && defined(__linux__))
+
+struct ext4_tune_sb_params {
+	__u32 set_flags;
+	__u32 checkinterval;
+	__u16 errors_behavior;
+	__u16 mnt_count;
+	__u16 max_mnt_count;
+	__u16 raid_stride;
+	__u64 last_check_time;
+	__u64 reserved_blocks;
+	__u64 blocks_count;
+	__u32 default_mnt_opts;
+	__u32 reserved_uid;
+	__u32 reserved_gid;
+	__u32 raid_stripe_width;
+	__u16 encoding;
+	__u16 encoding_flags;
+	__u8  def_hash_alg;
+	__u8  pad_1;
+	__u16 pad_2;
+	__u32 feature_compat;
+	__u32 feature_incompat;
+	__u32 feature_ro_compat;
+	__u32 set_feature_compat_mask;
+	__u32 set_feature_incompat_mask;
+	__u32 set_feature_ro_compat_mask;
+	__u32 clear_feature_compat_mask;
+	__u32 clear_feature_incompat_mask;
+	__u32 clear_feature_ro_compat_mask;
+	__u8  mount_opts[64];
+	__u8  pad[64];
+};
+
+#define EXT4_TUNE_FL_ERRORS_BEHAVIOR	0x00000001
+#define EXT4_TUNE_FL_MNT_COUNT		0x00000002
+#define EXT4_TUNE_FL_MAX_MNT_COUNT	0x00000004
+#define EXT4_TUNE_FL_CHECKINTRVAL	0x00000008
+#define EXT4_TUNE_FL_LAST_CHECK_TIME	0x00000010
+#define EXT4_TUNE_FL_RESERVED_BLOCKS	0x00000020
+#define EXT4_TUNE_FL_RESERVED_UID	0x00000040
+#define EXT4_TUNE_FL_RESERVED_GID	0x00000080
+#define EXT4_TUNE_FL_DEFAULT_MNT_OPTS	0x00000100
+#define EXT4_TUNE_FL_DEF_HASH_ALG	0x00000200
+#define EXT4_TUNE_FL_RAID_STRIDE	0x00000400
+#define EXT4_TUNE_FL_RAID_STRIPE_WIDTH	0x00000800
+#define EXT4_TUNE_FL_MOUNT_OPTS		0x00001000
+#define EXT4_TUNE_FL_FEATURES		0x00002000
+#define EXT4_TUNE_FL_EDIT_FEATURES	0x00004000
+#define EXT4_TUNE_FL_FORCE_FSCK		0x00008000
+#define EXT4_TUNE_FL_ENCODING		0x00010000
+#define EXT4_TUNE_FL_ENCODING_FLAGS	0x00020000
+
+#define EXT4_IOC_GET_TUNE_SB_PARAM	_IOR('f', 45, struct ext4_tune_sb_params)
+#define EXT4_IOC_SET_TUNE_SB_PARAM	_IOW('f', 46, struct ext4_tune_sb_params)
+
+#endif
+
 extern int ask_yn(const char *string, int def);
 
 #define OPT_MAX_MOUNTCOUNT	 1
@@ -145,6 +203,8 @@ char *io_options;
 static int force, do_list_super, sparse_value = -1;
 static time_t last_check_time;
 static int max_mount_count, mount_count, mount_flags;
+static int fs_fd = -1;
+static char mntpt[PATH_MAX + 1];
 static unsigned long interval;
 static blk64_t reserved_blocks;
 static double reserved_ratio;
@@ -2052,6 +2112,10 @@ static void parse_tune2fs_options(int argc, char **argv)
 			}
 			if (max_mount_count == 0)
 				max_mount_count = -1;
+			else if (max_mount_count == 65536) {
+				max_mount_count = EXT2_DFL_MAX_MNT_COUNT +
+					(random() % EXT2_DFL_MAX_MNT_COUNT);
+			}
 			break;
 		case 'C':
 			opts[OPT_MOUNTCOUNT] = true;
@@ -2134,6 +2198,12 @@ static void parse_tune2fs_options(int argc, char **argv)
 					_("bad interval - %s"), optarg);
 				usage();
 			}
+			if ((unsigned long long)interval >= (1ULL << 32)) {
+				com_err(program_name, 0,
+					_("interval between checks is too big (%lu)"),
+					interval);
+				exit(1);
+			}
 			break;
 		case 'j':
 			opts[OPT_JOURNAL_SIZE] = true;
@@ -3105,73 +3175,204 @@ fs_update_journal_user(struct ext2_super_block *sb, __u8 old_uuid[UUID_SIZE])
 	return 0;
 }
 
-/*
- * Use FS_IOC_SETFSLABEL or FS_IOC_GETFSLABEL to set/get file system label
- * Return:	0 on success
- *		1 on error
- *		-1 when the old method should be used
- */
-static int handle_fslabel(int setlabel)
+static int get_mount_flags()
+{
+	errcode_t	ret;
+
+	ret = ext2fs_check_mount_point(device_name, &mount_flags,
+				       mntpt, sizeof(mntpt));
+	if (ret) {
+		com_err("ext2fs_check_mount_point", ret,
+			_("while determining whether %s is mounted."),
+			device_name);
+		return -1;
+	}
+
+#ifdef __linux__
+	if ((ret == 0) &&
+	    (mount_flags & EXT2_MF_MOUNTED) &&
+	    mntpt[0])
+		fs_fd = open(mntpt, O_RDONLY);
+#endif
+	return 0;
+}
+
+static int try_mounted_tune2fs()
 {
 #ifdef __linux__
 	errcode_t ret;
-	int mnt_flags, fd;
 	char label[FSLABEL_MAX];
-	unsigned int maxlen = FSLABEL_MAX - 1;
-	char mntpt[PATH_MAX + 1];
+	struct ext4_tune_sb_params params;
+	__u64 fs_blocks_count;
+	__u32 fs_feature_array[3], kernel_set_mask[3], kernel_clear_mask[3];
+	__u32 default_mnt_opts;
+	int fs_set_ops = 0;
 
-	ret = ext2fs_check_mount_point(device_name, &mnt_flags,
-					  mntpt, sizeof(mntpt));
-	if (ret)
-		return -1;
+	if (fs_fd < 0)
+		return 0;
 
-	if (!(mnt_flags & EXT2_MF_MOUNTED) ||
-	    (setlabel && (mnt_flags & EXT2_MF_READONLY)))
-		return -1;
+	if (opts[OPT_PRINT_LABEL] &&
+	    !ioctl(fs_fd, FS_IOC_GETFSLABEL, &label)) {
+		printf("%.*s\n", EXT2_LEN_STR(label));
+		opts[OPT_PRINT_LABEL] = false;
+	}
 
-	if (!mntpt[0])
-		return -1;
+	if (mount_flags & EXT2_MF_READONLY)
+		return 0;
 
-	fd = open(mntpt, O_RDONLY);
-	if (fd < 0)
-		return -1;
+	if (opts[OPT_LABEL]) {
+		unsigned int maxlen = FSLABEL_MAX - 1;
 
-	/* Get fs label */
-	if (!setlabel) {
-		if (ioctl(fd, FS_IOC_GETFSLABEL, &label)) {
-			close(fd);
-			if (errno == ENOTTY)
-				return -1;
-			com_err(mntpt, errno, _("while trying to get fs label"));
-			return 1;
+		/* If it's extN file system, truncate the label
+		   to appropriate size */
+		if (mount_flags & EXT2_MF_EXTFS)
+			maxlen = EXT2_LABEL_LEN;
+		if (strlen(new_label) > maxlen) {
+			fputs(_("Warning: label too long, truncating.\n"),
+			      stderr);
+			new_label[maxlen] = '\0';
 		}
-		close(fd);
-		printf("%.*s\n", EXT2_LEN_STR(label));
-		return 0;
+		if (ioctl(fs_fd, FS_IOC_SETFSLABEL, new_label) == 0)
+			opts[OPT_LABEL] = false;
 	}
 
-	/* If it's extN file system, truncate the label to appropriate size */
-	if (mnt_flags & EXT2_MF_EXTFS)
-		maxlen = EXT2_LABEL_LEN;
-	if (strlen(new_label) > maxlen) {
-		fputs(_("Warning: label too long, truncating.\n"),
-		      stderr);
-		new_label[maxlen] = '\0';
-	}
+	if (ioctl(fs_fd, EXT4_IOC_GET_TUNE_SB_PARAM, &params))
+		return 0;
 
-	/* Set fs label */
-	if (ioctl(fd, FS_IOC_SETFSLABEL, new_label)) {
-		close(fd);
-		if (errno == ENOTTY)
+	fs_set_ops = params.set_flags;
+	fs_blocks_count = params.blocks_count;
+	fs_feature_array[0] = params.feature_compat;
+	fs_feature_array[1] = params.feature_incompat;
+	fs_feature_array[2] = params.feature_ro_compat;
+	kernel_set_mask[0] = params.set_feature_compat_mask;
+	kernel_set_mask[1] = params.set_feature_incompat_mask;
+	kernel_set_mask[2] = params.set_feature_ro_compat_mask;
+	kernel_clear_mask[0] = params.clear_feature_compat_mask;
+	kernel_clear_mask[1] = params.clear_feature_incompat_mask;
+	kernel_clear_mask[2] = params.clear_feature_ro_compat_mask;
+	default_mnt_opts = params.default_mnt_opts;
+
+	memset(&params, 0, sizeof(params));
+
+#define SIMPLE_SET_PARAM(OPT, FLAG, PARAM_FIELD, VALUE) \
+	if (opts[OPT] && (fs_set_ops & FLAG)) { 	\
+		params.set_flags |= FLAG; 		\
+		params.PARAM_FIELD = VALUE;		\
+	}
+	SIMPLE_SET_PARAM(OPT_ERROR_BEHAVIOR, EXT4_TUNE_FL_ERRORS_BEHAVIOR,
+			 errors_behavior, errors);
+	SIMPLE_SET_PARAM(OPT_MOUNTCOUNT, EXT4_TUNE_FL_MNT_COUNT,
+			 set_flags, mount_count);
+	SIMPLE_SET_PARAM(OPT_MAX_MOUNTCOUNT, EXT4_TUNE_FL_MAX_MNT_COUNT,
+			 set_flags, max_mount_count);
+	SIMPLE_SET_PARAM(OPT_CHECKINTERVAL, EXT4_TUNE_FL_CHECKINTRVAL,
+			 set_flags, interval);
+	SIMPLE_SET_PARAM(OPT_CHECKTIME, EXT4_TUNE_FL_LAST_CHECK_TIME,
+			 last_check_time, last_check_time);
+	SIMPLE_SET_PARAM(OPT_RESUID, EXT4_TUNE_FL_RESERVED_UID,
+			 reserved_uid, resuid);
+	SIMPLE_SET_PARAM(OPT_RESGID, EXT4_TUNE_FL_RESERVED_GID,
+			 reserved_gid, resgid);
+	if (opts[OPT_RESERVED_RATIO] && !opts[OPT_RESERVED_BLOCKS]) {
+		reserved_blocks = reserved_ratio * fs_blocks_count / 100.0;
+		opts[OPT_RESERVED_BLOCKS] = true;
+	}
+	SIMPLE_SET_PARAM(OPT_RESERVED_BLOCKS, EXT4_TUNE_FL_RESERVED_BLOCKS,
+			 reserved_blocks, reserved_blocks);
+	SIMPLE_SET_PARAM(OPT_RAID_STRIDE, EXT4_TUNE_FL_RAID_STRIDE,
+			 raid_stride, stride);
+	SIMPLE_SET_PARAM(OPT_RAID_STRIPE_WIDTH, EXT4_TUNE_FL_RAID_STRIPE_WIDTH,
+			 raid_stripe_width, stripe_width);
+	SIMPLE_SET_PARAM(OPT_ENCODING, EXT4_TUNE_FL_ENCODING,
+			encoding, encoding);
+	SIMPLE_SET_PARAM(OPT_ENCODING_FLAGS, EXT4_TUNE_FL_ENCODING_FLAGS,
+			encoding_flags, encoding_flags);
+	if (opts[OPT_MNTOPTS] &&
+	    (fs_set_ops & EXT4_TUNE_FL_DEFAULT_MNT_OPTS)) {
+		if (e2p_edit_mntopts(mntopts_cmd, &default_mnt_opts, ~0)) {
+			fprintf(stderr, _("Invalid mount option set: %s\n"),
+				mntopts_cmd);
 			return -1;
-		com_err(mntpt, errno, _("while trying to set fs label"));
-		return 1;
+		}
+		params.set_flags |= EXT4_TUNE_FL_DEFAULT_MNT_OPTS;
+		params.default_mnt_opts = default_mnt_opts;
+	}
+	if (opts[OPT_MOUNT_OPTS] &&
+	    (fs_set_ops & EXT4_TUNE_FL_MOUNT_OPTS)) {
+		params.set_flags |= EXT4_TUNE_FL_MOUNT_OPTS;
+		strncpy(params.mount_opts, ext_mount_opts,
+			sizeof(params.mount_opts));
+		params.mount_opts[sizeof(params.mount_opts) - 1] = 0;
+	}
+	if (opts[OPT_FEATURES] &&
+	    (fs_set_ops & EXT4_TUNE_FL_FEATURES) &&
+	    !e2p_edit_feature2(features_cmd, fs_feature_array,
+			       kernel_set_mask, kernel_clear_mask,
+			       NULL, NULL)) {
+		params.set_flags |= EXT4_TUNE_FL_FEATURES;
+		params.feature_compat = fs_feature_array[0];
+		params.feature_incompat = fs_feature_array[1];
+		params.feature_ro_compat = fs_feature_array[2];
+	}
+	if (opts[OPT_FORCE_FSCK] &&
+	    (fs_set_ops & EXT4_TUNE_FL_FORCE_FSCK))
+		params.set_flags |= EXT4_TUNE_FL_FORCE_FSCK;
+
+	if (ioctl(fs_fd, EXT4_IOC_SET_TUNE_SB_PARAM, &params) == 0) {
+		if (opts[OPT_ERROR_BEHAVIOR])
+			printf(_("Setting error behavior to %d\n"), errors);
+		if (opts[OPT_MOUNTCOUNT])
+			printf(_("Setting current mount count to %d\n"),
+			       mount_count);
+		if (opts[OPT_MAX_MOUNTCOUNT])
+			printf(_("Setting maximal mount count to %d\n"),
+			       max_mount_count);
+		if (opts[OPT_CHECKINTERVAL])
+			printf(_("Setting interval between checks to %lu seconds\n"),
+			       interval);
+		if (opts[OPT_CHECKTIME])
+			printf(_("Setting time filesystem last checked to %s\n"),
+			       ctime(&last_check_time));
+		if (opts[OPT_RESUID])
+			printf(_("Setting reserved blocks uid to %lu\n"),
+			       resuid);
+		if (opts[OPT_RESGID])
+			printf(_("Setting reserved blocks gid to %lu\n"),
+			       resgid);
+		if (opts[OPT_RESERVED_BLOCKS])
+			printf(_("Setting reserved blocks count to %llu\n"),
+			       (unsigned long long) reserved_blocks);
+		if (opts[OPT_RAID_STRIDE])
+			printf(_("Setting stride size to %d\n"), stride);
+		if (opts[OPT_RAID_STRIPE_WIDTH])
+			printf(_("Setting stripe width to %d\n"),
+			       stripe_width);
+		if (opts[OPT_MOUNT_OPTS])
+			printf(_("Setting extended default mount options to '%s'\n"),
+			       ext_mount_opts);
+		if (opts[OPT_ENCODING])
+			printf(_("Setting encoding to '%s'\n"), encoding_str);
+		if (opts[OPT_ENCODING_FLAGS])
+			printf(_("Setting encoding_flags to '%s'\n"),
+			       encoding_flags_str);
+		if (opts[OPT_FORCE_FSCK])
+			printf(_("Setting filesystem error flag to force fsck.\n"));
+		opts[OPT_ERROR_BEHAVIOR] = opts[OPT_MOUNTCOUNT] =
+			opts[OPT_MAX_MOUNTCOUNT] = opts[OPT_CHECKINTERVAL] =
+			opts[OPT_CHECKTIME] = opts[OPT_RESUID] =
+			opts[OPT_RESGID] = opts[OPT_RESERVED_RATIO] =
+			opts[OPT_RESERVED_BLOCKS] = opts[OPT_MNTOPTS] =
+			opts[OPT_RAID_STRIDE] = opts[OPT_RAID_STRIPE_WIDTH] =
+			opts[OPT_MOUNT_OPTS] = opts[OPT_FEATURES] =
+			opts[OPT_FORCE_FSCK] = opts[OPT_ENCODING] =
+			opts[OPT_ENCODING_FLAGS] = false;
+		printf("online tune superblock succeeded\n");
+	} else {
+		perror("ioctl EXT4_IOC_SET_TUNE_SB_PARAM");
+		return -1;
 	}
-	close(fd);
-	return 0;
-#else
-	return -1;
 #endif
+	return 0;
 }
 
 #ifndef BUILD_AS_LIB
@@ -3186,7 +3387,6 @@ int tune2fs_main(int argc, char **argv)
 	io_manager io_ptr, io_ptr_orig = NULL;
 	int rc = 0;
 	char default_undo_file[1] = { 0 };
-	char mntpt[PATH_MAX + 1] = { 0 };
 	int fd = -1;
 	struct fsuuid *fsuuid = NULL;
 
@@ -3220,19 +3420,21 @@ int tune2fs_main(int argc, char **argv)
 #endif
 		io_ptr = unix_io_manager;
 
-	/*
-	 * Try the get/set fs label using ioctls before we even attempt
-	 * to open the file system.
-	 */
-	if (opts[OPT_LABEL] || opts[OPT_PRINT_LABEL]) {
-		rc = handle_fslabel(opts[OPT_LABEL]);
-		if (rc != -1) {
-#ifndef BUILD_AS_LIB
-			exit(rc);
+	if (get_mount_flags() < 0 || try_mounted_tune2fs() << 0) {
+#ifdef BUILD_AS_LIB
+		return -1;
+#else
+		exit(1);
+#endif
+	}
+
+	if (!tune_opts_requested()) {
+		/* printf("No further tune opts left\n"); */
+#ifdef BUILD_AS_LIB
+		return 0;
+#else
+		exit(0);
 #endif
-			return rc;
-		}
-		rc = 0;
 	}
 
 retry_open:
@@ -3338,16 +3540,6 @@ retry_open:
 		goto closefs;
 	}
 
-	retval = ext2fs_check_mount_point(device_name, &mount_flags,
-					mntpt, sizeof(mntpt));
-	if (retval) {
-		com_err("ext2fs_check_mount_point", retval,
-			_("while determining whether %s is mounted."),
-			device_name);
-		rc = 1;
-		goto closefs;
-	}
-
 #ifdef NO_RECOVERY
 	/* Warn if file system needs recovery and it is opened for writing. */
 	if ((open_flag & EXT2_FLAG_RW) && !(mount_flags & EXT2_MF_MOUNTED) &&
@@ -3382,9 +3574,6 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 	fs->flags |= EXT2_FLAG_SUPER_ONLY;
 
 	if (opts[OPT_MAX_MOUNTCOUNT]) {
-		if (max_mount_count == 65536)
-			max_mount_count = EXT2_DFL_MAX_MNT_COUNT +
-				(random() % EXT2_DFL_MAX_MNT_COUNT);
 		sb->s_max_mnt_count = max_mount_count;
 		ext2fs_mark_super_dirty(fs);
 		printf(_("Setting maximal mount count to %d\n"),
@@ -3410,13 +3599,6 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 		}
 	}
 	if (opts[OPT_CHECKINTERVAL]) {
-		if ((unsigned long long)interval >= (1ULL << 32)) {
-			com_err(program_name, 0,
-				_("interval between checks is too big (%lu)"),
-				interval);
-			rc = 1;
-			goto closefs;
-		}
 		sb->s_checkinterval = interval;
 		ext2fs_mark_super_dirty(fs);
 		printf(_("Setting interval between checks to %lu seconds\n"),
@@ -3494,7 +3676,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 			sizeof(sb->s_last_mounted));
 		ext2fs_mark_super_dirty(fs);
 	}
-	if (mntopts_cmd) {
+	if (opts[OPT_MNTOPTS]) {
 		rc = update_mntopts(fs, mntopts_cmd);
 		if (rc)
 			goto closefs;
-- 
2.51.0


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

* Re: [PATCH 2/3] tune2fs: rework parse_extended_opts() so it only parses the option string
  2025-09-17  3:28 ` [PATCH 2/3] tune2fs: rework parse_extended_opts() so it only parses the option string Theodore Ts'o
@ 2025-09-17  6:15   ` Andreas Dilger
  0 siblings, 0 replies; 9+ messages in thread
From: Andreas Dilger @ 2025-09-17  6:15 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Ext4 Developers List, Theodore Ts'o

On Sep 16, 2025, at 21:28, Theodore Ts'o <tytso@mit.edu> wrote:
> 
> The parse_extended_opts() was doing two things: interpreting the
> string passed into the command line and modifying the file system's
> superblock.  Separate out the file system modification and move it out
> from parse_extended_opts().
> 
> This allows the user to specify more than one -E command-line option,
> and it also allows some of the file system changes to be modified via
> an ioctl for a mounted file system.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
> misc/tune2fs.c | 211 +++++++++++++++++++++++++++----------------------
> 1 file changed, 118 insertions(+), 93 deletions(-)
> 
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index 1b3716e1..e752c328 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -123,8 +123,19 @@ extern int ask_yn(const char *string, int def);
> #define OPT_JOURNAL_OPTS    18
> #define OPT_MNTOPTS        19
> #define OPT_FEATURES        20
> -#define OPT_EXTENDED_CMD    21
> -#define MAX_OPTS        22
> +#define OPT_CLEAR_MMP        21
> +#define OPT_MMP_INTERVAL    22
> +#define OPT_FORCE_FSCK        23
> +#define OPT_TEST_FS        24
> +#define OPT_CLEAR_TEST_FS    25
> +#define OPT_RAID_STRIDE        26
> +#define OPT_RAID_STRIPE_WIDTH    27
> +#define OPT_HASH_ALG        28
> +#define OPT_MOUNT_OPTS        29
> +#define OPT_ENCODING        30
> +#define OPT_ENCODING_FLAGS    31
> +#define OPT_ORPHAN_FILE_SIZE    32
> +#define MAX_OPTS        33

This looks like it should be an enum?

Cheers, Andreas

> static bool opts[MAX_OPTS];
> 
> const char *program_name = "tune2fs";
> @@ -132,7 +143,6 @@ char *device_name;
> char *new_label, *new_last_mounted, *requested_uuid;
> char *io_options;
> static int force, do_list_super, sparse_value = -1;
> -static int clear_mmp;
> static time_t last_check_time;
> static int max_mount_count, mount_count, mount_flags;
> static unsigned long interval;
> @@ -140,12 +150,16 @@ static blk64_t reserved_blocks;
> static double reserved_ratio;
> static unsigned long resgid, resuid;
> static unsigned short errors;
> +static unsigned long mmp_interval;
> +static int hash_alg;
> +static char *hash_alg_str;
> +static int encoding;
> +static __u16 encoding_flags;
> +static char *encoding_str, *encoding_flags_str;
> static int open_flag;
> static char *features_cmd;
> static char *mntopts_cmd;
> static int stride, stripe_width;
> -static int stride_set, stripe_width_set;
> -static char *extended_cmd;
> static unsigned long new_inode_size;
> static char *ext_mount_opts;
> static int quota_enable[MAXQUOTAS];
> @@ -153,7 +167,6 @@ static int rewrite_checksums;
> static int feature_64bit;
> static int fsck_requested;
> static char *undo_file;
> -int enabling_casefold;
> 
> int journal_size, journal_fc_size, journal_flags;
> char *journal_device;
> @@ -184,6 +197,8 @@ void do_findfs(int argc, char **argv);
> int journal_enable_debug = -1;
> #endif
> 
> +static int parse_extended_opts(const char *ext_opts);
> +
> static void usage(void)
> {
>    fprintf(stderr,
> @@ -1645,7 +1660,6 @@ mmp_error:
>        }
>        fs->super->s_encoding = EXT4_ENC_UTF8_12_1;
>        fs->super->s_encoding_flags = e2p_get_encoding_flags(EXT4_ENC_UTF8_12_1);
> -        enabling_casefold = 1;
>    }
> 
>    if (FEATURE_OFF(E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_CASEFOLD)) {
> @@ -1661,7 +1675,6 @@ mmp_error:
>        }
>        fs->super->s_encoding = 0;
>        fs->super->s_encoding_flags = 0;
> -        enabling_casefold = 0;
>    }
> 
>    if (FEATURE_ON(E2P_FEATURE_INCOMPAT,
> @@ -2066,8 +2079,8 @@ static void parse_tune2fs_options(int argc, char **argv)
>            }
>            break;
>        case 'E':
> -            opts[OPT_EXTENDED_CMD] = true;
> -            extended_cmd = optarg;
> +            if (parse_extended_opts(optarg))
> +                exit(1);
>            break;
>        case 'f': /* Force */
>            force++;
> @@ -2259,6 +2272,11 @@ static void parse_tune2fs_options(int argc, char **argv)
>            argv[optind]);
>        exit(1);
>    }
> +    if (opts[OPT_ENCODING_FLAGS] && !opts[OPT_ENCODING]) {
> +        fprintf(stderr, _("error: An encoding must be explicitly "
> +                  "specified when passing encoding-flags\n"));
> +        exit(1);
> +    }
> }
> 
> #ifdef CONFIG_BUILD_FINDFS
> @@ -2282,23 +2300,22 @@ void do_findfs(int argc, char **argv)
> }
> #endif
> 
> -static int parse_extended_opts(ext2_filsys fs, const char *opts)
> +#define member_size(type, member) (sizeof( ((type *)0)->member ))
> +
> +static int parse_extended_opts(const char *ext_opts)
> {
> -    struct ext2_super_block *sb = fs->super;
>    char    *buf, *token, *next, *p, *arg;
> -    int    len, hash_alg;
> +    int    len;
>    int    r_usage = 0;
> -    int encoding = 0;
> -    char    *encoding_flags = NULL;
> 
> -    len = strlen(opts);
> +    len = strlen(ext_opts);
>    buf = malloc(len+1);
>    if (!buf) {
>        fprintf(stderr, "%s",
>            _("Couldn't allocate memory to parse options!\n"));
>        return 1;
>    }
> -    strcpy(buf, opts);
> +    strcpy(buf, ext_opts);
>    for (token = buf; token && *token; token = next) {
>        p = strchr(token, ',');
>        next = 0;
> @@ -2313,14 +2330,13 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
>        }
>        if (strcmp(token, "clear-mmp") == 0 ||
>            strcmp(token, "clear_mmp") == 0) {
> -            clear_mmp = 1;
> +            opts[OPT_CLEAR_MMP] = true;
>        } else if (strcmp(token, "mmp_update_interval") == 0) {
> -            unsigned long intv;
>            if (!arg) {
>                r_usage++;
>                continue;
>            }
> -            intv = strtoul(arg, &p, 0);
> +            mmp_interval = strtoul(arg, &p, 0);
>            if (*p) {
>                fprintf(stderr,
>                    _("Invalid mmp_update_interval: %s\n"),
> @@ -2328,34 +2344,22 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
>                r_usage++;
>                continue;
>            }
> -            if (intv == 0) {
> -                intv = EXT4_MMP_UPDATE_INTERVAL;
> -            } else if (intv > EXT4_MMP_MAX_UPDATE_INTERVAL) {
> +            if (mmp_interval == 0) {
> +                mmp_interval = EXT4_MMP_UPDATE_INTERVAL;
> +            } else if (mmp_interval > EXT4_MMP_MAX_UPDATE_INTERVAL) {
>                fprintf(stderr,
>                    _("mmp_update_interval too big: %lu\n"),
> -                    intv);
> +                    mmp_interval);
>                r_usage++;
>                continue;
>            }
> -            printf(P_("Setting multiple mount protection update "
> -                  "interval to %lu second\n",
> -                  "Setting multiple mount protection update "
> -                  "interval to %lu seconds\n", intv),
> -                   intv);
> -            sb->s_mmp_update_interval = intv;
> -            ext2fs_mark_super_dirty(fs);
> +            opts[OPT_MMP_INTERVAL] = true;
>        } else if (!strcmp(token, "force_fsck")) {
> -            sb->s_state |= EXT2_ERROR_FS;
> -            printf(_("Setting filesystem error flag to force fsck.\n"));
> -            ext2fs_mark_super_dirty(fs);
> +            opts[OPT_FORCE_FSCK] = true;
>        } else if (!strcmp(token, "test_fs")) {
> -            sb->s_flags |= EXT2_FLAGS_TEST_FILESYS;
> -            printf("Setting test filesystem flag\n");
> -            ext2fs_mark_super_dirty(fs);
> +            opts[OPT_TEST_FS] = true;
>        } else if (!strcmp(token, "^test_fs")) {
> -            sb->s_flags &= ~EXT2_FLAGS_TEST_FILESYS;
> -            printf("Clearing test filesystem flag\n");
> -            ext2fs_mark_super_dirty(fs);
> +            opts[OPT_CLEAR_TEST_FS] = true;
>        } else if (strcmp(token, "stride") == 0) {
>            if (!arg) {
>                r_usage++;
> @@ -2369,7 +2373,7 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
>                r_usage++;
>                continue;
>            }
> -            stride_set = 1;
> +            opts[OPT_RAID_STRIDE] = true;
>        } else if (strcmp(token, "stripe-width") == 0 ||
>               strcmp(token, "stripe_width") == 0) {
>            if (!arg) {
> @@ -2384,7 +2388,7 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
>                r_usage++;
>                continue;
>            }
> -            stripe_width_set = 1;
> +            opts[OPT_RAID_STRIPE_WIDTH] = true;
>        } else if (strcmp(token, "hash_alg") == 0 ||
>               strcmp(token, "hash-alg") == 0) {
>            if (!arg) {
> @@ -2399,21 +2403,21 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
>                r_usage++;
>                continue;
>            }
> -            sb->s_def_hash_version = hash_alg;
> -            printf(_("Setting default hash algorithm "
> -                 "to %s (%d)\n"),
> -                   arg, hash_alg);
> -            ext2fs_mark_super_dirty(fs);
> +            hash_alg_str = strdup(arg);
> +            opts[OPT_HASH_ALG] = true;
>        } else if (!strcmp(token, "mount_opts")) {
>            if (!arg) {
>                r_usage++;
>                continue;
>            }
> -            if (strlen(arg) >= sizeof(fs->super->s_mount_opts)) {
> +            if (strlen(arg) >=
> +                member_size(struct ext2_super_block,
> +                    s_mount_opts)) {
>                fprintf(stderr,
>                    "Extended mount options too long\n");
>                continue;
>            }
> +            opts[OPT_MOUNT_OPTS] = true;
>            ext_mount_opts = strdup(arg);
>        } else if (!strcmp(token, "encoding")) {
>            if (!arg) {
> @@ -2426,36 +2430,33 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
>                r_usage++;
>                continue;
>            }
> -            if (ext2fs_has_feature_casefold(sb) && !enabling_casefold) {
> -                fprintf(stderr, _("Cannot alter existing encoding\n"));
> -                r_usage++;
> -                continue;
> -            }
>            encoding = e2p_str2encoding(arg);
>            if (encoding < 0) {
>                fprintf(stderr, _("Invalid encoding: %s\n"), arg);
>                r_usage++;
>                continue;
>            }
> -            enabling_casefold = 1;
> -            sb->s_encoding = encoding;
> -            printf(_("Setting encoding to '%s'\n"), arg);
> -            sb->s_encoding_flags =
> -                e2p_get_encoding_flags(sb->s_encoding);
> +            encoding_str = strdup(arg);
> +            opts[OPT_ENCODING] = true;
>        } else if (!strcmp(token, "encoding_flags")) {
>            if (!arg) {
>                r_usage++;
>                continue;
>            }
> -            encoding_flags = arg;
> +            if (e2p_str2encoding_flags(EXT4_ENC_UTF8_12_1,
> +                           arg, &encoding_flags)) {
> +                fprintf(stderr,
> +        _("error: Invalid encoding flag: %s\n"), arg);
> +                r_usage++;
> +            }
> +            encoding_flags_str = strdup(arg);
> +            opts[OPT_ENCODING_FLAGS] = true;
>        } else if (!strcmp(token, "orphan_file_size")) {
>            if (!arg) {
>                r_usage++;
>                continue;
>            }
> -            orphan_file_blocks = parse_num_blocks2(arg,
> -                         fs->super->s_log_block_size);
> -
> +            orphan_file_blocks = parse_num_blocks2(arg, 0);
>            if (orphan_file_blocks < 1) {
>                fprintf(stderr,
>                    _("Invalid size of orphan file %s\n"),
> @@ -2463,30 +2464,10 @@ static int parse_extended_opts(ext2_filsys fs, const char *opts)
>                r_usage++;
>                continue;
>            }
> +            opts[OPT_ORPHAN_FILE_SIZE] = true;
>        } else
>            r_usage++;
>    }
> -
> -    if (encoding > 0 && !r_usage) {
> -        sb->s_encoding_flags =
> -            e2p_get_encoding_flags(sb->s_encoding);
> -
> -        if (encoding_flags &&
> -            e2p_str2encoding_flags(sb->s_encoding, encoding_flags,
> -                       &sb->s_encoding_flags)) {
> -            fprintf(stderr, _("error: Invalid encoding flag: %s\n"),
> -                    encoding_flags);
> -            r_usage++;
> -        } else if (encoding_flags)
> -            printf(_("Setting encoding_flags to '%s'\n"),
> -                 encoding_flags);
> -        ext2fs_set_feature_casefold(sb);
> -        ext2fs_mark_super_dirty(fs);
> -    } else if (encoding_flags && !r_usage) {
> -        fprintf(stderr, _("error: An encoding must be explicitly "
> -                  "specified when passing encoding-flags\n"));
> -        r_usage++;
> -    }
>    if (r_usage) {
>        fprintf(stderr, "%s", _("\nBad options specified.\n\n"
>            "Extended options are separated by commas, "
> @@ -3518,27 +3499,64 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
>        if (rc)
>            goto closefs;
>    }
> +    if (ext2fs_has_feature_casefold(sb) && opts[OPT_ENCODING]) {
> +        fprintf(stderr, _("Cannot alter existing encoding\n"));
> +        rc = 1;
> +        goto closefs;
> +    }
>    if (features_cmd) {
>        rc = update_feature_set(fs, features_cmd);
>        if (rc)
>            goto closefs;
>    }
> -    if (extended_cmd) {
> -        rc = parse_extended_opts(fs, extended_cmd);
> -        if (rc)
> -            goto closefs;
> -        if (clear_mmp && !force) {
> +    if (opts[OPT_CLEAR_MMP]) {
> +        if (!force) {
>            fputs(_("Error in using clear_mmp. "
>                "It must be used with -f\n"),
>                  stderr);
>            rc = 1;
>            goto closefs;
>        }
> -    }
> -    if (clear_mmp) {
>        rc = ext2fs_mmp_clear(fs);
>        goto closefs;
>    }
> +    if (opts[OPT_MMP_INTERVAL]) {
> +        printf(P_("Setting multiple mount protection update "
> +              "interval to %lu second\n",
> +              "Setting multiple mount protection update "
> +              "interval to %lu seconds\n", mmp_interval),
> +               mmp_interval);
> +        sb->s_mmp_update_interval = mmp_interval;
> +        ext2fs_mark_super_dirty(fs);
> +    }
> +    if (opts[OPT_FORCE_FSCK]) {
> +        sb->s_state |= EXT2_ERROR_FS;
> +        printf(_("Setting filesystem error flag to force fsck.\n"));
> +        ext2fs_mark_super_dirty(fs);
> +    }
> +    if (opts[OPT_TEST_FS]) {
> +        sb->s_flags |= EXT2_FLAGS_TEST_FILESYS;
> +        printf("Setting test filesystem flag\n");
> +        ext2fs_mark_super_dirty(fs);
> +    }
> +    if (opts[OPT_CLEAR_TEST_FS]) {
> +        sb->s_flags &= ~EXT2_FLAGS_TEST_FILESYS;
> +        printf("Clearing test filesystem flag\n");
> +        ext2fs_mark_super_dirty(fs);
> +    }
> +    if (opts[OPT_ENCODING]) {
> +        ext2fs_set_feature_casefold(sb);
> +        sb->s_encoding = encoding;
> +        printf(_("Setting encoding to '%s'\n"), encoding_str);
> +        if (opts[OPT_ENCODING_FLAGS]) {
> +            sb->s_encoding_flags = encoding_flags;
> +            printf(_("Setting encoding_flags to '%s'\n"),
> +                   encoding_flags_str);
> +        } else
> +            sb->s_encoding_flags =
> +                e2p_get_encoding_flags(sb->s_encoding);
> +            ext2fs_mark_super_dirty(fs);
> +    }
>    if (journal_size || journal_device) {
>        rc = add_journal(fs);
>        if (rc)
> @@ -3554,6 +3572,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
>            rc = 1;
>            goto closefs;
>        }
> +        orphan_file_blocks >>= fs->super->s_log_block_size;
>        err = ext2fs_create_orphan_file(fs, orphan_file_blocks);
>        if (err) {
>            com_err(program_name, err, "%s",
> @@ -3764,17 +3783,23 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
> 
>    if (do_list_super)
>        list_super(sb);
> -    if (stride_set) {
> +    if (opts[OPT_RAID_STRIDE]) {
>        sb->s_raid_stride = stride;
>        ext2fs_mark_super_dirty(fs);
>        printf(_("Setting stride size to %d\n"), stride);
>    }
> -    if (stripe_width_set) {
> +    if (opts[OPT_RAID_STRIPE_WIDTH]) {
>        sb->s_raid_stripe_width = stripe_width;
>        ext2fs_mark_super_dirty(fs);
>        printf(_("Setting stripe width to %d\n"), stripe_width);
>    }
> -    if (ext_mount_opts) {
> +    if (opts[OPT_HASH_ALG]) {
> +        sb->s_def_hash_version = hash_alg;
> +        printf(_("Setting default hash algorithm to %s (%d)\n"),
> +               hash_alg_str, hash_alg);
> +        ext2fs_mark_super_dirty(fs);
> +    }
> +    if (opts[OPT_MOUNT_OPTS]) {
>        strncpy((char *)(fs->super->s_mount_opts), ext_mount_opts,
>            sizeof(fs->super->s_mount_opts));
>        fs->super->s_mount_opts[sizeof(fs->super->s_mount_opts)-1] = 0;
> --
> 2.51.0
> 
> 

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

* Re: [PATCH 0/3] E2fsprogs: tune2fs: use an ioctl to update mounted fs
  2025-09-17  3:28 [PATCH 0/3] E2fsprogs: tune2fs: use an ioctl to update mounted fs Theodore Ts'o
                   ` (2 preceding siblings ...)
  2025-09-17  3:28 ` [PATCH 3/3] tune2fs: try to use the SET_TUNE_SB_PARAM ioctl on mounted file systems Theodore Ts'o
@ 2025-09-17  6:26 ` Andreas Dilger
  2025-09-18 16:55   ` Theodore Ts'o
  3 siblings, 1 reply; 9+ messages in thread
From: Andreas Dilger @ 2025-09-17  6:26 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Ext4 Developers List, Theodore Ts'o

On Sep 16, 2025, at 21:28, Theodore Ts'o <tytso@mit.edu> wrote:
> 
> Teach tune2fs to try use the new EXT4_IOC_SET_TUNE_SB_PARAM ioctl
> interface to update mounted file systems.  This will allow us to
> disallow read/write access to the block device while the file system
> is mounted, once we are sure the updated e2fsprogs is in use.
> 
> Theodore Ts'o (3):
>  tune2fs: reorganize command-line flag handling
>  tune2fs: rework parse_extended_opts() so it only parses the option
>    string
>  tune2fs: try to use the SET_TUNE_SB_PARAM ioctl on mounted file
>    systems

Have you considered to use a mount option (eg. mount.ext4) or a flag stored in the sb
to indicate that a new e2fsprogs is installed in userspace?

That would take the guesswork out of when to enable blocking direct superblock writes,
and allow this feature to be enabled sooner. 

Cheers, Andreas 

> 
> misc/tune2fs.c | 763 ++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 496 insertions(+), 267 deletions(-)
> 
> --
> 2.51.0
> 
> 

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

* Re: [PATCH 0/3] E2fsprogs: tune2fs: use an ioctl to update mounted fs
  2025-09-17  6:26 ` [PATCH 0/3] E2fsprogs: tune2fs: use an ioctl to update mounted fs Andreas Dilger
@ 2025-09-18 16:55   ` Theodore Ts'o
  0 siblings, 0 replies; 9+ messages in thread
From: Theodore Ts'o @ 2025-09-18 16:55 UTC (permalink / raw)
  To: Andreas Dilger; +Cc: Ext4 Developers List

On Wed, Sep 17, 2025 at 12:26:18AM -0600, Andreas Dilger wrote:
> On Sep 16, 2025, at 21:28, Theodore Ts'o <tytso@mit.edu> wrote:
> > 
> > Teach tune2fs to try use the new EXT4_IOC_SET_TUNE_SB_PARAM ioctl
> > interface to update mounted file systems.  This will allow us to
> > disallow read/write access to the block device while the file system
> > is mounted, once we are sure the updated e2fsprogs is in use.
> 
> Have you considered to use a mount option (eg. mount.ext4) or a flag
> stored in the sb to indicate that a new e2fsprogs is installed in
> userspace?

For better or for worse, currently whether writes to mounted block
devices are blocked is a global switch, controlled by the
bdev_allow_write_mounted boot command-line option and
CONFIG_BLK_DEV_WRITE_MOUNTED (which controls the default).  That's
because it was designed solely to reduce syzbot noise.

As far as I know, ext2 and ext4 are the only file sysetms which
require sysadmins to need write access to mounted file systems (for
tune2fs).  Most other file systems don't actually allow mounted file
system reconfiguration, or they have ioctl's to allow this.  So I
believe that if a system has a newer version of e2fsprogs, it should
be sufficient for the sysadmin to include
bdev_allow_write_mounted=false in the boot options.  (Or for a
distribution to set CONFIG_BLK_DEV_WRITE_MOUNTED if they can guarantee
that a sufficiently new version of e2fsprogs will be installed with a
particular kernel.)

I don't object to having a per-file sytstem mount option, but it would
require changes in block/bdev.c.  And it might involve more complexity
that would be exposed to the system adinistrators.  So unless there is
some other file system type beyond ext4 which might need write access
while the file system is mounetd, maybe we don't need a per-fs switch.
Maybe ext2, but I think most distributions are using
CONFIG_EXT4_USE_FOR_EXT2 so in practice I don't think it's necessary.

Cheers,

						- Ted

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

* Re: [PATCH 3/3] tune2fs: try to use the SET_TUNE_SB_PARAM ioctl on mounted file systems
  2025-09-17  3:28 ` [PATCH 3/3] tune2fs: try to use the SET_TUNE_SB_PARAM ioctl on mounted file systems Theodore Ts'o
@ 2025-09-18 17:47   ` Darrick J. Wong
  2025-09-19 15:01     ` Theodore Ts'o
  0 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2025-09-18 17:47 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Ext4 Developers List

On Tue, Sep 16, 2025 at 11:28:14PM -0400, Theodore Ts'o wrote:
> Try to uuse the new EXT4_IOC_GET_TUNE_SB_PARAM ioctl to update the
> superblock if the file system is mounted.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  misc/tune2fs.c | 352 +++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 267 insertions(+), 85 deletions(-)
> 
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index e752c328..b1ec3991 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -101,6 +101,64 @@ struct fsuuid {
>  #define EXT4_IOC_SETFSUUID	_IOW('f', 44, struct fsuuid)
>  #endif
>  
> +#if (!defined(EXT4_IOC_GET_TUNE_SB_PARAM) && defined(__linux__))
> +
> +struct ext4_tune_sb_params {
> +	__u32 set_flags;
> +	__u32 checkinterval;
> +	__u16 errors_behavior;
> +	__u16 mnt_count;
> +	__u16 max_mnt_count;
> +	__u16 raid_stride;
> +	__u64 last_check_time;
> +	__u64 reserved_blocks;
> +	__u64 blocks_count;
> +	__u32 default_mnt_opts;
> +	__u32 reserved_uid;
> +	__u32 reserved_gid;
> +	__u32 raid_stripe_width;
> +	__u16 encoding;
> +	__u16 encoding_flags;
> +	__u8  def_hash_alg;
> +	__u8  pad_1;
> +	__u16 pad_2;
> +	__u32 feature_compat;
> +	__u32 feature_incompat;
> +	__u32 feature_ro_compat;
> +	__u32 set_feature_compat_mask;
> +	__u32 set_feature_incompat_mask;
> +	__u32 set_feature_ro_compat_mask;
> +	__u32 clear_feature_compat_mask;
> +	__u32 clear_feature_incompat_mask;
> +	__u32 clear_feature_ro_compat_mask;
> +	__u8  mount_opts[64];
> +	__u8  pad[64];
> +};
> +
> +#define EXT4_TUNE_FL_ERRORS_BEHAVIOR	0x00000001
> +#define EXT4_TUNE_FL_MNT_COUNT		0x00000002
> +#define EXT4_TUNE_FL_MAX_MNT_COUNT	0x00000004
> +#define EXT4_TUNE_FL_CHECKINTRVAL	0x00000008
> +#define EXT4_TUNE_FL_LAST_CHECK_TIME	0x00000010
> +#define EXT4_TUNE_FL_RESERVED_BLOCKS	0x00000020
> +#define EXT4_TUNE_FL_RESERVED_UID	0x00000040
> +#define EXT4_TUNE_FL_RESERVED_GID	0x00000080
> +#define EXT4_TUNE_FL_DEFAULT_MNT_OPTS	0x00000100
> +#define EXT4_TUNE_FL_DEF_HASH_ALG	0x00000200
> +#define EXT4_TUNE_FL_RAID_STRIDE	0x00000400
> +#define EXT4_TUNE_FL_RAID_STRIPE_WIDTH	0x00000800
> +#define EXT4_TUNE_FL_MOUNT_OPTS		0x00001000
> +#define EXT4_TUNE_FL_FEATURES		0x00002000
> +#define EXT4_TUNE_FL_EDIT_FEATURES	0x00004000
> +#define EXT4_TUNE_FL_FORCE_FSCK		0x00008000
> +#define EXT4_TUNE_FL_ENCODING		0x00010000
> +#define EXT4_TUNE_FL_ENCODING_FLAGS	0x00020000
> +
> +#define EXT4_IOC_GET_TUNE_SB_PARAM	_IOR('f', 45, struct ext4_tune_sb_params)
> +#define EXT4_IOC_SET_TUNE_SB_PARAM	_IOW('f', 46, struct ext4_tune_sb_params)
> +
> +#endif
> +
>  extern int ask_yn(const char *string, int def);
>  
>  #define OPT_MAX_MOUNTCOUNT	 1
> @@ -145,6 +203,8 @@ char *io_options;
>  static int force, do_list_super, sparse_value = -1;
>  static time_t last_check_time;
>  static int max_mount_count, mount_count, mount_flags;
> +static int fs_fd = -1;
> +static char mntpt[PATH_MAX + 1];
>  static unsigned long interval;
>  static blk64_t reserved_blocks;
>  static double reserved_ratio;
> @@ -2052,6 +2112,10 @@ static void parse_tune2fs_options(int argc, char **argv)
>  			}
>  			if (max_mount_count == 0)
>  				max_mount_count = -1;
> +			else if (max_mount_count == 65536) {
> +				max_mount_count = EXT2_DFL_MAX_MNT_COUNT +
> +					(random() % EXT2_DFL_MAX_MNT_COUNT);
> +			}
>  			break;
>  		case 'C':
>  			opts[OPT_MOUNTCOUNT] = true;
> @@ -2134,6 +2198,12 @@ static void parse_tune2fs_options(int argc, char **argv)
>  					_("bad interval - %s"), optarg);
>  				usage();
>  			}
> +			if ((unsigned long long)interval >= (1ULL << 32)) {
> +				com_err(program_name, 0,
> +					_("interval between checks is too big (%lu)"),
> +					interval);
> +				exit(1);
> +			}
>  
> -/*
> - * Use FS_IOC_SETFSLABEL or FS_IOC_GETFSLABEL to set/get file system label
> - * Return:	0 on success
> - *		1 on error
> - *		-1 when the old method should be used
> - */
> -static int handle_fslabel(int setlabel)
> +static int get_mount_flags()
> +{
> +	errcode_t	ret;
> +
> +	ret = ext2fs_check_mount_point(device_name, &mount_flags,
> +				       mntpt, sizeof(mntpt));
> +	if (ret) {
> +		com_err("ext2fs_check_mount_point", ret,
> +			_("while determining whether %s is mounted."),
> +			device_name);
> +		return -1;
> +	}
> +
> +#ifdef __linux__
> +	if ((ret == 0) &&
> +	    (mount_flags & EXT2_MF_MOUNTED) &&
> +	    mntpt[0])
> +		fs_fd = open(mntpt, O_RDONLY);
> +#endif
> +	return 0;
> +}
> +
> +static int try_mounted_tune2fs()
>  {
>  #ifdef __linux__
>  	errcode_t ret;
> -	int mnt_flags, fd;
>  	char label[FSLABEL_MAX];
> -	unsigned int maxlen = FSLABEL_MAX - 1;
> -	char mntpt[PATH_MAX + 1];
> +	struct ext4_tune_sb_params params;
> +	__u64 fs_blocks_count;
> +	__u32 fs_feature_array[3], kernel_set_mask[3], kernel_clear_mask[3];
> +	__u32 default_mnt_opts;
> +	int fs_set_ops = 0;
>  
> -	ret = ext2fs_check_mount_point(device_name, &mnt_flags,
> -					  mntpt, sizeof(mntpt));
> -	if (ret)
> -		return -1;
> +	if (fs_fd < 0)
> +		return 0;
>  
> -	if (!(mnt_flags & EXT2_MF_MOUNTED) ||
> -	    (setlabel && (mnt_flags & EXT2_MF_READONLY)))
> -		return -1;
> +	if (opts[OPT_PRINT_LABEL] &&
> +	    !ioctl(fs_fd, FS_IOC_GETFSLABEL, &label)) {
> +		printf("%.*s\n", EXT2_LEN_STR(label));
> +		opts[OPT_PRINT_LABEL] = false;
> +	}
>  
> -	if (!mntpt[0])
> -		return -1;
> +	if (mount_flags & EXT2_MF_READONLY)
> +		return 0;
>  
> -	fd = open(mntpt, O_RDONLY);
> -	if (fd < 0)
> -		return -1;
> +	if (opts[OPT_LABEL]) {
> +		unsigned int maxlen = FSLABEL_MAX - 1;
>  
> -	/* Get fs label */
> -	if (!setlabel) {
> -		if (ioctl(fd, FS_IOC_GETFSLABEL, &label)) {
> -			close(fd);
> -			if (errno == ENOTTY)
> -				return -1;
> -			com_err(mntpt, errno, _("while trying to get fs label"));
> -			return 1;
> +		/* If it's extN file system, truncate the label
> +		   to appropriate size */
> +		if (mount_flags & EXT2_MF_EXTFS)
> +			maxlen = EXT2_LABEL_LEN;
> +		if (strlen(new_label) > maxlen) {
> +			fputs(_("Warning: label too long, truncating.\n"),
> +			      stderr);
> +			new_label[maxlen] = '\0';
>  		}
> -		close(fd);
> -		printf("%.*s\n", EXT2_LEN_STR(label));
> -		return 0;
> +		if (ioctl(fs_fd, FS_IOC_SETFSLABEL, new_label) == 0)
> +			opts[OPT_LABEL] = false;
>  	}
>  
> -	/* If it's extN file system, truncate the label to appropriate size */
> -	if (mnt_flags & EXT2_MF_EXTFS)
> -		maxlen = EXT2_LABEL_LEN;
> -	if (strlen(new_label) > maxlen) {
> -		fputs(_("Warning: label too long, truncating.\n"),
> -		      stderr);
> -		new_label[maxlen] = '\0';
> -	}
> +	if (ioctl(fs_fd, EXT4_IOC_GET_TUNE_SB_PARAM, &params))
> +		return 0;
>  
> -	/* Set fs label */
> -	if (ioctl(fd, FS_IOC_SETFSLABEL, new_label)) {
> -		close(fd);
> -		if (errno == ENOTTY)
> +	fs_set_ops = params.set_flags;
> +	fs_blocks_count = params.blocks_count;
> +	fs_feature_array[0] = params.feature_compat;
> +	fs_feature_array[1] = params.feature_incompat;
> +	fs_feature_array[2] = params.feature_ro_compat;
> +	kernel_set_mask[0] = params.set_feature_compat_mask;
> +	kernel_set_mask[1] = params.set_feature_incompat_mask;
> +	kernel_set_mask[2] = params.set_feature_ro_compat_mask;
> +	kernel_clear_mask[0] = params.clear_feature_compat_mask;
> +	kernel_clear_mask[1] = params.clear_feature_incompat_mask;
> +	kernel_clear_mask[2] = params.clear_feature_ro_compat_mask;
> +	default_mnt_opts = params.default_mnt_opts;
> +
> +	memset(&params, 0, sizeof(params));
> +
> +#define SIMPLE_SET_PARAM(OPT, FLAG, PARAM_FIELD, VALUE) \
> +	if (opts[OPT] && (fs_set_ops & FLAG)) { 	\
> +		params.set_flags |= FLAG; 		\
> +		params.PARAM_FIELD = VALUE;		\
> +	}
> +	SIMPLE_SET_PARAM(OPT_ERROR_BEHAVIOR, EXT4_TUNE_FL_ERRORS_BEHAVIOR,
> +			 errors_behavior, errors);
> +	SIMPLE_SET_PARAM(OPT_MOUNTCOUNT, EXT4_TUNE_FL_MNT_COUNT,
> +			 set_flags, mount_count);
> +	SIMPLE_SET_PARAM(OPT_MAX_MOUNTCOUNT, EXT4_TUNE_FL_MAX_MNT_COUNT,
> +			 set_flags, max_mount_count);
> +	SIMPLE_SET_PARAM(OPT_CHECKINTERVAL, EXT4_TUNE_FL_CHECKINTRVAL,
> +			 set_flags, interval);
> +	SIMPLE_SET_PARAM(OPT_CHECKTIME, EXT4_TUNE_FL_LAST_CHECK_TIME,
> +			 last_check_time, last_check_time);
> +	SIMPLE_SET_PARAM(OPT_RESUID, EXT4_TUNE_FL_RESERVED_UID,
> +			 reserved_uid, resuid);
> +	SIMPLE_SET_PARAM(OPT_RESGID, EXT4_TUNE_FL_RESERVED_GID,
> +			 reserved_gid, resgid);
> +	if (opts[OPT_RESERVED_RATIO] && !opts[OPT_RESERVED_BLOCKS]) {
> +		reserved_blocks = reserved_ratio * fs_blocks_count / 100.0;
> +		opts[OPT_RESERVED_BLOCKS] = true;
> +	}
> +	SIMPLE_SET_PARAM(OPT_RESERVED_BLOCKS, EXT4_TUNE_FL_RESERVED_BLOCKS,
> +			 reserved_blocks, reserved_blocks);
> +	SIMPLE_SET_PARAM(OPT_RAID_STRIDE, EXT4_TUNE_FL_RAID_STRIDE,
> +			 raid_stride, stride);
> +	SIMPLE_SET_PARAM(OPT_RAID_STRIPE_WIDTH, EXT4_TUNE_FL_RAID_STRIPE_WIDTH,
> +			 raid_stripe_width, stripe_width);
> +	SIMPLE_SET_PARAM(OPT_ENCODING, EXT4_TUNE_FL_ENCODING,
> +			encoding, encoding);
> +	SIMPLE_SET_PARAM(OPT_ENCODING_FLAGS, EXT4_TUNE_FL_ENCODING_FLAGS,
> +			encoding_flags, encoding_flags);
> +	if (opts[OPT_MNTOPTS] &&
> +	    (fs_set_ops & EXT4_TUNE_FL_DEFAULT_MNT_OPTS)) {
> +		if (e2p_edit_mntopts(mntopts_cmd, &default_mnt_opts, ~0)) {
> +			fprintf(stderr, _("Invalid mount option set: %s\n"),
> +				mntopts_cmd);
>  			return -1;
> -		com_err(mntpt, errno, _("while trying to set fs label"));
> -		return 1;
> +		}
> +		params.set_flags |= EXT4_TUNE_FL_DEFAULT_MNT_OPTS;
> +		params.default_mnt_opts = default_mnt_opts;
> +	}
> +	if (opts[OPT_MOUNT_OPTS] &&
> +	    (fs_set_ops & EXT4_TUNE_FL_MOUNT_OPTS)) {
> +		params.set_flags |= EXT4_TUNE_FL_MOUNT_OPTS;
> +		strncpy(params.mount_opts, ext_mount_opts,
> +			sizeof(params.mount_opts));
> +		params.mount_opts[sizeof(params.mount_opts) - 1] = 0;
> +	}
> +	if (opts[OPT_FEATURES] &&
> +	    (fs_set_ops & EXT4_TUNE_FL_FEATURES) &&
> +	    !e2p_edit_feature2(features_cmd, fs_feature_array,
> +			       kernel_set_mask, kernel_clear_mask,
> +			       NULL, NULL)) {
> +		params.set_flags |= EXT4_TUNE_FL_FEATURES;
> +		params.feature_compat = fs_feature_array[0];
> +		params.feature_incompat = fs_feature_array[1];
> +		params.feature_ro_compat = fs_feature_array[2];
> +	}
> +	if (opts[OPT_FORCE_FSCK] &&
> +	    (fs_set_ops & EXT4_TUNE_FL_FORCE_FSCK))
> +		params.set_flags |= EXT4_TUNE_FL_FORCE_FSCK;
> +
> +	if (ioctl(fs_fd, EXT4_IOC_SET_TUNE_SB_PARAM, &params) == 0) {
> +		if (opts[OPT_ERROR_BEHAVIOR])
> +			printf(_("Setting error behavior to %d\n"), errors);
> +		if (opts[OPT_MOUNTCOUNT])
> +			printf(_("Setting current mount count to %d\n"),
> +			       mount_count);
> +		if (opts[OPT_MAX_MOUNTCOUNT])
> +			printf(_("Setting maximal mount count to %d\n"),
> +			       max_mount_count);
> +		if (opts[OPT_CHECKINTERVAL])
> +			printf(_("Setting interval between checks to %lu seconds\n"),
> +			       interval);
> +		if (opts[OPT_CHECKTIME])
> +			printf(_("Setting time filesystem last checked to %s\n"),
> +			       ctime(&last_check_time));
> +		if (opts[OPT_RESUID])
> +			printf(_("Setting reserved blocks uid to %lu\n"),
> +			       resuid);
> +		if (opts[OPT_RESGID])
> +			printf(_("Setting reserved blocks gid to %lu\n"),
> +			       resgid);
> +		if (opts[OPT_RESERVED_BLOCKS])
> +			printf(_("Setting reserved blocks count to %llu\n"),
> +			       (unsigned long long) reserved_blocks);
> +		if (opts[OPT_RAID_STRIDE])
> +			printf(_("Setting stride size to %d\n"), stride);
> +		if (opts[OPT_RAID_STRIPE_WIDTH])
> +			printf(_("Setting stripe width to %d\n"),
> +			       stripe_width);
> +		if (opts[OPT_MOUNT_OPTS])
> +			printf(_("Setting extended default mount options to '%s'\n"),
> +			       ext_mount_opts);
> +		if (opts[OPT_ENCODING])
> +			printf(_("Setting encoding to '%s'\n"), encoding_str);
> +		if (opts[OPT_ENCODING_FLAGS])
> +			printf(_("Setting encoding_flags to '%s'\n"),
> +			       encoding_flags_str);
> +		if (opts[OPT_FORCE_FSCK])
> +			printf(_("Setting filesystem error flag to force fsck.\n"));
> +		opts[OPT_ERROR_BEHAVIOR] = opts[OPT_MOUNTCOUNT] =
> +			opts[OPT_MAX_MOUNTCOUNT] = opts[OPT_CHECKINTERVAL] =
> +			opts[OPT_CHECKTIME] = opts[OPT_RESUID] =
> +			opts[OPT_RESGID] = opts[OPT_RESERVED_RATIO] =
> +			opts[OPT_RESERVED_BLOCKS] = opts[OPT_MNTOPTS] =
> +			opts[OPT_RAID_STRIDE] = opts[OPT_RAID_STRIPE_WIDTH] =
> +			opts[OPT_MOUNT_OPTS] = opts[OPT_FEATURES] =
> +			opts[OPT_FORCE_FSCK] = opts[OPT_ENCODING] =
> +			opts[OPT_ENCODING_FLAGS] = false;
> +		printf("online tune superblock succeeded\n");
> +	} else {
> +		perror("ioctl EXT4_IOC_SET_TUNE_SB_PARAM");
> +		return -1;
>  	}
> -	close(fd);
> -	return 0;
> -#else
> -	return -1;

Shouldn't this still return 1 if this isn't being built on __linux__?

>  #endif
> +	return 0;
>  }
>  
>  #ifndef BUILD_AS_LIB
> @@ -3186,7 +3387,6 @@ int tune2fs_main(int argc, char **argv)
>  	io_manager io_ptr, io_ptr_orig = NULL;
>  	int rc = 0;
>  	char default_undo_file[1] = { 0 };
> -	char mntpt[PATH_MAX + 1] = { 0 };
>  	int fd = -1;
>  	struct fsuuid *fsuuid = NULL;
>  
> @@ -3220,19 +3420,21 @@ int tune2fs_main(int argc, char **argv)
>  #endif
>  		io_ptr = unix_io_manager;
>  
> -	/*
> -	 * Try the get/set fs label using ioctls before we even attempt
> -	 * to open the file system.
> -	 */
> -	if (opts[OPT_LABEL] || opts[OPT_PRINT_LABEL]) {
> -		rc = handle_fslabel(opts[OPT_LABEL]);
> -		if (rc != -1) {
> -#ifndef BUILD_AS_LIB
> -			exit(rc);
> +	if (get_mount_flags() < 0 || try_mounted_tune2fs() << 0) {

Why shift left here                                        ^^ ??

> +#ifdef BUILD_AS_LIB
> +		return -1;
> +#else
> +		exit(1);
> +#endif
> +	}
> +
> +	if (!tune_opts_requested()) {
> +		/* printf("No further tune opts left\n"); */
> +#ifdef BUILD_AS_LIB
> +		return 0;
> +#else
> +		exit(0);
>  #endif
> -			return rc;
> -		}
> -		rc = 0;
>  	}
>  
>  retry_open:
> @@ -3338,16 +3540,6 @@ retry_open:
>  		goto closefs;
>  	}
>  
> -	retval = ext2fs_check_mount_point(device_name, &mount_flags,
> -					mntpt, sizeof(mntpt));
> -	if (retval) {
> -		com_err("ext2fs_check_mount_point", retval,
> -			_("while determining whether %s is mounted."),
> -			device_name);
> -		rc = 1;
> -		goto closefs;
> -	}
> -
>  #ifdef NO_RECOVERY
>  	/* Warn if file system needs recovery and it is opened for writing. */
>  	if ((open_flag & EXT2_FLAG_RW) && !(mount_flags & EXT2_MF_MOUNTED) &&
> @@ -3382,9 +3574,6 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
>  	fs->flags |= EXT2_FLAG_SUPER_ONLY;
>  
>  	if (opts[OPT_MAX_MOUNTCOUNT]) {
> -		if (max_mount_count == 65536)
> -			max_mount_count = EXT2_DFL_MAX_MNT_COUNT +
> -				(random() % EXT2_DFL_MAX_MNT_COUNT);
>  		sb->s_max_mnt_count = max_mount_count;
>  		ext2fs_mark_super_dirty(fs);
>  		printf(_("Setting maximal mount count to %d\n"),
> @@ -3410,13 +3599,6 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
>  		}
>  	}
>  	if (opts[OPT_CHECKINTERVAL]) {
> -		if ((unsigned long long)interval >= (1ULL << 32)) {
> -			com_err(program_name, 0,
> -				_("interval between checks is too big (%lu)"),
> -				interval);
> -			rc = 1;
> -			goto closefs;
> -		}
>  		sb->s_checkinterval = interval;
>  		ext2fs_mark_super_dirty(fs);
>  		printf(_("Setting interval between checks to %lu seconds\n"),
> @@ -3494,7 +3676,7 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
>  			sizeof(sb->s_last_mounted));
>  		ext2fs_mark_super_dirty(fs);
>  	}
> -	if (mntopts_cmd) {
> +	if (opts[OPT_MNTOPTS]) {
>  		rc = update_mntopts(fs, mntopts_cmd);
>  		if (rc)
>  			goto closefs;
> -- 
> 2.51.0
> 
> 

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

* Re: [PATCH 3/3] tune2fs: try to use the SET_TUNE_SB_PARAM ioctl on mounted file systems
  2025-09-18 17:47   ` Darrick J. Wong
@ 2025-09-19 15:01     ` Theodore Ts'o
  0 siblings, 0 replies; 9+ messages in thread
From: Theodore Ts'o @ 2025-09-19 15:01 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Ext4 Developers List

On Thu, Sep 18, 2025 at 10:47:24AM -0700, Darrick J. Wong wrote:
> > -#else
> > -	return -1;
> 
> Shouldn't this still return 1 if this isn't being built on __linux__?
> 
> >  #endif
> > +	return 0;
> >  }

The calling convention for try_mounted_tune2fs() is to return -1 if
the caller should bail out and exit; and 0 if we should fall back to
modifying the block device thle old fashioned way.  So that's what
should happen if tune2fs is run on FreeBSD or GNU Hurd.

I'll add a comment documenting this.

> > +	if (get_mount_flags() < 0 || try_mounted_tune2fs() << 0) {
> 
> Why shift left here                                        ^^ ??

Whoops, typo that should have been < 0.

		     	  	      	     - Ted

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

end of thread, other threads:[~2025-09-19 15:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-17  3:28 [PATCH 0/3] E2fsprogs: tune2fs: use an ioctl to update mounted fs Theodore Ts'o
2025-09-17  3:28 ` [PATCH 1/3] tune2fs: reorganize command-line flag handling Theodore Ts'o
2025-09-17  3:28 ` [PATCH 2/3] tune2fs: rework parse_extended_opts() so it only parses the option string Theodore Ts'o
2025-09-17  6:15   ` Andreas Dilger
2025-09-17  3:28 ` [PATCH 3/3] tune2fs: try to use the SET_TUNE_SB_PARAM ioctl on mounted file systems Theodore Ts'o
2025-09-18 17:47   ` Darrick J. Wong
2025-09-19 15:01     ` Theodore Ts'o
2025-09-17  6:26 ` [PATCH 0/3] E2fsprogs: tune2fs: use an ioctl to update mounted fs Andreas Dilger
2025-09-18 16:55   ` Theodore Ts'o

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).