Re: Metadata_csum is still enabled after trying to disable it.

["Theodore Y. Ts'o" <tytso@mit.edu>]

On Wed, Nov 21, 2018 at 07:01:19PM -0800, Darrick J. Wong wrote:
> > Is it a bug ?
> 
> Yes.
> 
> > How can I actually disable this parameter ?
>

Actually, the metadata_csum feature *was* disabled.  You would have
seen that if you looked at the "Features: " line printed by dumpe2fs
-h or tune2fs -l.  The bug was that we were erroneously warning that
the tune2fs -U operation would take a long time when flex_bg was
enabled, but metadata_csum was not.  So it was misleading wrong, but
it actually turning off the metadata_csum feature.

Cheers,

					- Ted

From 0eeb17d0610fd512c1038dabb39370420ffb47a4 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <tytso@mit.edu>
Date: Fri, 23 Nov 2018 22:34:31 -0500
Subject: [PATCH] tune2fs: fix false warning that a UUID change will take a
 long time

If the file system only has the flex_bg feature enabled (with out the
metadata_csum feature enabled), it won't take a long time time fix up
the checksums after changing the UUID.  While it does need to
recalculate all of the checksums in the block group descriptors, that
doesn't take a long time.

Also, if the ea_data feature is enabled, changing the UUID will also
take a long time, and we weren't warning the user about that case.

Fix up the warning message so it doesn't mislead people, and is more
accurate.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 misc/tune2fs.c           | 17 +++++++++--------
 tests/t_dangerous/expect | 29 ++++++++++++++++++++++++-----
 tests/t_dangerous/script | 33 +++++++++++++++++++++++++++------
 3 files changed, 60 insertions(+), 19 deletions(-)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index ec977b8c3..7c5ba0c77 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -3221,6 +3221,15 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 		char buf[SUPERBLOCK_SIZE] __attribute__ ((aligned(8)));
 		__u8 old_uuid[UUID_SIZE];
 
+		if (!ext2fs_has_feature_csum_seed(fs->super) &&
+		    (ext2fs_has_feature_metadata_csum(fs->super) ||
+		     ext2fs_has_feature_ea_inode(fs->super))) {
+			check_fsck_needed(fs,
+				_("Setting the UUID on this "
+				  "filesystem could take some time."));
+			rewrite_checksums = 1;
+		}
+
 		if (ext2fs_has_group_desc_csum(fs)) {
 			/*
 			 * Changing the UUID on a metadata_csum FS requires
@@ -3241,10 +3250,6 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 				try_confirm_csum_seed_support();
 				exit(1);
 			}
-			if (!ext2fs_has_feature_csum_seed(fs->super))
-				check_fsck_needed(fs,
-					_("Setting UUID on a checksummed "
-					  "filesystem could take some time."));
 
 			/*
 			 * Determine if the block group checksums are
@@ -3302,10 +3307,6 @@ _("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
 		}
 
 		ext2fs_mark_super_dirty(fs);
-		if (!ext2fs_has_feature_csum_seed(fs->super) &&
-		    (ext2fs_has_feature_metadata_csum(fs->super) ||
-		     ext2fs_has_feature_ea_inode(fs->super)))
-			rewrite_checksums = 1;
 	}
 
 	if (I_flag) {
diff --git a/tests/t_dangerous/expect b/tests/t_dangerous/expect
index a9903b750..31aaf4fad 100644
--- a/tests/t_dangerous/expect
+++ b/tests/t_dangerous/expect
@@ -43,10 +43,6 @@ tune2fs -I 512 test.img
 Resizing inodes could take some time.
 Proceed anyway (or wait 5 seconds to proceed) ? (y,N) 
 Exit status is 1
-tune2fs -U random test.img
-Setting UUID on a checksummed filesystem could take some time.
-Proceed anyway (or wait 5 seconds to proceed) ? (y,N) 
-Exit status is 1
 
 Change in FS metadata:
 Pass 1: Checking inodes, blocks, and sizes
@@ -79,7 +75,7 @@ Resizing inodes could take some time.
 Proceed anyway (or wait 5 seconds to proceed) ? (y,N) Setting inode size 512
 Exit status is 0
 tune2fs -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 test.img
-Setting UUID on a checksummed filesystem could take some time.
+Setting the UUID on this filesystem could take some time.
 Proceed anyway (or wait 5 seconds to proceed) ? (y,N) Exit status is 0
 Backing up journal inode block information.
 
@@ -99,3 +95,26 @@ Pass 4: Checking reference counts
 Pass 5: Checking group summary information
 
 Exit status is 0
+ 
+Testing with metadata checksum enabled
+Creating filesystem with 524288 1k blocks and 65536 inodes
+Superblock backups stored on blocks: 
+	8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409
+
+Allocating group tables:      \b\b\b\b\bdone                            
+Writing inode tables:      \b\b\b\b\bdone                            
+Creating journal (16384 blocks): done
+Creating 445 huge file(s) with 1024 blocks each: done
+Writing superblocks and filesystem accounting information:      \b\b\b\b\bdone
+
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+
+Exit status is 0
+tune2fs -U random test.img
+Setting the UUID on this filesystem could take some time.
+Proceed anyway (or wait 5 seconds to proceed) ? (y,N) 
+Exit status is 1
diff --git a/tests/t_dangerous/script b/tests/t_dangerous/script
index b71421881..f684d56d3 100644
--- a/tests/t_dangerous/script
+++ b/tests/t_dangerous/script
@@ -17,6 +17,18 @@ cat > $CONF << ENDL
 		hugefiles_size = 1M
 		zero_hugefiles = false
 	}
+	ext4m = {
+		features = has_journal,extent,huge_file,^flex_bg,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,resize_inode,64bit,metadata_csum
+		blocksize = 1024
+		inode_size = 256
+		make_hugefiles = true
+		hugefiles_dir = /
+		hugefiles_slack = 32M
+		hugefiles_name = aaaaa
+		hugefiles_digits = 4
+		hugefiles_size = 1M
+		zero_hugefiles = false
+	}
 ENDL
 
 echo "tune2fs dangerous prompts test" > $OUT
@@ -62,12 +74,6 @@ echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -I 512 $TMPFILE >> $OUT 2>&1
 status=$?
 echo Exit status is $status >> $OUT
 
-# change uuid
-echo "tune2fs -U random test.img" >> $OUT
-echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -U random $TMPFILE >> $OUT 2>&1
-status=$?
-echo Exit status is $status >> $OUT
-
 # check
 $FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
 
@@ -111,6 +117,21 @@ $FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
 status=$?
 echo Exit status is $status >> $OUT
 
+echo " " >> $OUT
+echo "Testing with metadata checksum enabled" >> $OUT
+
+MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4m -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
+$DUMPE2FS $TMPFILE 2> /dev/null | grep '^Group 0:' -B99 -A20 | sed -f $cmd_dir/filter.sed > $OUT.before
+$FSCK $FSCK_OPT -f -N test_filesys $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# change uuid
+echo "tune2fs -U random test.img" >> $OUT
+echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -U random $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
 rm $TMPFILE $OUT.before $OUT.after $CONF
 
 #
-- 
2.19.1