2007-03-14 21:13:15

by Kalpak Shah

[permalink] [raw]
Subject: [PATCH] e2fsprogs: Check journal inode sanity and recreate journal

Hi,

Currently e2fsck does not check the sanity of all the journal blocks.
E2fsck does check EXT2_N_BLOCKS to check is they are sane but if any
indirect blocks belonging to the journal become sparse due to
corruption, e2fsck would not be able to correct this. If such a
filesystem is mounted, it suddenly goes read-only if the sparse blocks
are accessed. So this patch adds sanity checking for all the journal
blocks.

Also, it adds support to recreate the journal if the journal had been
deleted. This means that an ext3 filesystem does not end up as an ext2
filesystem. The journal size is taken from the backup journal inode in
the superblock, if it exists or the default journal size will be used to
recreate the journal.

Also another small change is that, if the backup journal inode from the
superblock has been used, it should be written to disk.

This patch caused some existing regression tests to fail since the
journal was being recreated. I have attached a patch to correct the
expected output for f_badjourblks and f_miss_journal tests. Also I have
attached a regression test for this patch, f_badjour_indblks, which has
a corrupt indirect block in the journal inode.

Signed-off-by: Kalpak Shah <[email protected]>
Signed-off-by: Andreas Dilger <[email protected]>

Index: e2fsprogs-1.39/e2fsck/journal.c
===================================================================
--- e2fsprogs-1.39.orig/e2fsck/journal.c
+++ e2fsprogs-1.39/e2fsck/journal.c
@@ -206,6 +206,7 @@ static errcode_t e2fsck_get_journal(e2fs
int ext_journal = 0;
int tried_backup_jnl = 0;
int i;
+ unsigned int lblock;

clear_problem_context(&pctx);

@@ -283,6 +284,13 @@ static errcode_t e2fsck_get_journal(e2fs
goto try_backup_journal;
}
}
+ if (tried_backup_jnl && !(ctx->options & E2F_OPT_READONLY)) {
+ retval = ext2fs_write_inode(ctx->fs, sb->s_journal_inum,
+ &j_inode->i_ext2);
+ if (retval)
+ goto errout;
+ }
+
journal->j_maxlen = j_inode->i_ext2.i_size / journal->j_blocksize;

#ifdef USE_INODE_IO
@@ -299,6 +307,20 @@ static errcode_t e2fsck_get_journal(e2fs
if ((retval = journal_bmap(journal, 0, &start)) != 0)
goto errout;
#endif
+ for (lblock = 0; lblock < j_inode->i_ext2.i_size /
+ journal->j_blocksize; lblock++) {
+ unsigned long pblock;
+
+ if ((retval = journal_bmap(journal, lblock,
+ &pblock)) != 0) {
+ goto errout;
+ }
+ if (pblock == 0 || pblock < sb->s_first_data_block ||
+ pblock >= sb->s_blocks_count) {
+ retval = EXT2_ET_BAD_BLOCK_NUM;
+ goto errout;
+ }
+ }
} else {
ext_journal = 1;
if (!ctx->journal_name) {
@@ -418,7 +440,7 @@ static errcode_t e2fsck_journal_fix_bad_
"filesystem is now ext2 only ***\n\n");
sb->s_feature_compat &= ~EXT3_FEATURE_COMPAT_HAS_JOURNAL;
sb->s_journal_inum = 0;
- ctx->flags |= E2F_FLAG_JOURNAL_INODE; /* FIXME: todo */
+ ctx->flags |= E2F_FLAG_JOURNAL_INODE;
e2fsck_clear_recover(ctx, 1);
return 0;
}
Index: e2fsprogs-1.39/e2fsck/unix.c
===================================================================
--- e2fsprogs-1.39.orig/e2fsck/unix.c
+++ e2fsprogs-1.39/e2fsck/unix.c
@@ -847,6 +847,7 @@ int main (int argc, char *argv[])
e2fsck_t ctx;
struct problem_context pctx;
int flags, run_result;
+ int journal_blocks, journal_size;

clear_problem_context(&pctx);
#ifdef MTRACE
@@ -1141,8 +1142,43 @@ restart:
" but we'll try to go on...\n"));
}

