On Mon, Dec 19, 2011 at 11:28:24AM +0100, Andreas Dilger wrote:

On 2011-12-14, at 2:14 AM, Darrick J. Wong wrote:

quoted

Detect mismatches of the inode and checksum, and prompt the user to fix the
situation.

@@ -739,6 +740,12 @@ void e2fsck_pass1(e2fsck_t ctx)

		pctx.ino = ino;
		pctx.inode = inode;
		ctx->stashed_ino = ino;
+
+		/* Clear corrupt inode */
+		if (pctx.errcode == EXT2_ET_INODE_CSUM_INVALID &&
+		    fix_problem(ctx, PR_1_INODE_CSUM_INVALID, &pctx))
+			goto clear_inode;

If the user enters "n" here to clear the inode, does it make sense to
also ask if they want the checksum be corrected?  If this is also "n"
(as is the case with "e2fsck -n") then nothing is done, but in some
cases it makes sense to allow the user to keep the inode rather than
only having the option to erase it.

I was thinking something like this might work:

if (csum_invalid)
	ask and then jump to clear_inode;

...all other inode sanity checks go here...

if (csum_was_invalid)
	ask and correct inode checksum;

That way, if the inode manages to survive all the other sanity checks, the user
has the opportunity to save the inode.  If the inode contains junk, then the
user will probably clear the inode on account of the garbage.

Come to think of it, this is probably a good idea for everything else.  I
think?

quoted

diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index f042b89..89dc72b 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c

@@ -934,7 +934,12 @@ static struct e2fsck_problem problem_table[] = {
+	/* inode checksum does not match inode */
+	{ PR_1_INODE_CSUM_INVALID,
+	  N_("@i %i checksum does not match @i.  "),
+	  PROMPT_FIX, PR_PREEN_OK },

Also, "PROMPT_FIX" is misleading if the "fix" is to erase the inode.
It should instead be "PROMPT_CLEAR_INODE".

Ok.

--D

Cheers, Andreas