All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] e2fsprogs: save checksum seeds; fix broken xattr editing; misc fixes
@ 2016-02-13 22:37 Darrick J. Wong
  2016-02-13 22:37 ` [PATCH 1/9] libext2fs: store checksum seed in superblock Darrick J. Wong
                   ` (8 more replies)
  0 siblings, 9 replies; 24+ messages in thread
From: Darrick J. Wong @ 2016-02-13 22:37 UTC (permalink / raw)
  To: tytso, darrick.wong; +Cc: linux-ext4

Hi all,

Here's a short patch series that rolls up all my pending fixes for
e2fsprogs.  The first five patches introduce the "metdata_csum_seed"
feature which decouples the metadata block checksums from the FS UUID
so that said UUID can be changed while the FS is mounted.

Patch #6 fixes filefrag not to report holes as fragmentation.

Patch #7 changes tune2fs so that prior to making invasive changes to
the filesystem it'll check that the FS has been fsck'd recently and if
so confirms with the administrator that they actually want to proceed.

Patch #8 teaches tune2fs to recover the journal if necessary.  This
prevents tune2fs modifications from being clobbered by a journal
replay.

Patch #9 fixes a bug reported by Richard Purdie wherein we didn't sort
extended attributes in an external block according to the kernel's
expectations, which results in mke2fs creating attributes that the
kernel cannot read.