+ /*
+ * Save the journal size in megabytes.
+ * Try and use the journal size from the backup else let e2fsck
+ * find the default journal size.
+ */
+ if (sb->s_jnl_backup_type == EXT3_JNL_BACKUP_BLOCKS)
+ journal_size = sb->s_jnl_blocks[16] >> 20;
+ else
+ journal_size = -1;
+
run_result = e2fsck_run(ctx);
e2fsck_clear_progbar(ctx);
+
+ if (ctx->flags & E2F_FLAG_JOURNAL_INODE) {
+ if (fix_problem(ctx, PR_6_RECREATE_JOURNAL, &pctx)) {
+ journal_blocks = ext2fs_figure_journal_size(journal_size, fs);
+
+ if (journal_blocks <= 0) {
+ fs->super->s_feature_compat &=
+ ~EXT3_FEATURE_COMPAT_HAS_JOURNAL;
+ goto no_journal;
+ }
+ printf(_("Creating journal (%d blocks): "), journal_blocks);
+ fflush(stdout);
+ retval = ext2fs_add_journal_inode(fs, journal_blocks, 0);
+ if (retval) {
+ com_err("Error ", retval,
+ _("\n\twhile trying to create journal"));
+ goto no_journal;
+ }
+ printf(_(" Done.\n"));
+ printf(_("\n*** journal has been re-created - "
+ "filesystem is now ext3 again ***\n"));
+ }
+ }
+no_journal:
+
if (run_result == E2F_FLAG_RESTART) {
printf(_("Restarting e2fsck from the beginning...\n"));
retval = e2fsck_reset_context(ctx);
Index: e2fsprogs-1.39/lib/ext2fs/ext2fs.h
===================================================================
--- e2fsprogs-1.39.orig/lib/ext2fs/ext2fs.h
+++ e2fsprogs-1.39/lib/ext2fs/ext2fs.h
@@ -869,6 +869,8 @@ extern errcode_t ext2fs_add_journal_devi
ext2_filsys journal_dev);
extern errcode_t ext2fs_add_journal_inode(ext2_filsys fs, blk_t size,
int flags);
+extern int ext2fs_figure_journal_size(int size, ext2_filsys fs);
+

