Re: [PATCH][10/28] e2fsprogs-uninit.patch

[Theodore Tso <tytso@mit.edu>]

On Sat, Feb 02, 2008 at 01:34:44AM -0700, Andreas Dilger wrote:
> Index: e2fsprogs-1.40.5/e2fsck/super.c
> ===================================================================
> @@ -626,6 +631,50 @@ void check_super_block(e2fsck_t ctx)
	...
> +		if (!ext2fs_group_desc_csum_verify(sb, i, gd)) {
> +			if (fix_problem(ctx, PR_0_GDT_CSUM, &pctx)) {
> +				gd->bg_flags &=	~(EXT2_BG_BLOCK_UNINIT |
> +				                  EXT2_BG_INODE_UNINIT);
> +				gd->bg_itable_unused = 0;
> +			}
> +			ext2fs_unmark_valid(fs);
> +		}
> +
	...
> +
> +		gd->bg_checksum = ext2fs_group_desc_csum(fs->super, i, gd);

This last looks horribly wrong.  check_super_block() is merely
supposed to check to see if the superblock and block gorup descriptos
looks OK, and to mark the filesystem as invalid if anything looks
insane.  It should *not* modifying the block group descriptor, and it
certainly should not be doing so without first checking to see if the
filesystem has been opened read/only or calling
ext2fs_mark_super_dirty(fs) after making a change (that it shouldn't
do).

In fact there already is a check to see if the checksum has verified
correctly (see the first part of the patch which I quoted), so I think
the best thing to do is to remove that last bit.  I'm not sure why
it's there at all, in fact....

       	       	    	     	      	   - Ted