[PATCH 1/4] Fix support for configure --enable-jbd-debug

[PATCH 1/4] Fix support for configure --enable-jbd-debug

[Theodore Ts'o]

We haven't used this in a while, so it's bitrotted a bit.  Fix it up
so that it works correctly.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 debugfs/debugfs.c       | 22 +++++++++++++++++++++-
 e2fsck/unix.c           |  2 +-
 lib/ext2fs/kernel-jbd.h | 12 ++++++++++++
 misc/fuse2fs.c          |  4 ++++
 misc/tune2fs.c          |  4 ++++
 5 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
index 0cf0837..ba8be40 100644
--- a/debugfs/debugfs.c
+++ b/debugfs/debugfs.c
@@ -41,6 +41,10 @@ extern char *optarg;
 #define BUFSIZ 8192
 #endif
 
+#ifdef CONFIG_JBD_DEBUG		/* Enabled by configure --enable-jbd-debug */
+int journal_enable_debug = -1;
+#endif
+
 ss_request_table *extra_cmds;
 const char *debug_prog_name;
 int sci_idx;
@@ -76,7 +80,7 @@ static int debugfs_setup_tdb(const char *device_name, char *undo_file,
 	 * Configuration via a conf file would be
 	 * nice
 	 */
-	tdb_dir = getenv("E2FSPROGS_UNDO_DIR");
+	tdb_dir = ss_safe_getenv("E2FSPROGS_UNDO_DIR");
 	if (!tdb_dir)
 		tdb_dir = "/var/lib/e2fsprogs";
 
@@ -2395,6 +2399,9 @@ int main(int argc, char **argv)
 	const char	*opt_string = "niwcR:f:b:s:Vd:Dz:";
 	char		*undo_file = NULL;
 #endif
+#ifdef CONFIG_JBD_DEBUG
+	char		*jbd_debug;
+#endif
 
 	if (debug_prog_name == 0)
 #ifdef READ_ONLY
@@ -2406,6 +2413,19 @@ int main(int argc, char **argv)
 	fprintf (stderr, "%s %s (%s)\n", debug_prog_name,
 		 E2FSPROGS_VERSION, E2FSPROGS_DATE);
 
+#ifdef CONFIG_JBD_DEBUG
+	jbd_debug = ss_safe_getenv("DEBUGFS_JBD_DEBUG");
+	if (jbd_debug) {
+		int res = sscanf(jbd_debug, "%d", &journal_enable_debug);
+
+		if (res != 1) {
+			fprintf(stderr,
+				"DEBUGFS_JBD_DEBUG \"%s\" not an integer\n\n",
+				jbd_debug);
+			exit(1);
+		}
+	}
+#endif
 	while ((c = getopt (argc, argv, opt_string)) != EOF) {
 		switch (c) {
 		case 'R':
diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index e54e2ce..959b4dd 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -66,7 +66,7 @@ static char *bad_blocks_file;
 
 e2fsck_t e2fsck_global_ctx;	/* Try your very best not to use this! */
 
-#ifdef CONFIG_JBD_DEBUG		/* Enabled by configure --enable-jfs-debug */
+#ifdef CONFIG_JBD_DEBUG		/* Enabled by configure --enable-jbd-debug */
 int journal_enable_debug = -1;
 #endif
 
diff --git a/lib/ext2fs/kernel-jbd.h b/lib/ext2fs/kernel-jbd.h
index 842809d..c1fec3f 100644
--- a/lib/ext2fs/kernel-jbd.h
+++ b/lib/ext2fs/kernel-jbd.h
@@ -46,8 +46,20 @@ extern int journal_enable_debug;
 	} while (0)
 #else
 #ifdef __GNUC__
+#ifdef __KERNEL__
 #define jbd_debug(f, a...)	/**/
 #else
+extern int journal_enable_debug;
+#define jbd_debug(n, f, a...)						\
+	do {								\
+		if ((n) <= journal_enable_debug) {			\
+			printf("(%s, %d): %s: ",			\
+				__FILE__, __LINE__, __func__);		\
+			printf(f, ## a);				\
+		}							\
+	} while (0)
+#endif /*__KERNEL__ */
+#else
 #define jbd_debug(f, ...)	/**/
 #endif
 #endif
diff --git a/misc/fuse2fs.c b/misc/fuse2fs.c
index 4412fe3..c75527e 100644
--- a/misc/fuse2fs.c
+++ b/misc/fuse2fs.c
@@ -95,6 +95,10 @@ static ext2_filsys global_fs; /* Try not to use this directly */
 
 errcode_t ext2fs_run_ext3_journal(ext2_filsys *fs);
 
+#ifdef CONFIG_JBD_DEBUG		/* Enabled by configure --enable-jbd-debug */
+int journal_enable_debug = -1;
+#endif
+
 /* ACL translation stuff */
 #ifdef TRANSLATE_LINUX_ACLS
 /*
diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 10ce58f..a1923f9 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -124,6 +124,10 @@ static const char *please_dir_fsck =
 void do_findfs(int argc, char **argv);
 #endif
 
+#ifdef CONFIG_JBD_DEBUG		/* Enabled by configure --enable-jbd-debug */
+int journal_enable_debug = -1;
+#endif
+
 static void usage(void)
 {
 	fprintf(stderr,
-- 
2.5.0

[PATCH 2/4] e2fsck: use specific CRC and corruption errors in journal recovery

[Theodore Ts'o]

Sync up with kernel commit 6a797d27: "ext4: call out CRC and
corruption errors with specific error codes".

This allows us to distinguish between CRC errors and I/O errors.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 e2fsck/jfs_user.h | 7 +++++++
 e2fsck/recovery.c | 8 ++++----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/e2fsck/jfs_user.h b/e2fsck/jfs_user.h
index be81f43..f46bb1e 100644
--- a/e2fsck/jfs_user.h
+++ b/e2fsck/jfs_user.h
@@ -181,6 +181,13 @@ extern e2fsck_t e2fsck_global_ctx;  /* Try your very best not to use this! */
 
 #endif /* DEBUGFS */
 
+#ifndef EFSBADCRC
+#define EFSBADCRC	EBADMSG		/* Bad CRC detected */
+#endif
+#ifndef EFSCORRUPTED
+#define EFSCORRUPTED	EUCLEAN		/* Filesystem is corrupted */
+#endif
+
 /* recovery.c */
 extern int	journal_recover    (journal_t *journal);
 extern int	journal_skip_recovery (journal_t *);
diff --git a/e2fsck/recovery.c b/e2fsck/recovery.c
index 41bda03..b828ed4 100644
--- a/e2fsck/recovery.c
+++ b/e2fsck/recovery.c
@@ -140,7 +140,7 @@ static int jread(struct buffer_head **bhp, journal_t *journal,
 
 	if (offset >= journal->j_maxlen) {
 		printk(KERN_ERR "JBD2: corrupted journal superblock\n");
-		return -EIO;
+		return -EFSCORRUPTED;
 	}
 
 	err = journal_bmap(journal, offset, &blocknr);
@@ -524,7 +524,7 @@ static int do_one_pass(journal_t *journal,
 			if (descr_csum_size > 0 &&
 			    !jbd2_descr_block_csum_verify(journal,
 							  bh->b_data)) {
-				err = -EIO;
+				err = -EFSBADCRC;
 				brelse(bh);
 				goto failed;
 			}
@@ -598,7 +598,7 @@ static int do_one_pass(journal_t *journal,
 						journal, tag, obh->b_data,
 						ext2fs_be32_to_cpu(tmp->h_sequence))) {
 						brelse(obh);
-						success = -EIO;
+						success = -EFSBADCRC;
 						printk(KERN_ERR "JBD2: Invalid "
 						       "checksum recovering "
 						       "block %llu in log\n",
@@ -844,7 +844,7 @@ static int scan_revoke_records(journal_t *journal, struct buffer_head *bh,
 	rcount = ext2fs_be32_to_cpu(header->r_count);
 
 	if (!jbd2_revoke_block_csum_verify(journal, header))
-		return -EINVAL;
+		return -EFSBADCRC;
 
 	if (journal_has_csum_v2or3(journal))
 		csum_size = sizeof(struct journal_revoke_tail);
-- 
2.5.0

[PATCH 3/4] e2fsck: don't try to set a UUID on metadata_csum file systems

[Theodore Ts'o]

If the UUID field is NULL, e2fsck will try to generate and set a new
UUID.  This will cause massive problems if the metadata_csum feature
is set, so avoid doing so in that case.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 e2fsck/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/e2fsck/super.c b/e2fsck/super.c
index e09c14c..dec70bd 100644
--- a/e2fsck/super.c
+++ b/e2fsck/super.c
@@ -756,6 +756,7 @@ void check_super_block(e2fsck_t ctx)
 	 * if the id changes under the kernel remounting rw may fail.
 	 */
 	if (!(ctx->options & E2F_OPT_READONLY) && uuid_is_null(sb->s_uuid) &&
+	    !ext2fs_has_feature_metadata_csum(ctx->fs->super) &&
 	    (!csum_flag || !(ctx->mount_flags & EXT2_MF_MOUNTED))) {
 		if (fix_problem(ctx, PR_0_ADD_UUID, &pctx)) {
 			uuid_generate(sb->s_uuid);
-- 
2.5.0

Re: [PATCH 3/4] e2fsck: don't try to set a UUID on metadata_csum file systems

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1307 bytes --]

On Apr 15, 2016, at 7:13 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> 
> If the UUID field is NULL, e2fsck will try to generate and set a new
> UUID.  This will cause massive problems if the metadata_csum feature
> is set, so avoid doing so in that case.

Should enabling the metadata csum feature complain/fail if the checksum
is unset?  It doesn't make sense to use a NULL UUID as the checksum salt.

Cheers, Andreas

> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
> e2fsck/super.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/e2fsck/super.c b/e2fsck/super.c
> index e09c14c..dec70bd 100644
> --- a/e2fsck/super.c
> +++ b/e2fsck/super.c
> @@ -756,6 +756,7 @@ void check_super_block(e2fsck_t ctx)
> 	 * if the id changes under the kernel remounting rw may fail.
> 	 */
> 	if (!(ctx->options & E2F_OPT_READONLY) && uuid_is_null(sb->s_uuid) &&
> +	    !ext2fs_has_feature_metadata_csum(ctx->fs->super) &&
> 	    (!csum_flag || !(ctx->mount_flags & EXT2_MF_MOUNTED))) {
> 		if (fix_problem(ctx, PR_0_ADD_UUID, &pctx)) {
> 			uuid_generate(sb->s_uuid);
> --
> 2.5.0
> 
> --
> 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


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] e2fsck: don't try to set a UUID on metadata_csum file systems

[Theodore Ts'o]

On Fri, Apr 15, 2016 at 09:06:04PM -0600, Andreas Dilger wrote:
> > If the UUID field is NULL, e2fsck will try to generate and set a new
> > UUID.  This will cause massive problems if the metadata_csum feature
> > is set, so avoid doing so in that case.
> 
> Should enabling the metadata csum feature complain/fail if the checksum
> is unset?  It doesn't make sense to use a NULL UUID as the checksum salt.

Well, for hard disks, it's certainly not a great idea, although it
won't automatically break things.  For images that are being cloned
for VM's, the UUID's are all going to be the same anyway, so whether
they are all NULL or all "964ef7ca-ddce-4ce2-906e-d2d69be29b1f"
doesn't really matter a whole lot.  And the same holds true if someone
recreates a file system on a HDD with an explicitly specified UUID
which is the same as what had previously been used on that file
system.

The real problem was the mistake we made in overloading the UUID is
the checksum seed.  We've fixed that, but it's going to be quite a
while before we can assume that it is generally available.

I could see adding a warning message to mke2fs if the user specifies a
UUID (whether it be NULL or any other value) but I'm not really sure
it's worth it.

Cheers,

						- Ted
						

[PATCH 4/4] e2fsck: don't abort if the journal is corrupted due to checksum errors

[Theodore Ts'o]

If e2fsck_run_ext3_journal() returns an error indicating that a CRC
error was detected, we shouldn't abort, but instead proceed so the
file system can be fixed.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 e2fsck/unix.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 959b4dd..cfd33e4 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -53,6 +53,7 @@ extern int optind;
 #include "support/plausible.h"
 #include "e2fsck.h"
 #include "problem.h"
+#include "jfs_user.h"
 #include "../version.h"
 
 /* Command line options */
@@ -1645,7 +1646,7 @@ failure:
 		retval = e2fsck_check_ext3_journal(ctx);
 		if (retval) {
 			com_err(ctx->program_name, retval,
-				_("while checking ext3 journal for %s"),
+				_("while checking journal for %s"),
 				ctx->device_name);
 			fatal_error(ctx, 0);
 		}
@@ -1677,9 +1678,10 @@ failure:
 			retval = e2fsck_run_ext3_journal(ctx);
 			if (retval) {
 				com_err(ctx->program_name, retval,
-				_("while recovering ext3 journal of %s"),
+				_("while recovering journal of %s"),
 					ctx->device_name);
-				fatal_error(ctx, 0);
+				if ((retval != EFSBADCRC) && (retval != EFSCORRUPTED))
+					fatal_error(ctx, 0);
 			}
 			ext2fs_close_free(&ctx->fs);
 			ctx->flags |= E2F_FLAG_RESTARTED;
-- 
2.5.0