/* openfs.c */
extern errcode_t ext2fs_open(const char *name, int flags, int superblock,
Index: e2fsprogs-1.39/lib/ext2fs/mkjournal.c
===================================================================
--- e2fsprogs-1.39.orig/lib/ext2fs/mkjournal.c
+++ e2fsprogs-1.39/lib/ext2fs/mkjournal.c
@@ -308,6 +308,57 @@ errcode_t ext2fs_add_journal_device(ext2
}

/*
+ * Determine the number of journal blocks to use, either via
+ * user-specified # of megabytes, or via some intelligently selected
+ * defaults.
+ *
+ * Find a reasonable journal file size (in blocks) given the number of blocks
+ * in the filesystem. For very small filesystems, it is not reasonable to
+ * have a journal that fills more than half of the filesystem.
+ */
+int ext2fs_figure_journal_size(int size, ext2_filsys fs)
+{
+ blk_t j_blocks;
+
+ if (fs->super->s_blocks_count < 2048) {
+ fputs(("\nFilesystem too small for a journal\n"), stderr);
+ return 0;
+ }
+
+ if (size > 0) {
+ j_blocks = size * 1024 / (fs->blocksize / 1024);
+ if (j_blocks < 1024 || j_blocks > 102400) {
+ fprintf(stderr, ("\nThe requested journal "
+ "size is %d blocks; it must be\n"
+ "between 1024 and 102400 blocks. "
+ "Aborting.\n"),
+ j_blocks);
+ return -ERANGE;
+ }
+ if (j_blocks > fs->super->s_free_blocks_count ||
+ j_blocks > fs->super->s_blocks_count / 2) {
+ fputs(("\nJournal size too big for filesystem.\n"),
+ stderr);
+ return -EFBIG;
+ }
+ return j_blocks;
+ }
+
+ if (fs->super->s_blocks_count < 32768)
+ j_blocks = 1024;
+ else if (fs->super->s_blocks_count < 256*1024)
+ j_blocks = 4096;
+ else if (fs->super->s_blocks_count < 512*1024)
+ j_blocks = 8192;
+ else if (fs->super->s_blocks_count < 1024*1024)
+ j_blocks = 16384;
+ else
+ j_blocks = 32768;
+
+ return j_blocks;
+}
+
+/*
* This function adds a journal inode to a filesystem, using either
* POSIX routines if the filesystem is mounted, or using direct I/O
* functions if it is not.
Index: e2fsprogs-1.39/misc/mke2fs.c
===================================================================
--- e2fsprogs-1.39.orig/misc/mke2fs.c
+++ e2fsprogs-1.39/misc/mke2fs.c
@@ -1649,8 +1649,10 @@ int main (int argc, char *argv[])
} else if ((journal_size) ||
(fs_param.s_feature_compat &
EXT3_FEATURE_COMPAT_HAS_JOURNAL)) {
- journal_blocks = figure_journal_size(journal_size, fs);
+ journal_blocks = ext2fs_figure_journal_size(journal_size, fs);

+ if (journal_blocks < 0)
+ exit(1);
if (!journal_blocks) {
fs->super->s_feature_compat &=
~EXT3_FEATURE_COMPAT_HAS_JOURNAL;
Index: e2fsprogs-1.39/misc/tune2fs.c
===================================================================
--- e2fsprogs-1.39.orig/misc/tune2fs.c
+++ e2fsprogs-1.39/misc/tune2fs.c
@@ -417,8 +417,10 @@ static void add_journal(ext2_filsys fs)
} else if (journal_size) {
fputs(_("Creating journal inode: "), stdout);
fflush(stdout);
- journal_blocks = figure_journal_size(journal_size, fs);
+ journal_blocks = ext2fs_figure_journal_size(journal_size, fs);

+ if (journal_blocks < 0)
+ exit(1);
retval = ext2fs_add_journal_inode(fs, journal_blocks,
journal_flags);
if (retval) {
Index: e2fsprogs-1.39/misc/util.c
===================================================================
--- e2fsprogs-1.39.orig/misc/util.c
+++ e2fsprogs-1.39/misc/util.c
@@ -238,57 +238,6 @@ void parse_journal_opts(const char *opts
}
}

-/*
- * Determine the number of journal blocks to use, either via
- * user-specified # of megabytes, or via some intelligently selected
- * defaults.
- *
- * Find a reasonable journal file size (in blocks) given the number of blocks
- * in the filesystem. For very small filesystems, it is not reasonable to
- * have a journal that fills more than half of the filesystem.
- */
-int figure_journal_size(int size, ext2_filsys fs)
-{
- blk_t j_blocks;
-
- if (fs->super->s_blocks_count < 2048) {
- fputs(_("\nFilesystem too small for a journal\n"), stderr);
- return 0;
- }
-
- if (size > 0) {
- j_blocks = size * 1024 / (fs->blocksize / 1024);
- if (j_blocks < 1024 || j_blocks > 102400) {
- fprintf(stderr, _("\nThe requested journal "
- "size is %d blocks; it must be\n"
- "between 1024 and 102400 blocks. "
- "Aborting.\n"),
- j_blocks);
- exit(1);
- }
- if (j_blocks > fs->super->s_free_blocks_count) {
- fputs(_("\nJournal size too big for filesystem.\n"),
- stderr);
- exit(1);
- }
- return j_blocks;
- }
-
- if (fs->super->s_blocks_count < 32768)
- j_blocks = 1024;
- else if (fs->super->s_blocks_count < 256*1024)
- j_blocks = 4096;
- else if (fs->super->s_blocks_count < 512*1024)
- j_blocks = 8192;
- else if (fs->super->s_blocks_count < 1024*1024)
- j_blocks = 16384;
- else
- j_blocks = 32768;
-
-
- return j_blocks;
-}
-
void print_check_message(ext2_filsys fs)
{
printf(_("This filesystem will be automatically "
Index: e2fsprogs-1.39/misc/util.h
===================================================================
--- e2fsprogs-1.39.orig/misc/util.h
+++ e2fsprogs-1.39/misc/util.h
@@ -22,5 +22,4 @@ extern void proceed_question(void);
extern void check_plausibility(const char *device);
extern void parse_journal_opts(const char *opts);
extern void check_mount(const char *device, int force, const char *type);
-extern int figure_journal_size(int size, ext2_filsys fs);
extern void print_check_message(ext2_filsys fs);
Index: e2fsprogs-1.39/e2fsck/problem.c
===================================================================
--- e2fsprogs-1.39.orig/e2fsck/problem.c
+++ e2fsprogs-1.39/e2fsck/problem.c
@@ -1479,6 +1479,11 @@ static struct e2fsck_problem problem_tab
" +(%i--%j)",
PROMPT_NONE, PR_LATCH_IBITMAP | PR_PREEN_OK | PR_PREEN_NOMSG },

+ /* Recreate journal if E2F_FLAG_JOURNAL_INODE flag is set */
+ { PR_6_RECREATE_JOURNAL,
+ N_("Recreate journal to make the filesystem ext3 again?\n"),
+ PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },
+
{ 0 }
};

Index: e2fsprogs-1.39/e2fsck/problem.h
===================================================================
--- e2fsprogs-1.39.orig/e2fsck/problem.h
+++ e2fsprogs-1.39/e2fsck/problem.h
@@ -892,6 +892,13 @@ struct problem_context {
#define PR_5_INODE_RANGE_USED 0x050017

/*
+ * Post-Pass 5 errors
+ */
+
+/* Recreate the journal if E2F_FLAG_JOURNAL_INODE flag is set */
+#define PR_6_RECREATE_JOURNAL 0x060000
+
+/*
* Function declarations
*/
int fix_problem(e2fsck_t ctx, problem_t code, struct problem_context *pctx);


Thanks,
Kalpak.


Attachments:
e2fsprogs-tests-f_badjour_indblks.patch (14.85 kB)

2007-05-08 05:40:44

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] e2fsprogs: Check journal inode sanity and recreate journal

On Thu, Mar 15, 2007 at 02:43:47AM +0530, Kalpak Shah wrote:
> Index: e2fsprogs-1.39/lib/ext2fs/mkjournal.c
> ===================================================================
> --- e2fsprogs-1.39.orig/lib/ext2fs/mkjournal.c
> +++ e2fsprogs-1.39/lib/ext2fs/mkjournal.c
> + if (fs->super->s_blocks_count < 2048) {
> + fputs(("\nFilesystem too small for a journal\n"), stderr);
> + return 0;
> + }

Code in lib/ext2fs isn't allowed to do any output to stdio (except for
debugging purposes). It causes internationalization problems, and
it's just in general a bad idea for ext2fs library code to try to do
any UI.

- Ted

2007-05-24 21:19:01

by Kalpak Shah

[permalink] [raw]
Subject: Re: [PATCH] e2fsprogs: Check journal inode sanity and recreate journal

On Tue, 2007-05-08 at 01:40 -0400, Theodore Tso wrote:
> On Thu, Mar 15, 2007 at 02:43:47AM +0530, Kalpak Shah wrote:
> > Index: e2fsprogs-1.39/lib/ext2fs/mkjournal.c
> > ===================================================================
> > --- e2fsprogs-1.39.orig/lib/ext2fs/mkjournal.c
> > +++ e2fsprogs-1.39/lib/ext2fs/mkjournal.c
> > + if (fs->super->s_blocks_count < 2048) {
> > + fputs(("\nFilesystem too small for a journal\n"), stderr);
> > + return 0;
> > + }
>
> Code in lib/ext2fs isn't allowed to do any output to stdio (except for
> debugging purposes). It causes internationalization problems, and
> it's just in general a bad idea for ext2fs library code to try to do
> any UI.
>
> - Ted

Hi Ted,

I have reworked the patch to make sure that lib/ext2fs does not do any
output. Added that to my mistakes-not-to-be-made-again list.

Signed-off-by: Kalpak Shah <[email protected]>
Signed-off-by: Andreas Dilger <[email protected]>

Thanks,
Kalpak.


Attachments:
recreate-journal.patch (11.38 kB)