The patchset should apply cleanly against the -next branch as of 13
February (it hasn't been updated since 30 November 2015).  Comments
and questions are, as always, welcome.

(Except for patch #9, none of the patches in this set have changed
since the last posting on 4 Dec 2015.)

--D

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

* [PATCH 1/9] libext2fs: store checksum seed in superblock
  2016-02-13 22:37 [PATCH 0/9] e2fsprogs: save checksum seeds; fix broken xattr editing; misc fixes Darrick J. Wong
@ 2016-02-13 22:37 ` Darrick J. Wong
  2016-03-05 23:21   ` Theodore Ts'o
  2016-02-13 22:37 ` [PATCH 2/9] tune2fs: allow user to turn on saving the checksum seed Darrick J. Wong
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Darrick J. Wong @ 2016-02-13 22:37 UTC (permalink / raw)
  To: tytso, darrick.wong; +Cc: linux-ext4

Allow the filesystem to store the metadata checksum seed in the
superblock and add an incompat feature to say that we're using it.
This enables tune2fs to change the UUID on a mounted metadata_csum
FS without having to (racy!) rewrite all disk metadata.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 debugfs/set_fields.c        |    1 +
 lib/e2p/feature.c           |    2 ++
 lib/e2p/ls.c                |    4 ++++
 lib/ext2fs/csum.c           |    9 +++++++++
 lib/ext2fs/ext2_fs.h        |    7 +++++--
 lib/ext2fs/ext2fs.h         |   13 +++----------
 lib/ext2fs/swapfs.c         |    1 +
 lib/ext2fs/tst_super_size.c |    4 +++-
 8 files changed, 28 insertions(+), 13 deletions(-)


diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c
index ec2340d..5b13c34 100644
--- a/debugfs/set_fields.c
+++ b/debugfs/set_fields.c
@@ -173,6 +173,7 @@ static struct field_set_info super_fields[] = {
 	  FLAG_ARRAY, 4 },
 	{ "encrypt_pw_salt", &set_sb.s_encrypt_pw_salt, NULL, 16, parse_uuid },
 	{ "lpf_ino", &set_sb.s_lpf_ino, NULL, 4, parse_uint },
+	{ "checksum_seed", &set_sb.s_checksum_seed, NULL, 4, parse_uint },
 	{ 0, 0, 0, 0 }
 };
 
diff --git a/lib/e2p/feature.c b/lib/e2p/feature.c
index 737b0b9..35e8435 100644
--- a/lib/e2p/feature.c
+++ b/lib/e2p/feature.c
@@ -95,6 +95,8 @@ static struct feature feature_list[] = {
 			"ea_inode"},
 	{       E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_DIRDATA,
 			"dirdata"},
+	{       E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_CSUM_SEED,
+			"metadata_csum_seed"},
 	{       E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_LARGEDIR,
 			"large_dir"},
 	{       E2P_FEATURE_INCOMPAT, EXT4_FEATURE_INCOMPAT_INLINE_DATA,
diff --git a/lib/e2p/ls.c b/lib/e2p/ls.c
index 6c82857..8df6bb2 100644
--- a/lib/e2p/ls.c
+++ b/lib/e2p/ls.c
@@ -450,6 +450,10 @@ void list_super2(struct ext2_super_block * sb, FILE *f)
 	if (!e2p_is_null_uuid(sb->s_encrypt_pw_salt))
 		fprintf(f, "Encryption PW Salt:       %s\n",
 			e2p_uuid2str(sb->s_encrypt_pw_salt));
+
+	if (ext2fs_has_feature_csum_seed(sb))
+		fprintf(f, "Checksum seed:            0x%08x\n",
+			sb->s_checksum_seed);
 }
 
 void list_super (struct ext2_super_block * s)
diff --git a/lib/ext2fs/csum.c b/lib/ext2fs/csum.c
index ab8b83f..ccb3057 100644
--- a/lib/ext2fs/csum.c
+++ b/lib/ext2fs/csum.c
@@ -30,6 +30,15 @@
 #define STATIC static
 #endif
 
+void ext2fs_init_csum_seed(ext2_filsys fs)
+{
+	if (ext2fs_has_feature_csum_seed(fs->super))
+		fs->csum_seed = fs->super->s_checksum_seed;
+	else if (ext2fs_has_feature_metadata_csum(fs->super))
+		fs->csum_seed = ext2fs_crc32c_le(~0, fs->super->s_uuid,
+						 sizeof(fs->super->s_uuid));
+}
+
 static __u32 ext2fs_mmp_csum(ext2_filsys fs, struct mmp_struct *mmp)
 {
 	int offset = offsetof(struct mmp_struct, mmp_checksum);
diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
index 1f62c58..d884586 100644
--- a/lib/ext2fs/ext2_fs.h
+++ b/lib/ext2fs/ext2_fs.h
@@ -719,7 +719,9 @@ struct ext2_super_block {
 	__u8	s_encrypt_algos[4];	/* Encryption algorithms in use  */
 	__u8	s_encrypt_pw_salt[16];	/* Salt used for string2key algorithm */
 	__le32	s_lpf_ino;		/* Location of the lost+found inode */
-	__le32	s_reserved[100];	/* Padding to the end of the block */
+	__le32	s_prj_quota_inum;	/* inode for tracking project quota */
+	__le32	s_checksum_seed;	/* crc32c(orig_uuid) if csum_seed set */
+	__le32	s_reserved[98];		/* Padding to the end of the block */
 	__u32	s_checksum;		/* crc32c(superblock) */
 };
 
@@ -804,7 +806,7 @@ struct ext2_super_block {
 #define EXT4_FEATURE_INCOMPAT_FLEX_BG		0x0200
 #define EXT4_FEATURE_INCOMPAT_EA_INODE		0x0400
 #define EXT4_FEATURE_INCOMPAT_DIRDATA		0x1000
-/* 0x2000 was EXT4_FEATURE_INCOMPAT_BG_USE_META_CSUM but this was never used */
+#define EXT4_FEATURE_INCOMPAT_CSUM_SEED		0x2000
 #define EXT4_FEATURE_INCOMPAT_LARGEDIR		0x4000 /* >2GB or 3-lvl htree */
 #define EXT4_FEATURE_INCOMPAT_INLINE_DATA	0x8000 /* data in inode */
 #define EXT4_FEATURE_INCOMPAT_ENCRYPT		0x10000
@@ -894,6 +896,7 @@ EXT4_FEATURE_INCOMPAT_FUNCS(mmp,		4, MMP)
 EXT4_FEATURE_INCOMPAT_FUNCS(flex_bg,		4, FLEX_BG)
 EXT4_FEATURE_INCOMPAT_FUNCS(ea_inode,		4, EA_INODE)
 EXT4_FEATURE_INCOMPAT_FUNCS(dirdata,		4, DIRDATA)
+EXT4_FEATURE_INCOMPAT_FUNCS(csum_seed,		4, CSUM_SEED)
 EXT4_FEATURE_INCOMPAT_FUNCS(largedir,		4, LARGEDIR)
 EXT4_FEATURE_INCOMPAT_FUNCS(inline_data,	4, INLINE_DATA)
 EXT4_FEATURE_INCOMPAT_FUNCS(encrypt,		4, ENCRYPT)
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 30e913c..d9ea105 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -580,7 +580,8 @@ typedef struct ext2_icount *ext2_icount_t;
 					 EXT4_LIB_INCOMPAT_MMP|\
 					 EXT4_FEATURE_INCOMPAT_64BIT|\
 					 EXT4_FEATURE_INCOMPAT_INLINE_DATA|\
-					 EXT4_FEATURE_INCOMPAT_ENCRYPT)
+					 EXT4_FEATURE_INCOMPAT_ENCRYPT|\
+					 EXT4_FEATURE_INCOMPAT_CSUM_SEED)
 
 #define EXT2_LIB_FEATURE_RO_COMPAT_SUPP	(EXT2_FEATURE_RO_COMPAT_SPARSE_SUPER|\
 					 EXT4_FEATURE_RO_COMPAT_HUGE_FILE|\
@@ -985,6 +986,7 @@ extern __u32 ext2fs_crc32_be(__u32 crc, unsigned char const *p, size_t len);
 extern __u32 ext2fs_crc32c_le(__u32 crc, unsigned char const *p, size_t len);
 
 /* csum.c */
+extern void ext2fs_init_csum_seed(ext2_filsys fs);
 extern errcode_t ext2fs_mmp_csum_set(ext2_filsys fs, struct mmp_struct *mmp);
 extern int ext2fs_mmp_csum_verify(ext2_filsys, struct mmp_struct *mmp);
 extern int ext2fs_verify_csum_type(ext2_filsys fs, struct ext2_super_block *sb);
@@ -1712,15 +1714,6 @@ extern void ext2fs_dirent_set_file_type(struct ext2_dir_entry *entry, int type);
 #endif /* __STDC_VERSION__ >= 199901L */
 #endif
 
-static inline void ext2fs_init_csum_seed(ext2_filsys fs)
-{
-	if (!ext2fs_has_feature_metadata_csum(fs->super))
-		return;
-
-	fs->csum_seed = ext2fs_crc32c_le(~0, fs->super->s_uuid,
-					 sizeof(fs->super->s_uuid));
-}
-
 #ifndef EXT2_CUSTOM_MEMORY_ROUTINES
 #include <string.h>
 /*
diff --git a/lib/ext2fs/swapfs.c b/lib/ext2fs/swapfs.c
index ee7a455..9d2c535 100644
--- a/lib/ext2fs/swapfs.c
+++ b/lib/ext2fs/swapfs.c
@@ -101,6 +101,7 @@ void ext2fs_swap_super(struct ext2_super_block * sb)
 		sb->s_jnl_blocks[i] = ext2fs_swab32(sb->s_jnl_blocks[i]);
 	sb->s_backup_bgs[0] = ext2fs_swab32(sb->s_backup_bgs[0]);
 	sb->s_backup_bgs[1] = ext2fs_swab32(sb->s_backup_bgs[1]);
+	sb->s_checksum_seed = ext2fs_swab32(sb->s_checksum_seed);
 }
 
 void ext2fs_swap_group_desc2(ext2_filsys fs, struct ext2_group_desc *gdp)
diff --git a/lib/ext2fs/tst_super_size.c b/lib/ext2fs/tst_super_size.c
index 8e3c21f..0adac41 100644
--- a/lib/ext2fs/tst_super_size.c
+++ b/lib/ext2fs/tst_super_size.c
@@ -140,7 +140,9 @@ int main(int argc, char **argv)
 	check_field(s_encrypt_algos, 4);
 	check_field(s_encrypt_pw_salt, 16);
 	check_field(s_lpf_ino, 4);
-	check_field(s_reserved, 100 * 4);
+	check_field(s_prj_quota_inum, 4);
+	check_field(s_checksum_seed, 4);
+	check_field(s_reserved, 98 * 4);
 	check_field(s_checksum, 4);
 	do_field("Superblock end", 0, 0, cur_offset, 1024);
 #endif


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

* [PATCH 2/9] tune2fs: allow user to turn on saving the checksum seed
  2016-02-13 22:37 [PATCH 0/9] e2fsprogs: save checksum seeds; fix broken xattr editing; misc fixes Darrick J. Wong
  2016-02-13 22:37 ` [PATCH 1/9] libext2fs: store checksum seed in superblock Darrick J. Wong
@ 2016-02-13 22:37 ` Darrick J. Wong
  2016-03-05 23:36   ` Theodore Ts'o
  2016-02-13 22:37 ` [PATCH 3/9] e2fsck: check the checksum seed feature flag is set correctly Darrick J. Wong
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Darrick J. Wong @ 2016-02-13 22:37 UTC (permalink / raw)
  To: tytso, darrick.wong; +Cc: linux-ext4

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 misc/tune2fs.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 6 deletions(-)


diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 4d57399..f9e654f 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -149,7 +149,8 @@ static __u32 ok_features[3] = {
 		EXT4_FEATURE_INCOMPAT_FLEX_BG |
 		EXT4_FEATURE_INCOMPAT_MMP |
 		EXT4_FEATURE_INCOMPAT_64BIT |
-		EXT4_FEATURE_INCOMPAT_ENCRYPT,
+		EXT4_FEATURE_INCOMPAT_ENCRYPT |
+		EXT4_FEATURE_INCOMPAT_CSUM_SEED,
 	/* R/O compat */
 	EXT2_FEATURE_RO_COMPAT_LARGE_FILE |
 		EXT4_FEATURE_RO_COMPAT_HUGE_FILE|
@@ -171,7 +172,8 @@ static __u32 clear_ok_features[3] = {
 	EXT2_FEATURE_INCOMPAT_FILETYPE |
 		EXT4_FEATURE_INCOMPAT_FLEX_BG |
 		EXT4_FEATURE_INCOMPAT_MMP |
-		EXT4_FEATURE_INCOMPAT_64BIT,
+		EXT4_FEATURE_INCOMPAT_64BIT |
+		EXT4_FEATURE_INCOMPAT_CSUM_SEED,
 	/* R/O compat */
 	EXT2_FEATURE_RO_COMPAT_LARGE_FILE |
 		EXT4_FEATURE_RO_COMPAT_HUGE_FILE|
@@ -948,6 +950,15 @@ static errcode_t disable_uninit_bg(ext2_filsys fs, __u32 csum_feature_flag)
 	return 0;
 }
 
+static void
+try_confirm_csum_seed_support(void)
+{
+	if (access("/sys/fs/ext4/features/metadata_csum_seed", R_OK))
+		fputs(_("WARNING: Could not confirm kernel support for "
+			"metadata_csum_seed.\n  This requires Linux >= "
+			"v4.4.\n"), stderr);
+}
+
 /*
  * Update the feature set as provided by the user.
  */
@@ -1212,6 +1223,8 @@ mmp_error:
 			 */
 			old_features[E2P_FEATURE_RO_INCOMPAT] |=
 				EXT4_FEATURE_RO_COMPAT_GDT_CSUM;
+		fs->super->s_checksum_seed = 0;
+		ext2fs_clear_feature_csum_seed(fs->super);
 	}
 
 	if (FEATURE_ON(E2P_FEATURE_RO_INCOMPAT,
@@ -1294,6 +1307,37 @@ mmp_error:
 			EXT4_ENCRYPTION_MODE_AES_256_CTS;
 	}
 
+	if (FEATURE_ON(E2P_FEATURE_INCOMPAT,
+		EXT4_FEATURE_INCOMPAT_CSUM_SEED)) {
+		if (!ext2fs_has_feature_metadata_csum(sb)) {
+			fputs(_("Setting feature 'metadata_csum_seed' "
+				"is only supported\non filesystems with "
+				"the metadata_csum feature enabled.\n"),
+				stderr);
+			return 1;
+		}
+		try_confirm_csum_seed_support();
+		fs->super->s_checksum_seed = fs->csum_seed;
+	}
+
+	if (FEATURE_OFF(E2P_FEATURE_INCOMPAT,
+		EXT4_FEATURE_INCOMPAT_CSUM_SEED)) {
+		__le32 uuid_seed;
+
+		uuid_seed = ext2fs_crc32c_le(~0, fs->super->s_uuid,
+					sizeof(fs->super->s_uuid));
+		if (fs->super->s_checksum_seed != uuid_seed &&
+		    (mount_flags & EXT2_MF_MOUNTED)) {
+			fputs(_("UUID has changed since enabling "
+				"metadata_csum.  Filesystem must be unmounted "
+				"\nto safely rewrite all metadata to "
+				"match the new UUID.\n"), stderr);
+			return 1;
+		}
+
+		rewrite_checksums = 1;
+	}
+
 	if (sb->s_rev_level == EXT2_GOOD_OLD_REV &&
 	    (sb->s_feature_compat || sb->s_feature_ro_compat ||
 	     sb->s_feature_incompat))
@@ -2937,13 +2981,22 @@ retry_open:
 
 		if (ext2fs_has_group_desc_csum(fs)) {
 			/*
-			 * Changing the UUID requires rewriting all metadata,
-			 * which can race with a mounted fs.  Don't allow that.
+			 * Changing the UUID on a metadata_csum FS requires
+			 * rewriting all metadata, which can race with a
+			 * mounted fs.  Don't allow that unless we're saving
+			 * the checksum seed.
 			 */
-			if ((mount_flags & EXT2_MF_MOUNTED) && !f_flag) {
+			if ((mount_flags & EXT2_MF_MOUNTED) &&
+			    !ext2fs_has_feature_csum_seed(fs->super) &&
+			    ext2fs_has_feature_metadata_csum(fs->super)) {
 				fputs(_("The UUID may only be "
 					"changed when the filesystem is "
 					"unmounted.\n"), stderr);
+				fputs(_("If you only use kernels newer than "
+					"v4.4, run 'tune2fs -O "
+					"metadata_csum_seed' and re-run this "
+					"command.\n"), stderr);
+				try_confirm_csum_seed_support();
 				exit(1);
 			}
 			if (check_fsck_needed(fs))
@@ -3005,7 +3058,8 @@ retry_open:
 		}
 
 		ext2fs_mark_super_dirty(fs);
-		if (ext2fs_has_feature_metadata_csum(fs->super))
+		if (ext2fs_has_feature_metadata_csum(fs->super) &&
+		    !ext2fs_has_feature_csum_seed(fs->super))
 			rewrite_checksums = 1;
 	}
 


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

* [PATCH 3/9] e2fsck: check the checksum seed feature flag is set correctly
  2016-02-13 22:37 [PATCH 0/9] e2fsprogs: save checksum seeds; fix broken xattr editing; misc fixes Darrick J. Wong
  2016-02-13 22:37 ` [PATCH 1/9] libext2fs: store checksum seed in superblock Darrick J. Wong
  2016-02-13 22:37 ` [PATCH 2/9] tune2fs: allow user to turn on saving the checksum seed Darrick J. Wong
@ 2016-02-13 22:37 ` Darrick J. Wong
  2016-03-05 23:37   ` Theodore Ts'o
  2016-02-13 22:37 ` [PATCH 4/9] mke2fs: store checksum seed at format time Darrick J. Wong
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Darrick J. Wong @ 2016-02-13 22:37 UTC (permalink / raw)
  To: tytso, darrick.wong; +Cc: linux-ext4

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 e2fsck/problem.c |    5 +++++
 e2fsck/problem.h |    3 +++
 e2fsck/super.c   |    9 +++++++++
 3 files changed, 17 insertions(+)


diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index a7da94d..971b0ad 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -467,6 +467,11 @@ static struct e2fsck_problem problem_table[] = {
 	  N_("External @j @S checksum does not match @S.  "),
 	  PROMPT_FIX, PR_PREEN_OK },
 
+	/* metadata_csum_seed means nothing without metadata_csum */
+	{ PR_0_CSUM_SEED_WITHOUT_META_CSUM,
+	  N_("@S metadata_csum_seed is not necessary without metadata_csum."),
+	  PROMPT_FIX, PR_PREEN_OK | PR_NO_OK},
+
 	/* Pass 1 errors */
 
 	/* Pass 1: Checking inodes, blocks, and sizes */
diff --git a/e2fsck/problem.h b/e2fsck/problem.h
index 3b92e41..b3f5b8f 100644
--- a/e2fsck/problem.h
+++ b/e2fsck/problem.h
@@ -268,6 +268,9 @@ struct problem_context {
 /* External journal has corrupt superblock */
 #define PR_0_EXT_JOURNAL_SUPER_CSUM_INVALID	0x00004A
 
+/* metadata_csum_seed means nothing without metadata_csum */
+#define PR_0_CSUM_SEED_WITHOUT_META_CSUM	0x00004B
+
 /*
  * Pass 1 errors
  */
diff --git a/e2fsck/super.c b/e2fsck/super.c
index af6d680..e09c14c 100644
--- a/e2fsck/super.c
+++ b/e2fsck/super.c
@@ -589,6 +589,15 @@ void check_super_block(e2fsck_t ctx)
 			ext2fs_group_desc_csum_set(fs, i);
 	}
 
+	/* We can't have ^metadata_csum,metadata_csum_seed */
+	if (!ext2fs_has_feature_metadata_csum(fs->super) &&
+	    ext2fs_has_feature_csum_seed(fs->super) &&
+	    fix_problem(ctx, PR_0_CSUM_SEED_WITHOUT_META_CSUM, &pctx)) {
+		ext2fs_clear_feature_csum_seed(fs->super);
+		fs->super->s_checksum_seed = 0;
+		ext2fs_mark_super_dirty(fs);
+	}
+
 	/* Is 64bit set and extents unset? */
 	if (ext2fs_has_feature_64bit(fs->super) &&
 	    !ext2fs_has_feature_extents(fs->super) &&


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

* [PATCH 4/9] mke2fs: store checksum seed at format time
  2016-02-13 22:37 [PATCH 0/9] e2fsprogs: save checksum seeds; fix broken xattr editing; misc fixes Darrick J. Wong
                   ` (2 preceding siblings ...)
  2016-02-13 22:37 ` [PATCH 3/9] e2fsck: check the checksum seed feature flag is set correctly Darrick J. Wong
@ 2016-02-13 22:37 ` Darrick J. Wong
  2016-03-06  0:19   ` Theodore Ts'o
  2016-02-13 22:37 ` [PATCH 5/9] tests: check proper operation of metadata_csum_seed Darrick J. Wong
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Darrick J. Wong @ 2016-02-13 22:37 UTC (permalink / raw)
  To: tytso, darrick.wong; +Cc: linux-ext4

Allow users to turn on metadata_csum_seed at format time so that UUIDs
can be live-changed at any time.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 misc/mke2fs.c |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)


diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index dd467f2..7d31e42 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -1074,7 +1074,8 @@ static __u32 ok_features[3] = {
 		EXT4_FEATURE_INCOMPAT_MMP |
 		EXT4_FEATURE_INCOMPAT_64BIT|
 		EXT4_FEATURE_INCOMPAT_INLINE_DATA|
-		EXT4_FEATURE_INCOMPAT_ENCRYPT,
+		EXT4_FEATURE_INCOMPAT_ENCRYPT |
+		EXT4_FEATURE_INCOMPAT_CSUM_SEED,
 	/* R/O compat */
 	EXT2_FEATURE_RO_COMPAT_LARGE_FILE|
 		EXT4_FEATURE_RO_COMPAT_HUGE_FILE|
@@ -2774,6 +2775,13 @@ int main (int argc, char *argv[])
 				 "Pass -O 64bit to rectify.\n"));
 	}
 
+	if (ext2fs_has_feature_csum_seed(fs->super) &&
+	    !ext2fs_has_feature_metadata_csum(fs->super)) {
+		printf("%s", _("The metadata_csum_seed feature "
+			       "requres the metadata_csum feature.\n"));
+		exit(1);
+	}
+
 	/* Calculate journal blocks */
 	if (!journal_device && ((journal_size) ||
 	    ext2fs_has_feature_journal(&fs_param)))
@@ -2829,6 +2837,11 @@ int main (int argc, char *argv[])
 		}
 	} else
 		uuid_generate(fs->super->s_uuid);
+
+	if (ext2fs_has_feature_csum_seed(fs->super))
+		fs->super->s_checksum_seed = ext2fs_crc32c_le(~0,
+				fs->super->s_uuid, sizeof(fs->super->s_uuid));
+
 	ext2fs_init_csum_seed(fs);
 
 	/*


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

* [PATCH 5/9] tests: check proper operation of metadata_csum_seed
  2016-02-13 22:37 [PATCH 0/9] e2fsprogs: save checksum seeds; fix broken xattr editing; misc fixes Darrick J. Wong
                   ` (3 preceding siblings ...)
  2016-02-13 22:37 ` [PATCH 4/9] mke2fs: store checksum seed at format time Darrick J. Wong
@ 2016-02-13 22:37 ` Darrick J. Wong
  2016-03-06  0:20   ` Theodore Ts'o
  2016-02-13 22:38 ` [PATCH 6/9] filefrag: accommodate holes when calculating expected values Darrick J. Wong
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Darrick J. Wong @ 2016-02-13 22:37 UTC (permalink / raw)
  To: tytso, darrick.wong; +Cc: linux-ext4

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 lib/ext2fs/ismounted.c                           |   14 ++++++
 tests/t_change_uuid/expect                       |    7 +++
 tests/t_change_uuid/script                       |   34 ++++++++++++++
 tests/t_change_uuid_mcsum/expect                 |    7 +++
 tests/t_change_uuid_mcsum/script                 |   34 ++++++++++++++
 tests/t_change_uuid_mcsum_mounted/expect         |    7 +++
 tests/t_change_uuid_mcsum_mounted/script         |   34 ++++++++++++++
 tests/t_change_uuid_mcsum_seed_mounted/expect    |   21 +++++++++
 tests/t_change_uuid_mcsum_seed_mounted/script    |   52 ++++++++++++++++++++++
 tests/t_change_uuid_mounted/expect               |    7 +++
 tests/t_change_uuid_mounted/script               |   34 ++++++++++++++
 tests/t_disable_changed_csum_seed/expect         |   17 +++++++
 tests/t_disable_changed_csum_seed/script         |   47 ++++++++++++++++++++
 tests/t_disable_changed_csum_seed_mounted/expect |   19 ++++++++
 tests/t_disable_changed_csum_seed_mounted/script |   47 ++++++++++++++++++++
 tests/t_disable_csum_seed/expect                 |   14 ++++++
 tests/t_disable_csum_seed/script                 |   43 ++++++++++++++++++
 tests/t_disable_meta_csum_and_seed/expect        |   14 ++++++
 tests/t_disable_meta_csum_and_seed/script        |   44 +++++++++++++++++++
 tests/t_enable_csum_seed/expect                  |   12 +++++
 tests/t_enable_csum_seed/script                  |   38 ++++++++++++++++
 tests/t_format_csum_seed/expect                  |   10 ++++
 tests/t_format_csum_seed/script                  |   34 ++++++++++++++
 23 files changed, 590 insertions(+)
 create mode 100644 tests/t_change_uuid/expect
 create mode 100755 tests/t_change_uuid/script
 create mode 100644 tests/t_change_uuid_mcsum/expect
 create mode 100755 tests/t_change_uuid_mcsum/script
 create mode 100644 tests/t_change_uuid_mcsum_mounted/expect
 create mode 100755 tests/t_change_uuid_mcsum_mounted/script
 create mode 100644 tests/t_change_uuid_mcsum_seed_mounted/expect
 create mode 100755 tests/t_change_uuid_mcsum_seed_mounted/script
 create mode 100644 tests/t_change_uuid_mounted/expect
 create mode 100755 tests/t_change_uuid_mounted/script
 create mode 100644 tests/t_disable_changed_csum_seed/expect
 create mode 100755 tests/t_disable_changed_csum_seed/script
 create mode 100644 tests/t_disable_changed_csum_seed_mounted/expect
 create mode 100755 tests/t_disable_changed_csum_seed_mounted/script
 create mode 100644 tests/t_disable_csum_seed/expect
 create mode 100755 tests/t_disable_csum_seed/script
 create mode 100644 tests/t_disable_meta_csum_and_seed/expect
 create mode 100755 tests/t_disable_meta_csum_and_seed/script
 create mode 100644 tests/t_enable_csum_seed/expect
 create mode 100755 tests/t_enable_csum_seed/script
 create mode 100644 tests/t_format_csum_seed/expect
 create mode 100755 tests/t_format_csum_seed/script


diff --git a/lib/ext2fs/ismounted.c b/lib/ext2fs/ismounted.c
index b1eff85..ae2e832 100644
--- a/lib/ext2fs/ismounted.c
+++ b/lib/ext2fs/ismounted.c
@@ -90,6 +90,20 @@ static errcode_t check_mntent_file(const char *mtab_file, const char *file,
 	int		fd;
 
 	*mount_flags = 0;
+
+	if (getenv("EXT2FS_PRETEND_RO_MOUNT")) {
+		*mount_flags = EXT2_MF_MOUNTED | EXT2_MF_READONLY;
+		if (getenv("EXT2FS_PRETEND_ROOTFS"))
+			*mount_flags = EXT2_MF_ISROOT;
+		return 0;
+	}
+	if (getenv("EXT2FS_PRETEND_RW_MOUNT")) {
+		*mount_flags = EXT2_MF_MOUNTED;
+		if (getenv("EXT2FS_PRETEND_ROOTFS"))
+			*mount_flags = EXT2_MF_ISROOT;
+		return 0;
+	}
+
 	if ((f = setmntent (mtab_file, "r")) == NULL) {
 		if (errno == ENOENT) {
 			if (getenv("EXT2FS_NO_MTAB_OK"))
diff --git a/tests/t_change_uuid/expect b/tests/t_change_uuid/expect
new file mode 100644
index 0000000..0a9b20e
--- /dev/null
+++ b/tests/t_change_uuid/expect
@@ -0,0 +1,7 @@
+create fs without metadata_csum
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+change UUID
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
+check filesystem
+fsck returns 0
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
diff --git a/tests/t_change_uuid/script b/tests/t_change_uuid/script
new file mode 100755
index 0000000..be8cbfa
--- /dev/null
+++ b/tests/t_change_uuid/script
@@ -0,0 +1,34 @@
+test_description="change uuid on a pre-metadata-csum"
+
+trap "rm -rf $TMPFILE $TMPFILE.conf" EXIT INT QUIT
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+OUT=$test_name.log
+EXP=$test_dir/expect
+rm -rf $OUT
+
+# Test command line option
+echo "create fs without metadata_csum" >> $OUT
+$MKE2FS -O ^metadata_csum,^metadata_csum_seed -U 6b33f586-a183-4383-921d-30da3fef2e5c -F $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "change UUID" >> $OUT
+$TUNE2FS -U a61be2e0-b3b8-4fbc-b2cd-d84b306b9731 $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+
diff --git a/tests/t_change_uuid_mcsum/expect b/tests/t_change_uuid_mcsum/expect
new file mode 100644
index 0000000..fb26e72
--- /dev/null
+++ b/tests/t_change_uuid_mcsum/expect
@@ -0,0 +1,7 @@
+create fs with metadata_csum
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+change UUID
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
+check filesystem
+fsck returns 0
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
diff --git a/tests/t_change_uuid_mcsum/script b/tests/t_change_uuid_mcsum/script
new file mode 100755
index 0000000..826d287
--- /dev/null
+++ b/tests/t_change_uuid_mcsum/script
@@ -0,0 +1,34 @@
+test_description="change uuid on a metadata-csum"
+
+trap "rm -rf $TMPFILE $TMPFILE.conf" EXIT INT QUIT
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+OUT=$test_name.log
+EXP=$test_dir/expect
+rm -rf $OUT
+
+# Test command line option
+echo "create fs with metadata_csum" >> $OUT
+$MKE2FS -O metadata_csum,^metadata_csum_seed -U 6b33f586-a183-4383-921d-30da3fef2e5c -F $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "change UUID" >> $OUT
+$TUNE2FS -U a61be2e0-b3b8-4fbc-b2cd-d84b306b9731 $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+
diff --git a/tests/t_change_uuid_mcsum_mounted/expect b/tests/t_change_uuid_mcsum_mounted/expect
new file mode 100644
index 0000000..e10e9a9
--- /dev/null
+++ b/tests/t_change_uuid_mcsum_mounted/expect
@@ -0,0 +1,7 @@
+create fs with metadata_csum
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+change UUID
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+check filesystem
+fsck returns 0
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
diff --git a/tests/t_change_uuid_mcsum_mounted/script b/tests/t_change_uuid_mcsum_mounted/script
new file mode 100755
index 0000000..0efcae5
--- /dev/null
+++ b/tests/t_change_uuid_mcsum_mounted/script
@@ -0,0 +1,34 @@
+test_description="change uuid on a metadata-csum"
+
+trap "rm -rf $TMPFILE $TMPFILE.conf" EXIT INT QUIT
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+OUT=$test_name.log
+EXP=$test_dir/expect
+rm -rf $OUT
+
+# Test command line option
+echo "create fs with metadata_csum" >> $OUT
+$MKE2FS -O metadata_csum,^metadata_csum_seed -U 6b33f586-a183-4383-921d-30da3fef2e5c -F $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "change UUID" >> $OUT
+EXT2FS_PRETEND_RW_MOUNT=1 $TUNE2FS -U a61be2e0-b3b8-4fbc-b2cd-d84b306b9731 $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+
diff --git a/tests/t_change_uuid_mcsum_seed_mounted/expect b/tests/t_change_uuid_mcsum_seed_mounted/expect
new file mode 100644
index 0000000..b2d2d91
--- /dev/null
+++ b/tests/t_change_uuid_mcsum_seed_mounted/expect
@@ -0,0 +1,21 @@
+create fs with metadata_csum
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+change UUID
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+check filesystem
+fsck returns 0
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+turn on csum seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+Checksum seed:            0x3ba62721
+check filesystem
+fsck returns 0
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+Checksum seed:            0x3ba62721
+change UUID
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
+Checksum seed:            0x3ba62721
+check filesystem
+fsck returns 0
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
+Checksum seed:            0x3ba62721
diff --git a/tests/t_change_uuid_mcsum_seed_mounted/script b/tests/t_change_uuid_mcsum_seed_mounted/script
new file mode 100755
index 0000000..05b0bd7
--- /dev/null
+++ b/tests/t_change_uuid_mcsum_seed_mounted/script
@@ -0,0 +1,52 @@
+test_description="change uuid on a metadata-csum with mcsum-seed"
+
+trap "rm -rf $TMPFILE $TMPFILE.conf" EXIT INT QUIT
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+OUT=$test_name.log
+EXP=$test_dir/expect
+rm -rf $OUT
+
+# Test command line option
+echo "create fs with metadata_csum" >> $OUT
+$MKE2FS -O metadata_csum,^metadata_csum_seed -U 6b33f586-a183-4383-921d-30da3fef2e5c -F $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "change UUID" >> $OUT
+EXT2FS_PRETEND_RW_MOUNT=1 $TUNE2FS -U a61be2e0-b3b8-4fbc-b2cd-d84b306b9731 $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "turn on csum seed" >> $OUT
+EXT2FS_PRETEND_RW_MOUNT=1 $TUNE2FS -O metadata_csuM_seed $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "change UUID" >> $OUT
+EXT2FS_PRETEND_RW_MOUNT=1 $TUNE2FS -U a61be2e0-b3b8-4fbc-b2cd-d84b306b9731 $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+
diff --git a/tests/t_change_uuid_mounted/expect b/tests/t_change_uuid_mounted/expect
new file mode 100644
index 0000000..0a9b20e
--- /dev/null
+++ b/tests/t_change_uuid_mounted/expect
@@ -0,0 +1,7 @@
+create fs without metadata_csum
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+change UUID
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
+check filesystem
+fsck returns 0
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
diff --git a/tests/t_change_uuid_mounted/script b/tests/t_change_uuid_mounted/script
new file mode 100755
index 0000000..827ff59
--- /dev/null
+++ b/tests/t_change_uuid_mounted/script
@@ -0,0 +1,34 @@
+test_description="change uuid on a mounted pre-metadata-csum"
+
+trap "rm -rf $TMPFILE $TMPFILE.conf" EXIT INT QUIT
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+OUT=$test_name.log
+EXP=$test_dir/expect
+rm -rf $OUT
+
+# Test command line option
+echo "create fs without metadata_csum" >> $OUT
+$MKE2FS -O ^metadata_csum,^metadata_csum_seed -U 6b33f586-a183-4383-921d-30da3fef2e5c -F $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "change UUID" >> $OUT
+EXT2FS_PRETEND_RW_MOUNT=1 $TUNE2FS -U a61be2e0-b3b8-4fbc-b2cd-d84b306b9731 $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+
diff --git a/tests/t_disable_changed_csum_seed/expect b/tests/t_disable_changed_csum_seed/expect
new file mode 100644
index 0000000..e00f367
--- /dev/null
+++ b/tests/t_disable_changed_csum_seed/expect
@@ -0,0 +1,17 @@
+create fs without csum_seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+turn on csum_seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+Checksum seed:            0x3ba62721
+change UUID
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
+Checksum seed:            0x3ba62721
+check filesystem
+fsck returns 0
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
+Checksum seed:            0x3ba62721
+turn off csum_seed
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
+check filesystem
+fsck returns 0
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
diff --git a/tests/t_disable_changed_csum_seed/script b/tests/t_disable_changed_csum_seed/script
new file mode 100755
index 0000000..9443731
--- /dev/null
+++ b/tests/t_disable_changed_csum_seed/script
@@ -0,0 +1,47 @@
+test_description="disable csum seed via tune2fs after changing uuid"
+
+trap "rm -rf $TMPFILE $TMPFILE.conf" EXIT INT QUIT
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+OUT=$test_name.log
+EXP=$test_dir/expect
+rm -rf $OUT
+
+# Test command line option
+echo "create fs without csum_seed" >> $OUT
+$MKE2FS -O metadata_csum,^metadata_csum_seed -U 6b33f586-a183-4383-921d-30da3fef2e5c -F $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "turn on csum_seed" >> $OUT
+$TUNE2FS -O metadata_csum_seed $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "change UUID" >> $OUT
+$TUNE2FS -U a61be2e0-b3b8-4fbc-b2cd-d84b306b9731 $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "turn off csum_seed" >> $OUT
+$TUNE2FS -O ^metadata_csum_seed $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+
diff --git a/tests/t_disable_changed_csum_seed_mounted/expect b/tests/t_disable_changed_csum_seed_mounted/expect
new file mode 100644
index 0000000..288494b
--- /dev/null
+++ b/tests/t_disable_changed_csum_seed_mounted/expect
@@ -0,0 +1,19 @@
+create fs without csum_seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+turn on csum_seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+Checksum seed:            0x3ba62721
+change UUID
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
+Checksum seed:            0x3ba62721
+check filesystem
+fsck returns 0
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
+Checksum seed:            0x3ba62721
+turn off csum_seed
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
+Checksum seed:            0x3ba62721
+check filesystem
+fsck returns 0
+Filesystem UUID:          a61be2e0-b3b8-4fbc-b2cd-d84b306b9731
+Checksum seed:            0x3ba62721
diff --git a/tests/t_disable_changed_csum_seed_mounted/script b/tests/t_disable_changed_csum_seed_mounted/script
new file mode 100755
index 0000000..3be6dd9
--- /dev/null
+++ b/tests/t_disable_changed_csum_seed_mounted/script
@@ -0,0 +1,47 @@
+test_description="disable csum seed on mounted fs via tune2fs after changing uuid"
+
+trap "rm -rf $TMPFILE $TMPFILE.conf" EXIT INT QUIT
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+OUT=$test_name.log
+EXP=$test_dir/expect
+rm -rf $OUT
+
+# Test command line option
+echo "create fs without csum_seed" >> $OUT
+$MKE2FS -O metadata_csum,^metadata_csum_seed -U 6b33f586-a183-4383-921d-30da3fef2e5c -F $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "turn on csum_seed" >> $OUT
+$TUNE2FS -O metadata_csum_seed $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "change UUID" >> $OUT
+$TUNE2FS -U a61be2e0-b3b8-4fbc-b2cd-d84b306b9731 $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "turn off csum_seed" >> $OUT
+EXT2FS_PRETEND_RW_MOUNT=1 $TUNE2FS -O ^metadata_csum_seed $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+
diff --git a/tests/t_disable_csum_seed/expect b/tests/t_disable_csum_seed/expect
new file mode 100644
index 0000000..e6c50fe
--- /dev/null
+++ b/tests/t_disable_csum_seed/expect
@@ -0,0 +1,14 @@
+create fs without csum_seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+turn on csum_seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+Checksum seed:            0x3ba62721
+check filesystem
+fsck returns 0
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+Checksum seed:            0x3ba62721
+turn off csum_seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+check filesystem
+fsck returns 0
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
diff --git a/tests/t_disable_csum_seed/script b/tests/t_disable_csum_seed/script
new file mode 100755
index 0000000..28fdc5c
--- /dev/null
+++ b/tests/t_disable_csum_seed/script
@@ -0,0 +1,43 @@
+test_description="disable csum seed via tune2fs"
+
+trap "rm -rf $TMPFILE $TMPFILE.conf" EXIT INT QUIT
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+OUT=$test_name.log
+EXP=$test_dir/expect
+rm -rf $OUT
+
+# Test command line option
+echo "create fs without csum_seed" >> $OUT
+$MKE2FS -O metadata_csum,^metadata_csum_seed -U 6b33f586-a183-4383-921d-30da3fef2e5c -F $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "turn on csum_seed" >> $OUT
+$TUNE2FS -O metadata_csum_seed $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "turn off csum_seed" >> $OUT
+$TUNE2FS -O ^metadata_csum_seed $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+
diff --git a/tests/t_disable_meta_csum_and_seed/expect b/tests/t_disable_meta_csum_and_seed/expect
new file mode 100644
index 0000000..a1c9cca
--- /dev/null
+++ b/tests/t_disable_meta_csum_and_seed/expect
@@ -0,0 +1,14 @@
+create fs without csum_seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+turn on csum_seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+Checksum seed:            0x3ba62721
+check filesystem
+fsck returns 0
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+Checksum seed:            0x3ba62721
+turn off metadata_csum
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+check filesystem
+fsck returns 0
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
diff --git a/tests/t_disable_meta_csum_and_seed/script b/tests/t_disable_meta_csum_and_seed/script
new file mode 100755
index 0000000..9eb18e3
--- /dev/null
+++ b/tests/t_disable_meta_csum_and_seed/script
@@ -0,0 +1,44 @@
+test_description="disable csum seed and csums via tune2fs"
+
+trap "rm -rf $TMPFILE $TMPFILE.conf" EXIT INT QUIT
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+OUT=$test_name.log
+EXP=$test_dir/expect
+rm -rf $OUT
+
+# Test command line option
+echo "create fs without csum_seed" >> $OUT
+$MKE2FS -O metadata_csum,^metadata_csum_seed -U 6b33f586-a183-4383-921d-30da3fef2e5c -F $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "turn on csum_seed" >> $OUT
+$TUNE2FS -O metadata_csum_seed $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "turn off metadata_csum" >> $OUT
+$TUNE2FS -O ^metadata_csum $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | grep 'metadata_csum' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+
diff --git a/tests/t_enable_csum_seed/expect b/tests/t_enable_csum_seed/expect
new file mode 100644
index 0000000..07d9a4f
--- /dev/null
+++ b/tests/t_enable_csum_seed/expect
@@ -0,0 +1,12 @@
+create fs without csum_seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+turn on csum_seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+Checksum seed:            0x3ba62721
+change uuid
+Filesystem UUID:          1dd136c6-e47a-4833-9bf5-519f8aacabe4
+Checksum seed:            0x3ba62721
+check filesystem
+fsck returns 0
+Filesystem UUID:          1dd136c6-e47a-4833-9bf5-519f8aacabe4
+Checksum seed:            0x3ba62721
diff --git a/tests/t_enable_csum_seed/script b/tests/t_enable_csum_seed/script
new file mode 100755
index 0000000..55cade6
--- /dev/null
+++ b/tests/t_enable_csum_seed/script
@@ -0,0 +1,38 @@
+test_description="enable csum seed via tune2fs"
+
+trap "rm -rf $TMPFILE $TMPFILE.conf" EXIT INT QUIT
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+OUT=$test_name.log
+EXP=$test_dir/expect
+rm -rf $OUT
+
+# Test command line option
+echo "create fs without csum_seed" >> $OUT
+$MKE2FS -O metadata_csum,^metadata_csum_seed -U 6b33f586-a183-4383-921d-30da3fef2e5c -F $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "turn on csum_seed" >> $OUT
+$TUNE2FS -O metadata_csum_seed $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "change uuid" >> $OUT
+$TUNE2FS -U 1dd136c6-e47a-4833-9bf5-519f8aacabe4 $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+
diff --git a/tests/t_format_csum_seed/expect b/tests/t_format_csum_seed/expect
new file mode 100644
index 0000000..39e6d6a
--- /dev/null
+++ b/tests/t_format_csum_seed/expect
@@ -0,0 +1,10 @@
+create fs with csum_seed
+Filesystem UUID:          6b33f586-a183-4383-921d-30da3fef2e5c
+Checksum seed:            0x3ba62721
+change uuid
+Filesystem UUID:          1dd136c6-e47a-4833-9bf5-519f8aacabe4
+Checksum seed:            0x3ba62721
+check filesystem
+fsck returns 0
+Filesystem UUID:          1dd136c6-e47a-4833-9bf5-519f8aacabe4
+Checksum seed:            0x3ba62721
diff --git a/tests/t_format_csum_seed/script b/tests/t_format_csum_seed/script
new file mode 100755
index 0000000..1829514
--- /dev/null
+++ b/tests/t_format_csum_seed/script
@@ -0,0 +1,34 @@
+test_description="format with csum_seed"
+
+trap "rm -rf $TMPFILE $TMPFILE.conf" EXIT INT QUIT
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+OUT=$test_name.log
+EXP=$test_dir/expect
+rm -rf $OUT
+
+# Test command line option
+echo "create fs with csum_seed" >> $OUT
+$MKE2FS -O metadata_csum,metadata_csum_seed -U 6b33f586-a183-4383-921d-30da3fef2e5c -F $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "change uuid" >> $OUT
+$TUNE2FS -U 1dd136c6-e47a-4833-9bf5-519f8aacabe4 $TMPFILE > /dev/null 2>&1
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+echo "check filesystem" >> $OUT
+$FSCK $FSCK_OPT -fy $TMPFILE > /dev/null 2>&1
+echo "fsck returns $?" >> $OUT
+$DUMPE2FS $TMPFILE 2>&1 | egrep '(Checksum seed:|UUID)' >> $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+


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

* [PATCH 6/9] filefrag: accommodate holes when calculating expected values
  2016-02-13 22:37 [PATCH 0/9] e2fsprogs: save checksum seeds; fix broken xattr editing; misc fixes Darrick J. Wong
                   ` (4 preceding siblings ...)
  2016-02-13 22:37 ` [PATCH 5/9] tests: check proper operation of metadata_csum_seed Darrick J. Wong
@ 2016-02-13 22:38 ` Darrick J. Wong
  2016-03-06  2:41   ` Theodore Ts'o
  2016-02-13 22:38 ` [PATCH 7/9] tune2fs: confirm dangerous operations Darrick J. Wong
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 24+ messages in thread
From: Darrick J. Wong @ 2016-02-13 22:38 UTC (permalink / raw)
  To: tytso, darrick.wong; +Cc: linux-ext4

Currently, filefrag's "expected physical block" column expects extent
records to be physically adjacent regardless of the amount of logical
block space between the two records.  This means that if we punch a
hole in a file, we get reports like this:

ext:   logical_offset:   physical_offset: length:  expected: flags:
4:     4096..    8343:   57376..  61623:   4248:
5:     8345..   10313:   61625..  63593:   1969:   61624:

Notice how it expects 8345 to map to 61624, and scores this against
the fragmentation of the file.  Flagging this as "unexpected" is
incorrect because the gap in the logical mapping is exactly the same
size as the gap in the physical extents.

Furthermore, this particular mapping leaves the door open to the
optimal mapping -- if a write to block 8344 causes it to be mapped to
61624, the entire range 4096-10313 can be mapped with a single extent.
Until that happens, there's no way to combine extents 4 and 5 because
of the gap in the logical mapping at block 8344.

Therefore, tweak the extent report to account for holes.

v2: Make it work for extents crossing FIEMAP calls, and clean up the
FIBMAP version to report correct expected values.

v3: Don't count physically but not logically contiguous extents
in the fragmentation summary.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 misc/filefrag.c |   79 ++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 49 insertions(+), 30 deletions(-)


diff --git a/misc/filefrag.c b/misc/filefrag.c
index 5bcde91..d89d3c9 100644
--- a/misc/filefrag.c
+++ b/misc/filefrag.c
@@ -208,9 +208,11 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents,
 	__u64 buf[2048];	/* __u64 for proper field alignment */
 	struct fiemap *fiemap = (struct fiemap *)buf;
 	struct fiemap_extent *fm_ext = &fiemap->fm_extents[0];
+	struct fiemap_extent fm_last = {0};
 	int count = (sizeof(buf) - sizeof(*fiemap)) /
 			sizeof(struct fiemap_extent);
 	unsigned long long expected = 0;
+	unsigned long long expected_dense = 0;
 	unsigned long flags = 0;
 	unsigned int i;
 	int fiemap_header_printed = 0;
@@ -254,8 +256,13 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents,
 		}
 
 		for (i = 0; i < fiemap->fm_mapped_extents; i++) {
+			expected_dense = fm_last.fe_physical +
+					 fm_last.fe_length;
+			expected = fm_last.fe_physical +
+				   fm_ext[i].fe_logical - fm_last.fe_logical;
 			if (fm_ext[i].fe_logical != 0 &&
-			    fm_ext[i].fe_physical != expected) {
+			    fm_ext[i].fe_physical != expected &&
+			    fm_ext[i].fe_physical != expected_dense) {
 				tot_extents++;
 			} else {
 				expected = 0;
@@ -265,10 +272,9 @@ static int filefrag_fiemap(int fd, int blk_shift, int *num_extents,
 			if (verbose)
 				print_extent_info(&fm_ext[i], n, expected,
 						  blk_shift, st);
-
-			expected = fm_ext[i].fe_physical + fm_ext[i].fe_length;
 			if (fm_ext[i].fe_flags & FIEMAP_EXTENT_LAST)
 				last = 1;
+			fm_last = fm_ext[i];
 			n++;
 		}
 
@@ -287,14 +293,15 @@ static int filefrag_fibmap(int fd, int blk_shift, int *num_extents,
 			   ext2fs_struct_stat *st,
 			   unsigned long numblocks, int is_ext2)
 {
-	struct fiemap_extent	fm_ext;
+	struct fiemap_extent	fm_ext, fm_last;
 	unsigned long		i, last_block;
-	unsigned long long	logical;
+	unsigned long long	logical, expected = 0;
 				/* Blocks per indirect block */
 	const long		bpib = st->st_blksize / 4;
 	int			count;
 
 	memset(&fm_ext, 0, sizeof(fm_ext));
+	memset(&fm_last, 0, sizeof(fm_last));
 	if (force_extent) {
 		fm_ext.fe_flags = FIEMAP_EXTENT_MERGED;
 	}
@@ -322,40 +329,52 @@ static int filefrag_fibmap(int fd, int blk_shift, int *num_extents,
 			return rc;
 		if (block == 0)
 			continue;
-		if (*num_extents == 0) {
-			(*num_extents)++;
-			if (force_extent) {
+
+		if (*num_extents == 0 || block != last_block + 1 ||
+		    fm_ext.fe_logical + fm_ext.fe_length != logical) {
+			/*
+			 * This is the start of a new extent; figure out where
+			 * we expected it to be and report the extent.
+			 */
+			if (*num_extents != 0 && fm_last.fe_length) {
+				expected = fm_last.fe_physical +
+					(fm_ext.fe_logical - fm_last.fe_logical);
+				if (expected == fm_ext.fe_physical)
+					expected = 0;
+			}
+			if (force_extent && *num_extents == 0)
 				print_extent_header();
-				fm_ext.fe_physical = block * st->st_blksize;
+			if (force_extent && *num_extents != 0) {
+				print_extent_info(&fm_ext, *num_extents - 1,
+						  expected, blk_shift, st);
 			}
-		}
-		count++;
-		if (force_extent && last_block != 0 &&
-		    (block != last_block + 1 ||
-		     fm_ext.fe_logical + fm_ext.fe_length != logical)) {
-			print_extent_info(&fm_ext, *num_extents - 1,
-					  (last_block + 1) * st->st_blksize,
-					  blk_shift, st);
-			fm_ext.fe_length = 0;
+			if (verbose && expected != 0) {
+				printf("Discontinuity: Block %llu is at %llu "
+				       "(was %llu)\n",
+					fm_ext.fe_logical / st->st_blksize,
+					fm_ext.fe_physical / st->st_blksize,
+					expected / st->st_blksize);
+			}
+			/* create the new extent */
+			fm_last = fm_ext;
 			(*num_extents)++;
-			fm_ext.fe_logical = logical;
 			fm_ext.fe_physical = block * st->st_blksize;
-		} else if (last_block && (block != last_block + 1)) {
-			if (verbose)
-				printf("Discontinuity: Block %ld is at %lu (was "
-				       "%lu)\n", i, block, last_block + 1);
-			fm_ext.fe_length = 0;
-			(*num_extents)++;
 			fm_ext.fe_logical = logical;
-			fm_ext.fe_physical = block * st->st_blksize;
+			fm_ext.fe_length = 0;
 		}
 		fm_ext.fe_length += st->st_blksize;
 		last_block = block;
 	}

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

* [PATCH 7/9] tune2fs: confirm dangerous operations
  2016-02-13 22:37 [PATCH 0/9] e2fsprogs: save checksum seeds; fix broken xattr editing; misc fixes Darrick J. Wong
                   ` (5 preceding siblings ...)
  2016-02-13 22:38 ` [PATCH 6/9] filefrag: accommodate holes when calculating expected values Darrick J. Wong
@ 2016-02-13 22:38 ` Darrick J. Wong
  2016-02-15  0:49   ` Andreas Dilger
  2016-02-13 22:38 ` [PATCH 8/9] tune2fs: recover the journal Darrick J. Wong
  2016-02-13 22:38 ` [PATCH 9/9] libext2fs: sort keys for xattr blocks Darrick J. Wong
  8 siblings, 1 reply; 24+ messages in thread
From: Darrick J. Wong @ 2016-02-13 22:38 UTC (permalink / raw)
  To: tytso, darrick.wong; +Cc: Andreas Dilger, linux-ext4

Give admins a short amount of time to confirm that they want to
proceed with a dangerous operation.  Refuse to perform the op
unless the filesystem is freshly checked.

Cc: Andreas Dilger <adilger@dilger.ca>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 misc/tune2fs.c           |   40 +++++++++-----
 tests/t_dangerous/expect |   97 +++++++++++++++++++++++++++++++++
 tests/t_dangerous/name   |    1 
 tests/t_dangerous/script |  134 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 259 insertions(+), 13 deletions(-)
 create mode 100644 tests/t_dangerous/expect
 create mode 100644 tests/t_dangerous/name
 create mode 100644 tests/t_dangerous/script


diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index f9e654f..457e9e5 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -405,14 +405,24 @@ static int update_mntopts(ext2_filsys fs, char *mntopts)
 	return 0;
 }
 
-static int check_fsck_needed(ext2_filsys fs)
+static void check_fsck_needed(ext2_filsys fs, const char *prompt)
 {
-	if (fs->super->s_state & EXT2_VALID_FS)
-		return 0;
-	printf("\n%s\n", _(please_fsck));
-	if (mount_flags & EXT2_MF_READONLY)
-		printf("%s", _("(and reboot afterwards!)\n"));
-	return 1;
+	/* Refuse to modify anything but a freshly checked valid filesystem. */
+	if (!(fs->super->s_state & EXT2_VALID_FS) ||
+	    (fs->super->s_state & EXT2_ERROR_FS) ||
+	    (fs->super->s_lastcheck < fs->super->s_mtime)) {
+		printf("\n%s\n", _(please_fsck));
+		if (mount_flags & EXT2_MF_READONLY)
+			printf("%s", _("(and reboot afterwards!)\n"));
+		exit(1);
+	}
+
+	/* Give the admin a few seconds to bail out of a dangerous op. */
+	if (!getenv("TUNE2FS_FORCE_PROMPT") && (!isatty(0) || !isatty(1)))
+		return;
+
+	puts(prompt);
+	proceed_question(5);
 }
 
 static void request_dir_fsck_afterwards(ext2_filsys fs)
@@ -1145,8 +1155,8 @@ mmp_error:
 
 	if (FEATURE_ON(E2P_FEATURE_RO_INCOMPAT,
 		       EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
-		if (check_fsck_needed(fs))
-			exit(1);
+		check_fsck_needed(fs,
+			_("Enabling checksums could take some time."));
 		if (mount_flags & EXT2_MF_MOUNTED) {
 			fputs(_("Cannot enable metadata_csum on a mounted "
 				"filesystem!\n"), stderr);
@@ -1186,8 +1196,8 @@ mmp_error:
 			EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
 		__u32	test_features[3];
 
-		if (check_fsck_needed(fs))
-			exit(1);
+		check_fsck_needed(fs,
+			_("Disabling checksums could take some time."));
 		if (mount_flags & EXT2_MF_MOUNTED) {
 			fputs(_("Cannot disable metadata_csum on a mounted "
 				"filesystem!\n"), stderr);
@@ -2784,6 +2794,8 @@ retry_open:
 			rc = 1;
 			goto closefs;
 		}
+		check_fsck_needed(fs,
+			_("Resizing inodes could take some time."));
 		/*
 		 * If inode resize is requested use the
 		 * Undo I/O manager
@@ -2999,8 +3011,10 @@ retry_open:
 				try_confirm_csum_seed_support();
 				exit(1);
 			}
-			if (check_fsck_needed(fs))
-				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
diff --git a/tests/t_dangerous/expect b/tests/t_dangerous/expect
new file mode 100644
index 0000000..353bd57
--- /dev/null
+++ b/tests/t_dangerous/expect
@@ -0,0 +1,97 @@
+tune2fs dangerous prompts test
+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
+
+tune2fs -O metadata_csum test.img
+
+Please run e2fsck on the filesystem.
+
+Exit status is 1
+tune2fs -O metadata_csum test.img
+Exit status is 0
+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 -O metadata_csum test.img
+Enabling checksums could take some time.
+Proceed anyway (or wait 5 seconds) ? (y,n) 
+Exit status is 1
+tune2fs -I 512 test.img
+Resizing inodes could take some time.
+Proceed anyway (or wait 5 seconds) ? (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) ? (y,n) 
+Exit status is 1
+
+Change in FS metadata:
+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 -O metadata_csum test.img
+Enabling checksums could take some time.
+Proceed anyway (or wait 5 seconds) ? (y,n) 
+Please run e2fsck -D on the filesystem.
+
+Exit status is 0
+test_filesys was not cleanly unmounted, check forced.
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 3A: Optimizing directories
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+
+
+
+tune2fs -I 512 test.img
+Resizing inodes could take some time.
+Proceed anyway (or wait 5 seconds) ? (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.
+Proceed anyway (or wait 5 seconds) ? (y,n) Exit status is 0
+Backing up journal inode block information.
+
+
+Change in FS metadata:
+@@ -1,3 +1,3 @@
+-Filesystem UUID:          6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf
+-Filesystem features:      has_journal ext_attr resize_inode dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
+-Inode size:	          256
++Filesystem UUID:          f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0
++Filesystem features:      has_journal ext_attr resize_inode dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
++Inode size:	          512
+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
diff --git a/tests/t_dangerous/name b/tests/t_dangerous/name
new file mode 100644
index 0000000..c9a1030
--- /dev/null
+++ b/tests/t_dangerous/name
@@ -0,0 +1 @@
+dangerous tune2fs operation prompts
diff --git a/tests/t_dangerous/script b/tests/t_dangerous/script
new file mode 100644
index 0000000..dc70ad2
--- /dev/null
+++ b/tests/t_dangerous/script
@@ -0,0 +1,134 @@
+FSCK_OPT=-fn
+OUT=$test_name.log
+EXP=$test_dir/expect
+CONF=$TMPFILE.conf
+
+cat > $CONF << ENDL
+[fs_types]
+	ext4h = {
+		features = has_journal,extent,huge_file,^flex_bg,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,resize_inode,64bit
+		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
+
+MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
+
+# trigger a filesystem check
+$DEBUGFS -w -R 'ssv mtime now' $TMPFILE > /dev/null 2>&1
+$DEBUGFS -w -R 'ssv lastcheck 20000' $TMPFILE > /dev/null 2>&1
+
+# add mcsum
+echo "tune2fs -O metadata_csum test.img" >> $OUT
+$TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# check fs
+$FSCK -f -y -N test_filesys $TMPFILE > /dev/null 2>&1
+
+# add mcsum
+echo "tune2fs -O metadata_csum test.img" >> $OUT
+$TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
+
+# dump and check
+$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
+
+# add mcsum
+echo "tune2fs -O metadata_csum test.img" >> $OUT
+echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# change inode size
+echo "tune2fs -I 512 test.img" >> $OUT
+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
+
+# dump and check
+$DUMPE2FS $TMPFILE 2> /dev/null | grep '^Group 0:' -B99 -A20 | sed -f $cmd_dir/filter.sed > $OUT.after
+echo "Change in FS metadata:" >> $OUT
+diff -u $OUT.before $OUT.after | tail -n +3 >> $OUT
+$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+$DUMPE2FS $TMPFILE 2> /dev/null | egrep '^(Filesystem features:|Filesystem UUID:|Inode size:)' > $OUT.before
+
+# add mcsum
+echo "tune2fs -O metadata_csum test.img" >> $OUT
+echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
+
+# change inode size
+echo "tune2fs -I 512 test.img" >> $OUT
+echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -I 512 $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# change uuid
+echo "tune2fs -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 test.img" >> $OUT
+echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# check
+$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
+
+# dump and check
+$DUMPE2FS $TMPFILE 2> /dev/null | egrep '^(Filesystem features:|Filesystem UUID:|Inode size:)' > $OUT.after
+echo "Change in FS metadata:" >> $OUT
+diff -u $OUT.before $OUT.after | tail -n +3 >> $OUT
+$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+rm $TMPFILE $OUT.before $OUT.after $CONF
+
+#
+# Do the verification
+#
+
+sed -f $cmd_dir/filter.sed -e "s;$TMPFILE;test.img;" -e 's/test_filesys:.*//g' < $OUT > $OUT.new
+mv $OUT.new $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+fi
+
+unset IMAGE FSCK_OPT OUT EXP CONF


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

* [PATCH 8/9] tune2fs: recover the journal
  2016-02-13 22:37 [PATCH 0/9] e2fsprogs: save checksum seeds; fix broken xattr editing; misc fixes Darrick J. Wong
                   ` (6 preceding siblings ...)
  2016-02-13 22:38 ` [PATCH 7/9] tune2fs: confirm dangerous operations Darrick J. Wong
@ 2016-02-13 22:38 ` Darrick J. Wong
  2016-03-06  5:23   ` Theodore Ts'o
  2016-02-13 22:38 ` [PATCH 9/9] libext2fs: sort keys for xattr blocks Darrick J. Wong
  8 siblings, 1 reply; 24+ messages in thread
From: Darrick J. Wong @ 2016-02-13 22:38 UTC (permalink / raw)
  To: tytso, darrick.wong; +Cc: linux-ext4

If the journal needs to be recovered to avoid clobbering whatever
changes tune2fs makes, do so.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 misc/Makefile.in              |    2 +
 misc/tune2fs.c                |   22 ++++++++++-----
 tests/t_replay_and_set/expect |   36 +++++++++++++++++++++++++
 tests/t_replay_and_set/name   |    1 +
 tests/t_replay_and_set/script |   60 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 112 insertions(+), 9 deletions(-)
 create mode 100644 tests/t_replay_and_set/expect
 create mode 100644 tests/t_replay_and_set/name
 create mode 100644 tests/t_replay_and_set/script


diff --git a/misc/Makefile.in b/misc/Makefile.in
index 52cd5bb..98872df 100644
--- a/misc/Makefile.in
+++ b/misc/Makefile.in
@@ -47,7 +47,7 @@ UMANPAGES=	chattr.1 lsattr.1 @UUID_CMT@ uuidgen.1
 
 LPROGS=		@E2INITRD_PROG@
 
-TUNE2FS_OBJS=	tune2fs.o util.o
+TUNE2FS_OBJS=	tune2fs.o util.o journal.o recovery.o revoke.o
 MKLPF_OBJS=	mklost+found.o
 MKE2FS_OBJS=	mke2fs.o util.o default_profile.o mk_hugefiles.o \
 			create_inode.o
diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 457e9e5..657349a 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -113,6 +113,7 @@ struct blk_move {
 	blk64_t new_loc;
 };
 
+errcode_t ext2fs_run_ext3_journal(ext2_filsys *fs);
 
 static const char *please_fsck = N_("Please run e2fsck on the filesystem.\n");
 static const char *please_dir_fsck =
@@ -3139,15 +3140,20 @@ retry_open:
 		free(ext_mount_opts);
 	}
 
-	/* Warn if file system needs recovery and it is opened for writing. */
+	/* Recover the journal if possible. */
 	if ((open_flag & EXT2_FLAG_RW) && !(mount_flags & EXT2_MF_MOUNTED) &&
-	    (sb->s_feature_compat & EXT3_FEATURE_COMPAT_HAS_JOURNAL) &&
-	    (sb->s_feature_incompat & EXT3_FEATURE_INCOMPAT_RECOVER)) {
-		fprintf(stderr,
-_("Warning: The journal is dirty. You may wish to replay the journal like:\n\n"
-  "\te2fsck -E journal_only %s\n\n"
-  "then rerun this command.  Otherwise, any changes made may be overwritten\n"
-  "by journal recovery.\n"), device_name);
+	    ext2fs_has_feature_journal_needs_recovery(fs->super)) {
+		errcode_t err;
+
+		printf(_("Recovering journal.\n"));
+		err = ext2fs_run_ext3_journal(&fs);
+		if (err) {
+			com_err("tune2fs", err, "while recovering journal.\n");
+			printf(_("Please run e2fsck -fy %s.\n"), argv[1]);
+			goto closefs;
+		}
+		ext2fs_clear_feature_journal_needs_recovery(fs->super);
+		ext2fs_mark_super_dirty(fs);
 	}
 
 	free(device_name);
diff --git a/tests/t_replay_and_set/expect b/tests/t_replay_and_set/expect
new file mode 100644
index 0000000..f491937
--- /dev/null
+++ b/tests/t_replay_and_set/expect
@@ -0,0 +1,36 @@
+64-bit filesystem support is not enabled.  The larger fields afforded by this feature enable full-strength checksumming.  Pass -O 64bit to rectify.
+Creating filesystem with 65536 4k blocks and 16384 inodes
+Superblock backups stored on blocks: 
+	32768
+
+Allocating group tables:    \b\b\bdone                            
+Writing inode tables:    \b\b\bdone                            
+Creating journal (4096 blocks): done
+Writing superblocks and filesystem accounting information:    \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
+test_filesys: 11/16384 files (0.0% non-contiguous), 5164/65536 blocks
+Exit status is 0
+debugfs write journal
+disable metadata_csum on a dirty-journal fs
+Recovering journal.
+fsck the whole mess
+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
+Block bitmap differences:  +(0--1050) +(32768--36880)
+Fix? yes
+
+Inode bitmap differences:  +(1--11)
+Fix? yes
+
+
+test_filesys: ***** FILE SYSTEM WAS MODIFIED *****
+test_filesys: 11/16384 files (0.0% non-contiguous), 5164/65536 blocks
+Exit status is 1
diff --git a/tests/t_replay_and_set/name b/tests/t_replay_and_set/name
new file mode 100644
index 0000000..4e4759e
--- /dev/null
+++ b/tests/t_replay_and_set/name
@@ -0,0 +1 @@
+recover journal and clear features
diff --git a/tests/t_replay_and_set/script b/tests/t_replay_and_set/script
new file mode 100644
index 0000000..1253099
--- /dev/null
+++ b/tests/t_replay_and_set/script
@@ -0,0 +1,60 @@
+if test -x $DEBUGFS_EXE; then
+
+FSCK_OPT=-fy
+OUT=$test_name.log
+if [ -f $test_dir/expect.gz ]; then
+	EXP=$test_name.tmp
+	gunzip < $test_dir/expect.gz > $EXP1
+else
+	EXP=$test_dir/expect
+fi
+
+cp /dev/null $OUT
+
+$MKE2FS -F -o Linux -b 4096 -O has_journal,metadata_csum -T ext4 $TMPFILE 65536 2>&1 | sed -f $cmd_dir/filter.sed >> $OUT 2>&1
+
+$FSCK -fy -N test_filesys $TMPFILE > $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -f $cmd_dir/filter.sed -e "s;$TMPFILE;test.img;" $OUT.new >> $OUT
+rm -f $OUT.new
+
+bitmaps="$($DUMPE2FS $TMPFILE 2>&1 | grep 'bitmap at' | sed -e 's/^.*bitmap at \([0-9]*\).*$/\1/g' | tr '\n' ',')"
+
+echo "debugfs write journal" >> $OUT
+echo "jo" > $TMPFILE.cmd
+echo "jw -b $bitmaps /dev/zero" >> $TMPFILE.cmd
+echo "jc" >> $TMPFILE.cmd
+$DEBUGFS_EXE -w -f $TMPFILE.cmd $TMPFILE 2>> $OUT.new > /dev/null
+sed -f $cmd_dir/filter.sed < $OUT.new >> $OUT
+rm -rf $OUT.new
+
+echo "disable metadata_csum on a dirty-journal fs" >> $OUT
+$TUNE2FS -O ^metadata_csum $TMPFILE 2>&1 | sed -f $cmd_dir/filter.sed >> $OUT 2>&1
+
+echo "fsck the whole mess" >> $OUT
+$FSCK -fy -N test_filesys $TMPFILE > $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -f $cmd_dir/filter.sed -e "s;$TMPFILE;test.img;" $OUT.new >> $OUT
+rm -f $OUT.new
+
+rm -f $TMPFILE
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+	rm -f $test_name.tmp
+fi
+
+unset IMAGE FSCK_OPT OUT EXP
+
+else #if test -x $DEBUGFS_EXE; then
+	echo "$test_name: $test_description: skipped"
+fi


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

* [PATCH 9/9] libext2fs: sort keys for xattr blocks
  2016-02-13 22:37 [PATCH 0/9] e2fsprogs: save checksum seeds; fix broken xattr editing; misc fixes Darrick J. Wong
                   ` (7 preceding siblings ...)
  2016-02-13 22:38 ` [PATCH 8/9] tune2fs: recover the journal Darrick J. Wong
@ 2016-02-13 22:38 ` Darrick J. Wong
  2016-02-14 10:37   ` Richard Purdie
  2016-03-06  3:55   ` Theodore Ts'o
  8 siblings, 2 replies; 24+ messages in thread
From: Darrick J. Wong @ 2016-02-13 22:38 UTC (permalink / raw)
  To: tytso, darrick.wong; +Cc: linux-ext4, Darren Hart, Richard Purdie

Richard Purdie reports that libext2fs doesn't sort attribute keys in
the xattr block correctly, causing the kernel to return -ENODATA when
querying attributes that should be there.  Therefore, sort attributes
so that whatever ends up in the xattr block is sorted according to
what the kernel expects.

Cc: Darren Hart <dvhart@linux.intel.com>
Reported-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 lib/ext2fs/ext_attr.c        |   24 +++++++++++-
 tests/d_xattr_sorting/expect |   29 ++++++++++++++
 tests/d_xattr_sorting/name   |    1 
 tests/d_xattr_sorting/script |   86 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 139 insertions(+), 1 deletion(-)
 create mode 100644 tests/d_xattr_sorting/expect
 create mode 100644 tests/d_xattr_sorting/name
 create mode 100644 tests/d_xattr_sorting/script


diff --git a/lib/ext2fs/ext_attr.c b/lib/ext2fs/ext_attr.c
index 0a4f8c0..b121837 100644
--- a/lib/ext2fs/ext_attr.c
+++ b/lib/ext2fs/ext_attr.c
@@ -254,10 +254,15 @@ static struct ea_name_index ea_names[] = {
 	{0, NULL},
 };
 
+static int find_ea_index(char *fullname, char **name, int *index);
+
 /* Push empty attributes to the end and inlinedata to the front. */
 static int attr_compare(const void *a, const void *b)
 {
 	const struct ext2_xattr *xa = a, *xb = b;
+	char *xa_suffix, *xb_suffix;
+	int xa_idx, xb_idx;
+	int cmp;
 
 	if (xa->name == NULL)
 		return +1;
@@ -267,7 +272,24 @@ static int attr_compare(const void *a, const void *b)
 		return -1;
 	else if (!strcmp(xb->name, "system.data"))
 		return +1;
-	return 0;
+
+	/*
+	 * Duplicate the kernel's sorting algorithm because xattr blocks
+	 * require sorted keys.
+	 */
+	xa_suffix = xa->name;
+	xb_suffix = xb->name;
+	xa_idx = xb_idx = 0;
+	find_ea_index(xa->name, &xa_suffix, &xa_idx);
+	find_ea_index(xb->name, &xb_suffix, &xb_idx);
+	cmp = xa_idx - xb_idx;
+	if (cmp)
+		return cmp;
+	cmp = strlen(xa_suffix) - strlen(xb_suffix);
+	if (cmp)
+		return cmp;
+	cmp = strcmp(xa_suffix, xb_suffix);
+	return cmp;
 }
 
 static const char *find_ea_prefix(int index)
diff --git a/tests/d_xattr_sorting/expect b/tests/d_xattr_sorting/expect
new file mode 100644
index 0000000..17da663
--- /dev/null
+++ b/tests/d_xattr_sorting/expect
@@ -0,0 +1,29 @@
+debugfs sort extended attributes
+mke2fs -Fq -b 1024 test.img 512
+Exit status is 0
+ea_set / security.SMEG64 -f /tmp/b
+Exit status is 0
+ea_set / security.imb -f /tmp/b
+Exit status is 0
+ea_set / user.moo cow
+Exit status is 0
+ea_list /
+Extended attributes:
+  user.moo = "cow" (3)
+  security.imb = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" (256)
+  security.SMEG64 = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" (256)
+Exit status is 0
+ea_get / security.imb
+xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+Exit status is 0
+ea_get / nosuchea
+ea_get: Extended attribute key not found while getting extended attribute
+Exit status is 0
+e2fsck -yf -N test_filesys
+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
+test_filesys: 11/64 files (0.0% non-contiguous), 29/512 blocks
+Exit status is 0
diff --git a/tests/d_xattr_sorting/name b/tests/d_xattr_sorting/name
new file mode 100644
index 0000000..dde8926
--- /dev/null
+++ b/tests/d_xattr_sorting/name
@@ -0,0 +1 @@
+sort extended attributes in debugfs
diff --git a/tests/d_xattr_sorting/script b/tests/d_xattr_sorting/script
new file mode 100644
index 0000000..30c189a
--- /dev/null
+++ b/tests/d_xattr_sorting/script
@@ -0,0 +1,86 @@
+if test -x $DEBUGFS_EXE; then
+
+OUT=$test_name.log
+EXP=$test_dir/expect
+VERIFY_FSCK_OPT=-yf
+
+TEST_DATA=$test_name.tmp
+VERIFY_DATA=$test_name.ver.tmp
+
+echo "debugfs sort extended attributes" > $OUT
+
+dd if=/dev/zero of=$TMPFILE bs=1k count=512 > /dev/null 2>&1
+
+echo "mke2fs -Fq -b 1024 test.img 512" >> $OUT
+
+$MKE2FS -Fq $TMPFILE 512 > /dev/null 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+perl -e 'print "x" x 256;' > /tmp/b
+
+echo "ea_set / security.SMEG64 -f /tmp/b" > $OUT.new
+$DEBUGFS -w -R "ea_set / security.SMEG64 -f /tmp/b" $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
+
+echo "ea_set / security.imb -f /tmp/b" > $OUT.new
+$DEBUGFS -w -R "ea_set / security.imb -f /tmp/b" $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
+
+echo "ea_set / user.moo cow" > $OUT.new
+$DEBUGFS -w -R "ea_set / user.moo cow" $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
+
+rm -rf /tmp/b
+
+echo "ea_list /" > $OUT.new
+$DEBUGFS -w -R "ea_list /" $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
+
+echo "ea_get / security.imb" > $OUT.new
+$DEBUGFS -w -R "ea_get / security.imb" $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
+
+echo "ea_get / nosuchea" > $OUT.new
+$DEBUGFS -w -R "ea_get / nosuchea" $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
+
+echo e2fsck $VERIFY_FSCK_OPT -N test_filesys > $OUT.new
+$FSCK $VERIFY_FSCK_OPT -N test_filesys $TMPFILE >> $OUT.new 2>&1
+status=$?
+echo Exit status is $status >> $OUT.new
+sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
+
+#
+# Do the verification
+#
+
+rm -f $TMPFILE $OUT.new
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+fi
+
+unset VERIFY_FSCK_OPT NATIVE_FSCK_OPT OUT EXP TEST_DATA VERIFY_DATA
+
+else #if test -x $DEBUGFS_EXE; then
+	echo "$test_name: $test_description: skipped"
+fi


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

* Re: [PATCH 9/9] libext2fs: sort keys for xattr blocks
  2016-02-13 22:38 ` [PATCH 9/9] libext2fs: sort keys for xattr blocks Darrick J. Wong
@ 2016-02-14 10:37   ` Richard Purdie
  2016-03-06  3:55   ` Theodore Ts'o
  1 sibling, 0 replies; 24+ messages in thread
From: Richard Purdie @ 2016-02-14 10:37 UTC (permalink / raw)
  To: Darrick J. Wong, tytso; +Cc: linux-ext4, Darren Hart

On Sat, 2016-02-13 at 14:38 -0800, Darrick J. Wong wrote:
> Richard Purdie reports that libext2fs doesn't sort attribute keys in
> the xattr block correctly, causing the kernel to return -ENODATA when
> querying attributes that should be there.  Therefore, sort attributes
> so that whatever ends up in the xattr block is sorted according to
> what the kernel expects.
> 
> Cc: Darren Hart <dvhart@linux.intel.com>
> Reported-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  lib/ext2fs/ext_attr.c        |   24 +++++++++++-
>  tests/d_xattr_sorting/expect |   29 ++++++++++++++
>  tests/d_xattr_sorting/name   |    1 
>  tests/d_xattr_sorting/script |   86
> ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 139 insertions(+), 1 deletion(-)
>  create mode 100644 tests/d_xattr_sorting/expect
>  create mode 100644 tests/d_xattr_sorting/name
>  create mode 100644 tests/d_xattr_sorting/script

I had some test scripts I'd used to debug this. I applied this patch
and those tests passed, thanks!

Tested-by: Richard Purdie <richard.purdie@linuxfoundation.org>

I'll update the version of this we're using in OpenEmbedded and the
Yocto Project to your patch.

Cheers,

Richard


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

* Re: [PATCH 7/9] tune2fs: confirm dangerous operations
  2016-02-13 22:38 ` [PATCH 7/9] tune2fs: confirm dangerous operations Darrick J. Wong
@ 2016-02-15  0:49   ` Andreas Dilger
  2016-02-15 16:53     ` Darrick J. Wong
  0 siblings, 1 reply; 24+ messages in thread
From: Andreas Dilger @ 2016-02-15  0:49 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: tytso, linux-ext4

On Feb 13, 2016, at 3:38 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> Give admins a short amount of time to confirm that they want to
> proceed with a dangerous operation.  Refuse to perform the op
> unless the filesystem is freshly checked.
> 
> Cc: Andreas Dilger <adilger@dilger.ca>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> misc/tune2fs.c           |   40 +++++++++-----
> tests/t_dangerous/expect |   97 +++++++++++++++++++++++++++++++++
> tests/t_dangerous/name   |    1 
> tests/t_dangerous/script |  134 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 259 insertions(+), 13 deletions(-)
> create mode 100644 tests/t_dangerous/expect
> create mode 100644 tests/t_dangerous/name
> create mode 100644 tests/t_dangerous/script
> 
> 
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index f9e654f..457e9e5 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -405,14 +405,24 @@ static int update_mntopts(ext2_filsys fs, char *mntopts)
> 	return 0;
> }
> 
> -static int check_fsck_needed(ext2_filsys fs)
> +static void check_fsck_needed(ext2_filsys fs, const char *prompt)
> {
> -	if (fs->super->s_state & EXT2_VALID_FS)
> -		return 0;
> -	printf("\n%s\n", _(please_fsck));
> -	if (mount_flags & EXT2_MF_READONLY)
> -		printf("%s", _("(and reboot afterwards!)\n"));
> -	return 1;
> +	/* Refuse to modify anything but a freshly checked valid filesystem. */
> +	if (!(fs->super->s_state & EXT2_VALID_FS) ||
> +	    (fs->super->s_state & EXT2_ERROR_FS) ||
> +	    (fs->super->s_lastcheck < fs->super->s_mtime)) {
> +		printf("\n%s\n", _(please_fsck));
> +		if (mount_flags & EXT2_MF_READONLY)
> +			printf("%s", _("(and reboot afterwards!)\n"));
> +		exit(1);
> +	}
> +
> +	/* Give the admin a few seconds to bail out of a dangerous op. */
> +	if (!getenv("TUNE2FS_FORCE_PROMPT") && (!isatty(0) || !isatty(1)))

One option would be to allow TUNE2FS_FORCE_PROMPT to contain the delay interval
instead of hard-coding this to 5s below?  IMHO, 5s is only long enough for
"oh sh**" and not enough to actually interrupt it unless you are watching the
output closely...

> +		return;
> +
> +	puts(prompt);
> +	proceed_question(5);
> }
> 
> static void request_dir_fsck_afterwards(ext2_filsys fs)
> @@ -1145,8 +1155,8 @@ mmp_error:
> 
> 	if (FEATURE_ON(E2P_FEATURE_RO_INCOMPAT,
> 		       EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
> -		if (check_fsck_needed(fs))
> -			exit(1);
> +		check_fsck_needed(fs,
> +			_("Enabling checksums could take some time."));
> 		if (mount_flags & EXT2_MF_MOUNTED) {
> 			fputs(_("Cannot enable metadata_csum on a mounted "
> 				"filesystem!\n"), stderr);
> @@ -1186,8 +1196,8 @@ mmp_error:
> 			EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
> 		__u32	test_features[3];
> 
> -		if (check_fsck_needed(fs))
> -			exit(1);
> +		check_fsck_needed(fs,
> +			_("Disabling checksums could take some time."));

Does disabling checksums actually take more time than just turning off the
feature in the superblock?  Without the feature flag all of the checksum
fields should be ignored (whether they contain 0 or stale checksums), and
they would be rewritten if/when the checksum feature is re-enabled.

> 		if (mount_flags & EXT2_MF_MOUNTED) {
> 			fputs(_("Cannot disable metadata_csum on a mounted "
> 				"filesystem!\n"), stderr);
> @@ -2784,6 +2794,8 @@ retry_open:
> 			rc = 1;
> 			goto closefs;
> 		}
> +		check_fsck_needed(fs,
> +			_("Resizing inodes could take some time."));
> 		/*
> 		 * If inode resize is requested use the
> 		 * Undo I/O manager
> @@ -2999,8 +3011,10 @@ retry_open:
> 				try_confirm_csum_seed_support();
> 				exit(1);
> 			}
> -			if (check_fsck_needed(fs))
> -				exit(1);
> +			if (!ext2fs_has_feature_csum_seed(fs->super))
> +				check_fsck_needed(fs,
> +					_("Setting UUID on a checksummed "
> +					  "filesystem could take some time."));

Wouldn't it make sense to enable the checksum seed feature in this case
instead of rewriting the checksums for the whole filesystem?

Cheers, Andreas

> 			/*
> 			 * Determine if the block group checksums are
> diff --git a/tests/t_dangerous/expect b/tests/t_dangerous/expect
> new file mode 100644
> index 0000000..353bd57
> --- /dev/null
> +++ b/tests/t_dangerous/expect
> @@ -0,0 +1,97 @@
> +tune2fs dangerous prompts test
> +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
> +
> +tune2fs -O metadata_csum test.img
> +
> +Please run e2fsck on the filesystem.
> +
> +Exit status is 1
> +tune2fs -O metadata_csum test.img
> +Exit status is 0
> +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 -O metadata_csum test.img
> +Enabling checksums could take some time.
> +Proceed anyway (or wait 5 seconds) ? (y,n) 
> +Exit status is 1
> +tune2fs -I 512 test.img
> +Resizing inodes could take some time.
> +Proceed anyway (or wait 5 seconds) ? (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) ? (y,n) 
> +Exit status is 1
> +
> +Change in FS metadata:
> +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 -O metadata_csum test.img
> +Enabling checksums could take some time.
> +Proceed anyway (or wait 5 seconds) ? (y,n) 
> +Please run e2fsck -D on the filesystem.
> +
> +Exit status is 0
> +test_filesys was not cleanly unmounted, check forced.
> +Pass 1: Checking inodes, blocks, and sizes
> +Pass 2: Checking directory structure
> +Pass 3: Checking directory connectivity
> +Pass 3A: Optimizing directories
> +Pass 4: Checking reference counts
> +Pass 5: Checking group summary information
> +
> +
> +
> +tune2fs -I 512 test.img
> +Resizing inodes could take some time.
> +Proceed anyway (or wait 5 seconds) ? (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.
> +Proceed anyway (or wait 5 seconds) ? (y,n) Exit status is 0
> +Backing up journal inode block information.
> +
> +
> +Change in FS metadata:
> +@@ -1,3 +1,3 @@
> +-Filesystem UUID:          6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf
> +-Filesystem features:      has_journal ext_attr resize_inode dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
> +-Inode size:	          256
> ++Filesystem UUID:          f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0
> ++Filesystem features:      has_journal ext_attr resize_inode dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> ++Inode size:	          512
> +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
> diff --git a/tests/t_dangerous/name b/tests/t_dangerous/name
> new file mode 100644
> index 0000000..c9a1030
> --- /dev/null
> +++ b/tests/t_dangerous/name
> @@ -0,0 +1 @@
> +dangerous tune2fs operation prompts
> diff --git a/tests/t_dangerous/script b/tests/t_dangerous/script
> new file mode 100644
> index 0000000..dc70ad2
> --- /dev/null
> +++ b/tests/t_dangerous/script
> @@ -0,0 +1,134 @@
> +FSCK_OPT=-fn
> +OUT=$test_name.log
> +EXP=$test_dir/expect
> +CONF=$TMPFILE.conf
> +
> +cat > $CONF << ENDL
> +[fs_types]
> +	ext4h = {
> +		features = has_journal,extent,huge_file,^flex_bg,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,resize_inode,64bit
> +		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
> +
> +MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
> +
> +# trigger a filesystem check
> +$DEBUGFS -w -R 'ssv mtime now' $TMPFILE > /dev/null 2>&1
> +$DEBUGFS -w -R 'ssv lastcheck 20000' $TMPFILE > /dev/null 2>&1
> +
> +# add mcsum
> +echo "tune2fs -O metadata_csum test.img" >> $OUT
> +$TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# check fs
> +$FSCK -f -y -N test_filesys $TMPFILE > /dev/null 2>&1
> +
> +# add mcsum
> +echo "tune2fs -O metadata_csum test.img" >> $OUT
> +$TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
> +
> +# dump and check
> +$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
> +
> +# add mcsum
> +echo "tune2fs -O metadata_csum test.img" >> $OUT
> +echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# change inode size
> +echo "tune2fs -I 512 test.img" >> $OUT
> +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
> +
> +# dump and check
> +$DUMPE2FS $TMPFILE 2> /dev/null | grep '^Group 0:' -B99 -A20 | sed -f $cmd_dir/filter.sed > $OUT.after
> +echo "Change in FS metadata:" >> $OUT
> +diff -u $OUT.before $OUT.after | tail -n +3 >> $OUT
> +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +$DUMPE2FS $TMPFILE 2> /dev/null | egrep '^(Filesystem features:|Filesystem UUID:|Inode size:)' > $OUT.before
> +
> +# add mcsum
> +echo "tune2fs -O metadata_csum test.img" >> $OUT
> +echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
> +
> +# change inode size
> +echo "tune2fs -I 512 test.img" >> $OUT
> +echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -I 512 $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# change uuid
> +echo "tune2fs -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 test.img" >> $OUT
> +echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# check
> +$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
> +
> +# dump and check
> +$DUMPE2FS $TMPFILE 2> /dev/null | egrep '^(Filesystem features:|Filesystem UUID:|Inode size:)' > $OUT.after
> +echo "Change in FS metadata:" >> $OUT
> +diff -u $OUT.before $OUT.after | tail -n +3 >> $OUT
> +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +rm $TMPFILE $OUT.before $OUT.after $CONF
> +
> +#
> +# Do the verification
> +#
> +
> +sed -f $cmd_dir/filter.sed -e "s;$TMPFILE;test.img;" -e 's/test_filesys:.*//g' < $OUT > $OUT.new
> +mv $OUT.new $OUT
> +
> +cmp -s $OUT $EXP
> +status=$?
> +
> +if [ "$status" = 0 ] ; then
> +	echo "$test_name: $test_description: ok"
> +	touch $test_name.ok
> +else
> +	echo "$test_name: $test_description: failed"
> +	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
> +fi
> +
> +unset IMAGE FSCK_OPT OUT EXP CONF
> 


Cheers, Andreas






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

* Re: [PATCH 7/9] tune2fs: confirm dangerous operations
  2016-02-15  0:49   ` Andreas Dilger
@ 2016-02-15 16:53     ` Darrick J. Wong
  2016-03-06  5:10       ` Theodore Ts'o
  0 siblings, 1 reply; 24+ messages in thread
From: Darrick J. Wong @ 2016-02-15 16:53 UTC (permalink / raw)
  To: Andreas Dilger; +Cc: tytso, linux-ext4

On Sun, Feb 14, 2016 at 05:49:42PM -0700, Andreas Dilger wrote:
> On Feb 13, 2016, at 3:38 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> > 
> > Give admins a short amount of time to confirm that they want to
> > proceed with a dangerous operation.  Refuse to perform the op
> > unless the filesystem is freshly checked.
> > 
> > Cc: Andreas Dilger <adilger@dilger.ca>
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > misc/tune2fs.c           |   40 +++++++++-----
> > tests/t_dangerous/expect |   97 +++++++++++++++++++++++++++++++++
> > tests/t_dangerous/name   |    1 
> > tests/t_dangerous/script |  134 ++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 259 insertions(+), 13 deletions(-)
> > create mode 100644 tests/t_dangerous/expect
> > create mode 100644 tests/t_dangerous/name
> > create mode 100644 tests/t_dangerous/script
> > 
> > 
> > diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> > index f9e654f..457e9e5 100644
> > --- a/misc/tune2fs.c
> > +++ b/misc/tune2fs.c
> > @@ -405,14 +405,24 @@ static int update_mntopts(ext2_filsys fs, char *mntopts)
> > 	return 0;
> > }
> > 
> > -static int check_fsck_needed(ext2_filsys fs)
> > +static void check_fsck_needed(ext2_filsys fs, const char *prompt)
> > {
> > -	if (fs->super->s_state & EXT2_VALID_FS)
> > -		return 0;
> > -	printf("\n%s\n", _(please_fsck));
> > -	if (mount_flags & EXT2_MF_READONLY)
> > -		printf("%s", _("(and reboot afterwards!)\n"));
> > -	return 1;
> > +	/* Refuse to modify anything but a freshly checked valid filesystem. */
> > +	if (!(fs->super->s_state & EXT2_VALID_FS) ||
> > +	    (fs->super->s_state & EXT2_ERROR_FS) ||
> > +	    (fs->super->s_lastcheck < fs->super->s_mtime)) {
> > +		printf("\n%s\n", _(please_fsck));
> > +		if (mount_flags & EXT2_MF_READONLY)
> > +			printf("%s", _("(and reboot afterwards!)\n"));
> > +		exit(1);
> > +	}
> > +
> > +	/* Give the admin a few seconds to bail out of a dangerous op. */
> > +	if (!getenv("TUNE2FS_FORCE_PROMPT") && (!isatty(0) || !isatty(1)))
> 
> One option would be to allow TUNE2FS_FORCE_PROMPT to contain the delay interval
> instead of hard-coding this to 5s below?  IMHO, 5s is only long enough for
> "oh sh**" and not enough to actually interrupt it unless you are watching the
> output closely...

<shrug> is the extra parsing code worth it, or would it be fine if we simply
set the timeout longer (15s?).

> 
> > +		return;
> > +
> > +	puts(prompt);
> > +	proceed_question(5);
> > }
> > 
> > static void request_dir_fsck_afterwards(ext2_filsys fs)
> > @@ -1145,8 +1155,8 @@ mmp_error:
> > 
> > 	if (FEATURE_ON(E2P_FEATURE_RO_INCOMPAT,
> > 		       EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
> > -		if (check_fsck_needed(fs))
> > -			exit(1);
> > +		check_fsck_needed(fs,
> > +			_("Enabling checksums could take some time."));
> > 		if (mount_flags & EXT2_MF_MOUNTED) {
> > 			fputs(_("Cannot enable metadata_csum on a mounted "
> > 				"filesystem!\n"), stderr);
> > @@ -1186,8 +1196,8 @@ mmp_error:
> > 			EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) {
> > 		__u32	test_features[3];
> > 
> > -		if (check_fsck_needed(fs))
> > -			exit(1);
> > +		check_fsck_needed(fs,
> > +			_("Disabling checksums could take some time."));
> 
> Does disabling checksums actually take more time than just turning off the
> feature in the superblock?  Without the feature flag all of the checksum
> fields should be ignored (whether they contain 0 or stale checksums), and
> they would be rewritten if/when the checksum feature is re-enabled.

IIRC removing checksums requires serious work -- changing the block group
descriptor checksums (if we're rolling back to GDT_CSUM) and removing the fake
directory entries that protect the checksum in dir blocks.

> 
> > 		if (mount_flags & EXT2_MF_MOUNTED) {
> > 			fputs(_("Cannot disable metadata_csum on a mounted "
> > 				"filesystem!\n"), stderr);
> > @@ -2784,6 +2794,8 @@ retry_open:
> > 			rc = 1;
> > 			goto closefs;
> > 		}
> > +		check_fsck_needed(fs,
> > +			_("Resizing inodes could take some time."));
> > 		/*
> > 		 * If inode resize is requested use the
> > 		 * Undo I/O manager
> > @@ -2999,8 +3011,10 @@ retry_open:
> > 				try_confirm_csum_seed_support();
> > 				exit(1);
> > 			}
> > -			if (check_fsck_needed(fs))
> > -				exit(1);
> > +			if (!ext2fs_has_feature_csum_seed(fs->super))
> > +				check_fsck_needed(fs,
> > +					_("Setting UUID on a checksummed "
> > +					  "filesystem could take some time."));
> 
> Wouldn't it make sense to enable the checksum seed feature in this case
> instead of rewriting the checksums for the whole filesystem?

It'll try to steer admins towards using the seed feature, but since the
seed is an incompat feature they can elect not to, in which case tune2fs
has to rewrite the whole fs.

--D

> 
> Cheers, Andreas
> 
> > 			/*
> > 			 * Determine if the block group checksums are
> > diff --git a/tests/t_dangerous/expect b/tests/t_dangerous/expect
> > new file mode 100644
> > index 0000000..353bd57
> > --- /dev/null
> > +++ b/tests/t_dangerous/expect
> > @@ -0,0 +1,97 @@
> > +tune2fs dangerous prompts test
> > +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
> > +
> > +tune2fs -O metadata_csum test.img
> > +
> > +Please run e2fsck on the filesystem.
> > +
> > +Exit status is 1
> > +tune2fs -O metadata_csum test.img
> > +Exit status is 0
> > +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 -O metadata_csum test.img
> > +Enabling checksums could take some time.
> > +Proceed anyway (or wait 5 seconds) ? (y,n) 
> > +Exit status is 1
> > +tune2fs -I 512 test.img
> > +Resizing inodes could take some time.
> > +Proceed anyway (or wait 5 seconds) ? (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) ? (y,n) 
> > +Exit status is 1
> > +
> > +Change in FS metadata:
> > +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 -O metadata_csum test.img
> > +Enabling checksums could take some time.
> > +Proceed anyway (or wait 5 seconds) ? (y,n) 
> > +Please run e2fsck -D on the filesystem.
> > +
> > +Exit status is 0
> > +test_filesys was not cleanly unmounted, check forced.
> > +Pass 1: Checking inodes, blocks, and sizes
> > +Pass 2: Checking directory structure
> > +Pass 3: Checking directory connectivity
> > +Pass 3A: Optimizing directories
> > +Pass 4: Checking reference counts
> > +Pass 5: Checking group summary information
> > +
> > +
> > +
> > +tune2fs -I 512 test.img
> > +Resizing inodes could take some time.
> > +Proceed anyway (or wait 5 seconds) ? (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.
> > +Proceed anyway (or wait 5 seconds) ? (y,n) Exit status is 0
> > +Backing up journal inode block information.
> > +
> > +
> > +Change in FS metadata:
> > +@@ -1,3 +1,3 @@
> > +-Filesystem UUID:          6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf
> > +-Filesystem features:      has_journal ext_attr resize_inode dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
> > +-Inode size:	          256
> > ++Filesystem UUID:          f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0
> > ++Filesystem features:      has_journal ext_attr resize_inode dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> > ++Inode size:	          512
> > +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
> > diff --git a/tests/t_dangerous/name b/tests/t_dangerous/name
> > new file mode 100644
> > index 0000000..c9a1030
> > --- /dev/null
> > +++ b/tests/t_dangerous/name
> > @@ -0,0 +1 @@
> > +dangerous tune2fs operation prompts
> > diff --git a/tests/t_dangerous/script b/tests/t_dangerous/script
> > new file mode 100644
> > index 0000000..dc70ad2
> > --- /dev/null
> > +++ b/tests/t_dangerous/script
> > @@ -0,0 +1,134 @@
> > +FSCK_OPT=-fn
> > +OUT=$test_name.log
> > +EXP=$test_dir/expect
> > +CONF=$TMPFILE.conf
> > +
> > +cat > $CONF << ENDL
> > +[fs_types]
> > +	ext4h = {
> > +		features = has_journal,extent,huge_file,^flex_bg,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,resize_inode,64bit
> > +		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
> > +
> > +MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
> > +
> > +# trigger a filesystem check
> > +$DEBUGFS -w -R 'ssv mtime now' $TMPFILE > /dev/null 2>&1
> > +$DEBUGFS -w -R 'ssv lastcheck 20000' $TMPFILE > /dev/null 2>&1
> > +
> > +# add mcsum
> > +echo "tune2fs -O metadata_csum test.img" >> $OUT
> > +$TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# check fs
> > +$FSCK -f -y -N test_filesys $TMPFILE > /dev/null 2>&1
> > +
> > +# add mcsum
> > +echo "tune2fs -O metadata_csum test.img" >> $OUT
> > +$TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -U 6fc3daa4-180d-4f2b-a6f2-f7a5efb79bcf $TMPFILE 524288 >> $OUT 2>&1
> > +
> > +# dump and check
> > +$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
> > +
> > +# add mcsum
> > +echo "tune2fs -O metadata_csum test.img" >> $OUT
> > +echo 'n' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# change inode size
> > +echo "tune2fs -I 512 test.img" >> $OUT
> > +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
> > +
> > +# dump and check
> > +$DUMPE2FS $TMPFILE 2> /dev/null | grep '^Group 0:' -B99 -A20 | sed -f $cmd_dir/filter.sed > $OUT.after
> > +echo "Change in FS metadata:" >> $OUT
> > +diff -u $OUT.before $OUT.after | tail -n +3 >> $OUT
> > +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +$DUMPE2FS $TMPFILE 2> /dev/null | egrep '^(Filesystem features:|Filesystem UUID:|Inode size:)' > $OUT.before
> > +
> > +# add mcsum
> > +echo "tune2fs -O metadata_csum test.img" >> $OUT
> > +echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -O metadata_csum $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
> > +
> > +# change inode size
> > +echo "tune2fs -I 512 test.img" >> $OUT
> > +echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -I 512 $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# change uuid
> > +echo "tune2fs -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 test.img" >> $OUT
> > +echo 'y' | TUNE2FS_FORCE_PROMPT=1 $TUNE2FS -U f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# check
> > +$FSCK -yD -N test_filesys $TMPFILE >> $OUT 2>&1
> > +
> > +# dump and check
> > +$DUMPE2FS $TMPFILE 2> /dev/null | egrep '^(Filesystem features:|Filesystem UUID:|Inode size:)' > $OUT.after
> > +echo "Change in FS metadata:" >> $OUT
> > +diff -u $OUT.before $OUT.after | tail -n +3 >> $OUT
> > +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +rm $TMPFILE $OUT.before $OUT.after $CONF
> > +
> > +#
> > +# Do the verification
> > +#
> > +
> > +sed -f $cmd_dir/filter.sed -e "s;$TMPFILE;test.img;" -e 's/test_filesys:.*//g' < $OUT > $OUT.new
> > +mv $OUT.new $OUT
> > +
> > +cmp -s $OUT $EXP
> > +status=$?
> > +
> > +if [ "$status" = 0 ] ; then
> > +	echo "$test_name: $test_description: ok"
> > +	touch $test_name.ok
> > +else
> > +	echo "$test_name: $test_description: failed"
> > +	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
> > +fi
> > +
> > +unset IMAGE FSCK_OPT OUT EXP CONF
> > 
> 
> 
> Cheers, Andreas
> 
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 1/9] libext2fs: store checksum seed in superblock
  2016-02-13 22:37 ` [PATCH 1/9] libext2fs: store checksum seed in superblock Darrick J. Wong
@ 2016-03-05 23:21   ` Theodore Ts'o
  0 siblings, 0 replies; 24+ messages in thread
From: Theodore Ts'o @ 2016-03-05 23:21 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-ext4

On Sat, Feb 13, 2016 at 02:37:32PM -0800, Darrick J. Wong wrote:
> Allow the filesystem to store the metadata checksum seed in the
> superblock and add an incompat feature to say that we're using it.
> This enables tune2fs to change the UUID on a mounted metadata_csum
> FS without having to (racy!) rewrite all disk metadata.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks, applied.

					- Ted

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

* Re: [PATCH 2/9] tune2fs: allow user to turn on saving the checksum seed
  2016-02-13 22:37 ` [PATCH 2/9] tune2fs: allow user to turn on saving the checksum seed Darrick J. Wong
@ 2016-03-05 23:36   ` Theodore Ts'o
  0 siblings, 0 replies; 24+ messages in thread
From: Theodore Ts'o @ 2016-03-05 23:36 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-ext4

On Sat, Feb 13, 2016 at 02:37:38PM -0800, Darrick J. Wong wrote:
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks, applied.

						0 Ted

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

* Re: [PATCH 3/9] e2fsck: check the checksum seed feature flag is set correctly
  2016-02-13 22:37 ` [PATCH 3/9] e2fsck: check the checksum seed feature flag is set correctly Darrick J. Wong
@ 2016-03-05 23:37   ` Theodore Ts'o
  0 siblings, 0 replies; 24+ messages in thread
From: Theodore Ts'o @ 2016-03-05 23:37 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-ext4

On Sat, Feb 13, 2016 at 02:37:44PM -0800, Darrick J. Wong wrote:
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks, applied.

						- Ted

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

* Re: [PATCH 4/9] mke2fs: store checksum seed at format time
  2016-02-13 22:37 ` [PATCH 4/9] mke2fs: store checksum seed at format time Darrick J. Wong
@ 2016-03-06  0:19   ` Theodore Ts'o
  0 siblings, 0 replies; 24+ messages in thread
From: Theodore Ts'o @ 2016-03-06  0:19 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-ext4

On Sat, Feb 13, 2016 at 02:37:51PM -0800, Darrick J. Wong wrote:
> Allow users to turn on metadata_csum_seed at format time so that UUIDs
> can be live-changed at any time.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Applied, thanks.

						- Ted

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

* Re: [PATCH 5/9] tests: check proper operation of metadata_csum_seed
  2016-02-13 22:37 ` [PATCH 5/9] tests: check proper operation of metadata_csum_seed Darrick J. Wong
@ 2016-03-06  0:20   ` Theodore Ts'o
  0 siblings, 0 replies; 24+ messages in thread
From: Theodore Ts'o @ 2016-03-06  0:20 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-ext4

On Sat, Feb 13, 2016 at 02:37:57PM -0800, Darrick J. Wong wrote:
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Applied, thanks.

						- Ted

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

* Re: [PATCH 6/9] filefrag: accommodate holes when calculating expected values
  2016-02-13 22:38 ` [PATCH 6/9] filefrag: accommodate holes when calculating expected values Darrick J. Wong
@ 2016-03-06  2:41   ` Theodore Ts'o
  0 siblings, 0 replies; 24+ messages in thread
From: Theodore Ts'o @ 2016-03-06  2:41 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-ext4

On Sat, Feb 13, 2016 at 02:38:04PM -0800, Darrick J. Wong wrote:
> Currently, filefrag's "expected physical block" column expects extent
> records to be physically adjacent regardless of the amount of logical
> block space between the two records.  This means that if we punch a
> hole in a file, we get reports like this:
> 
> ext:   logical_offset:   physical_offset: length:  expected: flags:
> 4:     4096..    8343:   57376..  61623:   4248:
> 5:     8345..   10313:   61625..  63593:   1969:   61624:
> 
> Notice how it expects 8345 to map to 61624, and scores this against
> the fragmentation of the file.  Flagging this as "unexpected" is
> incorrect because the gap in the logical mapping is exactly the same
> size as the gap in the physical extents.
> 
> Furthermore, this particular mapping leaves the door open to the
> optimal mapping -- if a write to block 8344 causes it to be mapped to
> 61624, the entire range 4096-10313 can be mapped with a single extent.
> Until that happens, there's no way to combine extents 4 and 5 because
> of the gap in the logical mapping at block 8344.
> 
> Therefore, tweak the extent report to account for holes.
> 
> v2: Make it work for extents crossing FIEMAP calls, and clean up the
> FIBMAP version to report correct expected values.
> 
> v3: Don't count physically but not logically contiguous extents
> in the fragmentation summary.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks, applied.

						- Ted

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

* Re: [PATCH 9/9] libext2fs: sort keys for xattr blocks
  2016-02-13 22:38 ` [PATCH 9/9] libext2fs: sort keys for xattr blocks Darrick J. Wong
  2016-02-14 10:37   ` Richard Purdie
@ 2016-03-06  3:55   ` Theodore Ts'o
  2016-03-06 22:08     ` Darrick J. Wong
  1 sibling, 1 reply; 24+ messages in thread
From: Theodore Ts'o @ 2016-03-06  3:55 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-ext4, Darren Hart, Richard Purdie

On Sat, Feb 13, 2016 at 02:38:24PM -0800, Darrick J. Wong wrote:
> Richard Purdie reports that libext2fs doesn't sort attribute keys in
> the xattr block correctly, causing the kernel to return -ENODATA when
> querying attributes that should be there.  Therefore, sort attributes
> so that whatever ends up in the xattr block is sorted according to
> what the kernel expects.
> 
> Cc: Darren Hart <dvhart@linux.intel.com>
> Reported-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Applied, with one minor change.  You should never use something like
/tmp/b in a test script becuase it's possible that /tmp/b might
already exist, and might be not even owned by the user.  (In my case
/tmp/b was a root owned file that was sufficiently big that it it
caused ea_set command to fail with a "out of space" error.)

If /tmp/b was writable by the user and was something previous, then
the developer might get after the test script executes "rm -rf /tmp/b".

						- Ted

diff --git a/tests/d_xattr_sorting/script b/tests/d_xattr_sorting/script
index 30c189a..30187f1 100644
--- a/tests/d_xattr_sorting/script
+++ b/tests/d_xattr_sorting/script
@@ -17,16 +17,18 @@ $MKE2FS -Fq $TMPFILE 512 > /dev/null 2>&1
 status=$?
 echo Exit status is $status >> $OUT
 
-perl -e 'print "x" x 256;' > /tmp/b
+B=$(mktemp ${TMPDIR:-/tmp}/b.XXXXXX)
+
+perl -e 'print "x" x 256;' > $B
 
 echo "ea_set / security.SMEG64 -f /tmp/b" > $OUT.new
-$DEBUGFS -w -R "ea_set / security.SMEG64 -f /tmp/b" $TMPFILE >> $OUT.new 2>&1
+$DEBUGFS -w -R "ea_set / security.SMEG64 -f $B" $TMPFILE >> $OUT.new 2>&1
 status=$?
 echo Exit status is $status >> $OUT.new
 sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
 
 echo "ea_set / security.imb -f /tmp/b" > $OUT.new
-$DEBUGFS -w -R "ea_set / security.imb -f /tmp/b" $TMPFILE >> $OUT.new 2>&1
+$DEBUGFS -w -R "ea_set / security.imb -f $B" $TMPFILE >> $OUT.new 2>&1
 status=$?
 echo Exit status is $status >> $OUT.new
 sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
@@ -37,7 +39,8 @@ status=$?
 echo Exit status is $status >> $OUT.new
 sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
 
-rm -rf /tmp/b
+rm -f $B
+unset B
 
 echo "ea_list /" > $OUT.new
 $DEBUGFS -w -R "ea_list /" $TMPFILE >> $OUT.new 2>&1

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

* Re: [PATCH 7/9] tune2fs: confirm dangerous operations
  2016-02-15 16:53     ` Darrick J. Wong
@ 2016-03-06  5:10       ` Theodore Ts'o
  2016-03-06  6:24         ` Darrick J. Wong
  0 siblings, 1 reply; 24+ messages in thread
From: Theodore Ts'o @ 2016-03-06  5:10 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Andreas Dilger, linux-ext4

I've tried it out, and I think 5 seconds is enough time.  I'll note
that just hitting the return key will abort the operation, so the user
has to type 'y', return in order to continue.  In fact I will probaby
change the prompt so it looks like this:

Proceed anyway (or wait %d seconds) (y,n) ? <n>

This should make it clear that if the sysadmin is not sure, just
hitting return is enough to cancel the operation.

> > On Feb 13, 2016, at 3:38 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> > > 
> > > Give admins a short amount of time to confirm that they want to
> > > proceed with a dangerous operation.  Refuse to perform the op
> > > unless the filesystem is freshly checked.
> > > 
> > > Cc: Andreas Dilger <adilger@dilger.ca>
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks, applied.

						- Ted

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

* Re: [PATCH 8/9] tune2fs: recover the journal
  2016-02-13 22:38 ` [PATCH 8/9] tune2fs: recover the journal Darrick J. Wong
@ 2016-03-06  5:23   ` Theodore Ts'o
  0 siblings, 0 replies; 24+ messages in thread
From: Theodore Ts'o @ 2016-03-06  5:23 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-ext4

On Sat, Feb 13, 2016 at 02:38:17PM -0800, Darrick J. Wong wrote:
> If the journal needs to be recovered to avoid clobbering whatever
> changes tune2fs makes, do so.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks, applied.

							- Ted

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

* Re: [PATCH 7/9] tune2fs: confirm dangerous operations
  2016-03-06  5:10       ` Theodore Ts'o
@ 2016-03-06  6:24         ` Darrick J. Wong
  0 siblings, 0 replies; 24+ messages in thread
From: Darrick J. Wong @ 2016-03-06  6:24 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Andreas Dilger, linux-ext4

On Sun, Mar 06, 2016 at 12:10:57AM -0500, Theodore Ts'o wrote:
> I've tried it out, and I think 5 seconds is enough time.  I'll note
> that just hitting the return key will abort the operation, so the user
> has to type 'y', return in order to continue.  In fact I will probaby
> change the prompt so it looks like this:
> 
> Proceed anyway (or wait %d seconds) (y,n) ? <n>

Seems fine to me.

--D

> 
> This should make it clear that if the sysadmin is not sure, just
> hitting return is enough to cancel the operation.
> 
> > > On Feb 13, 2016, at 3:38 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> > > > 
> > > > Give admins a short amount of time to confirm that they want to
> > > > proceed with a dangerous operation.  Refuse to perform the op
> > > > unless the filesystem is freshly checked.
> > > > 
> > > > Cc: Andreas Dilger <adilger@dilger.ca>
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Thanks, applied.
> 
> 						- Ted

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

* Re: [PATCH 9/9] libext2fs: sort keys for xattr blocks
  2016-03-06  3:55   ` Theodore Ts'o
@ 2016-03-06 22:08     ` Darrick J. Wong
  0 siblings, 0 replies; 24+ messages in thread
From: Darrick J. Wong @ 2016-03-06 22:08 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: linux-ext4, Darren Hart, Richard Purdie

On Sat, Mar 05, 2016 at 10:55:14PM -0500, Theodore Ts'o wrote:
> On Sat, Feb 13, 2016 at 02:38:24PM -0800, Darrick J. Wong wrote:
> > Richard Purdie reports that libext2fs doesn't sort attribute keys in
> > the xattr block correctly, causing the kernel to return -ENODATA when
> > querying attributes that should be there.  Therefore, sort attributes
> > so that whatever ends up in the xattr block is sorted according to
> > what the kernel expects.
> > 
> > Cc: Darren Hart <dvhart@linux.intel.com>
> > Reported-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Applied, with one minor change.  You should never use something like
> /tmp/b in a test script becuase it's possible that /tmp/b might
> already exist, and might be not even owned by the user.  (In my case
> /tmp/b was a root owned file that was sufficiently big that it it
> caused ea_set command to fail with a "out of space" error.)
> 
> If /tmp/b was writable by the user and was something previous, then
> the developer might get after the test script executes "rm -rf /tmp/b".

Oops, my bad.  Thank you for fixing this. :)

--D

> 
> 						- Ted
> 
> diff --git a/tests/d_xattr_sorting/script b/tests/d_xattr_sorting/script
> index 30c189a..30187f1 100644
> --- a/tests/d_xattr_sorting/script
> +++ b/tests/d_xattr_sorting/script
> @@ -17,16 +17,18 @@ $MKE2FS -Fq $TMPFILE 512 > /dev/null 2>&1
>  status=$?
>  echo Exit status is $status >> $OUT
>  
> -perl -e 'print "x" x 256;' > /tmp/b
> +B=$(mktemp ${TMPDIR:-/tmp}/b.XXXXXX)
> +
> +perl -e 'print "x" x 256;' > $B
>  
>  echo "ea_set / security.SMEG64 -f /tmp/b" > $OUT.new
> -$DEBUGFS -w -R "ea_set / security.SMEG64 -f /tmp/b" $TMPFILE >> $OUT.new 2>&1
> +$DEBUGFS -w -R "ea_set / security.SMEG64 -f $B" $TMPFILE >> $OUT.new 2>&1
>  status=$?
>  echo Exit status is $status >> $OUT.new
>  sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
>  
>  echo "ea_set / security.imb -f /tmp/b" > $OUT.new
> -$DEBUGFS -w -R "ea_set / security.imb -f /tmp/b" $TMPFILE >> $OUT.new 2>&1
> +$DEBUGFS -w -R "ea_set / security.imb -f $B" $TMPFILE >> $OUT.new 2>&1
>  status=$?
>  echo Exit status is $status >> $OUT.new
>  sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
> @@ -37,7 +39,8 @@ status=$?
>  echo Exit status is $status >> $OUT.new
>  sed -f $cmd_dir/filter.sed $OUT.new >> $OUT
>  
> -rm -rf /tmp/b
> +rm -f $B
> +unset B
>  
>  echo "ea_list /" > $OUT.new
>  $DEBUGFS -w -R "ea_list /" $TMPFILE >> $OUT.new 2>&1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2016-03-06 22:08 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-13 22:37 [PATCH 0/9] e2fsprogs: save checksum seeds; fix broken xattr editing; misc fixes Darrick J. Wong
2016-02-13 22:37 ` [PATCH 1/9] libext2fs: store checksum seed in superblock Darrick J. Wong
2016-03-05 23:21   ` Theodore Ts'o
2016-02-13 22:37 ` [PATCH 2/9] tune2fs: allow user to turn on saving the checksum seed Darrick J. Wong
2016-03-05 23:36   ` Theodore Ts'o
2016-02-13 22:37 ` [PATCH 3/9] e2fsck: check the checksum seed feature flag is set correctly Darrick J. Wong
2016-03-05 23:37   ` Theodore Ts'o
2016-02-13 22:37 ` [PATCH 4/9] mke2fs: store checksum seed at format time Darrick J. Wong
2016-03-06  0:19   ` Theodore Ts'o
2016-02-13 22:37 ` [PATCH 5/9] tests: check proper operation of metadata_csum_seed Darrick J. Wong
2016-03-06  0:20   ` Theodore Ts'o
2016-02-13 22:38 ` [PATCH 6/9] filefrag: accommodate holes when calculating expected values Darrick J. Wong
2016-03-06  2:41   ` Theodore Ts'o
2016-02-13 22:38 ` [PATCH 7/9] tune2fs: confirm dangerous operations Darrick J. Wong
2016-02-15  0:49   ` Andreas Dilger
2016-02-15 16:53     ` Darrick J. Wong
2016-03-06  5:10       ` Theodore Ts'o
2016-03-06  6:24         ` Darrick J. Wong
2016-02-13 22:38 ` [PATCH 8/9] tune2fs: recover the journal Darrick J. Wong
2016-03-06  5:23   ` Theodore Ts'o
2016-02-13 22:38 ` [PATCH 9/9] libext2fs: sort keys for xattr blocks Darrick J. Wong
2016-02-14 10:37   ` Richard Purdie
2016-03-06  3:55   ` Theodore Ts'o
2016-03-06 22:08     ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.