| Message ID | 20180808115256.15885-1-lczerner@redhat.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | e2fsck: remove resize inode if both resize_inode and meta_bg are enabled | expand |
On Aug 8, 2018, at 5:52 AM, Lukas Czerner <lczerner@redhat.com> wrote: > > Previous e2fsprogs versions allowed to create a file system with both > resize_inode and meta_bg enabled. This was fixed by upstream commit > 42e77d5d ("libext2fs: don't create filesystems with meta_bg and resize_inode") > > However e2fsck still does not recognize the conflict and will attempt to > clear and recreate resize_inode if it's corrupted due to this incompatible > feature combination, though it will create it in the same wrong layout. > > Fix it by teaching e2fsck to recognize resize_inode and meta_bg > conflict and fixing it by disabling and clearing resize inode. > > Signed-off-by: Lukas Czerner <lczerner@redhat.com> Reviewed-by: Andreas Dilger <adilger@dilger.ca> > --- > e2fsck/problem.c | 6 ++++++ > e2fsck/problem.h | 3 +++ > e2fsck/super.c | 8 ++++++++ > 3 files changed, 17 insertions(+) > > diff --git a/e2fsck/problem.c b/e2fsck/problem.c > index a0a3cfec..e439539e 100644 > --- a/e2fsck/problem.c > +++ b/e2fsck/problem.c > @@ -339,6 +339,12 @@ static struct e2fsck_problem problem_table[] = { > N_("Resize @i not valid. "), > PROMPT_RECREATE, 0 }, > > + /* Meta_bg and resize_inode are not compatible, disable resize_inode*/ > + { PR_0_DISABLE_RESIZE_INODE, > + N_("Resize_@i and meta_bg features are enabled. Those features are\n" > + "not compatible. Resize @i should be disabled. "), > + PROMPT_FIX, 0 }, > + > /* Superblock last mount time is in the future */ > { PR_0_FUTURE_SB_LAST_MOUNT, > N_("@S last mount time (%t,\n\tnow = %T) is in the future.\n"), > diff --git a/e2fsck/problem.h b/e2fsck/problem.h > index 7db122ab..2c79169e 100644 > --- a/e2fsck/problem.h > +++ b/e2fsck/problem.h > @@ -285,6 +285,9 @@ struct problem_context { > /* Inode count in the superblock incorrect */ > #define PR_0_INODE_COUNT_BIG 0x000050 > > +/* Meta_bg and resize_inode are not compatible, remove resize_inode*/ > +#define PR_0_DISABLE_RESIZE_INODE 0x000051 > + > /* > * Pass 1 errors > */ > diff --git a/e2fsck/super.c b/e2fsck/super.c > index eb7ab0d1..e5932be6 100644 > --- a/e2fsck/super.c > +++ b/e2fsck/super.c > @@ -436,6 +436,14 @@ void check_resize_inode(e2fsck_t ctx) > > clear_problem_context(&pctx); > > + if (ext2fs_has_feature_resize_inode(fs->super) && > + ext2fs_has_feature_meta_bg(fs->super) && > + fix_problem(ctx, PR_0_DISABLE_RESIZE_INODE, &pctx)) { > + ext2fs_clear_feature_resize_inode(fs->super); > + fs->super->s_reserved_gdt_blocks = 0; > + ext2fs_mark_super_dirty(fs); > + } > + > /* > * If the resize inode feature isn't set, then > * s_reserved_gdt_blocks must be zero. > -- > 2.17.1 > Cheers, Andreas
On Wed, Aug 08, 2018 at 01:52:56PM +0200, Lukas Czerner wrote: > Previous e2fsprogs versions allowed to create a file system with both > resize_inode and meta_bg enabled. This was fixed by upstream commit > 42e77d5d ("libext2fs: don't create filesystems with meta_bg and resize_inode") > > However e2fsck still does not recognize the conflict and will attempt to > clear and recreate resize_inode if it's corrupted due to this incompatible > feature combination, though it will create it in the same wrong layout. > > Fix it by teaching e2fsck to recognize resize_inode and meta_bg > conflict and fixing it by disabling and clearing resize inode. > > Signed-off-by: Lukas Czerner <lczerner@redhat.com> I can tell that you didn't run "make check" before sending the commit out; because if you did, you would have noticed this: *** Unordered problem table: curr code = 0x00000031: @S last mount time (%t, now = %T) is in the future. *** prev code = 0x00000051: Resize_@i and meta_bg features are enabled. Those features are not compatible. Resize @i should be disabled. *** This is a programming error in e2fsck make: *** [Makefile:469: check] Error 1o I fixed this up, and ran a quick test case, so I'll apply it, but it would be great if you could create a regression test case. Thanks! - Ted
On Sat, Aug 11, 2018 at 07:04:30PM -0400, Theodore Y. Ts'o wrote: > On Wed, Aug 08, 2018 at 01:52:56PM +0200, Lukas Czerner wrote: > > Previous e2fsprogs versions allowed to create a file system with both > > resize_inode and meta_bg enabled. This was fixed by upstream commit > > 42e77d5d ("libext2fs: don't create filesystems with meta_bg and resize_inode") > > > > However e2fsck still does not recognize the conflict and will attempt to > > clear and recreate resize_inode if it's corrupted due to this incompatible > > feature combination, though it will create it in the same wrong layout. > > > > Fix it by teaching e2fsck to recognize resize_inode and meta_bg > > conflict and fixing it by disabling and clearing resize inode. > > > > Signed-off-by: Lukas Czerner <lczerner@redhat.com> > > I can tell that you didn't run "make check" before sending the commit > out; because if you did, you would have noticed this: Guitly as charged :) > > *** Unordered problem table: > curr code = 0x00000031: @S last mount time (%t, > now = %T) is in the future. > > *** prev code = 0x00000051: Resize_@i and meta_bg features are enabled. Those features are > not compatible. Resize @i should be disabled. > *** This is a programming error in e2fsck > make: *** [Makefile:469: check] Error 1o > > I fixed this up, and ran a quick test case, so I'll apply it, but it > would be great if you could create a regression test case. Thanks! Yeah, I was considering it, then I decided it was probably not worth it. But sure, I can create one. -Lukas > > Thanks! > > - Ted
diff --git a/e2fsck/problem.c b/e2fsck/problem.c index a0a3cfec..e439539e 100644 --- a/e2fsck/problem.c +++ b/e2fsck/problem.c @@ -339,6 +339,12 @@ static struct e2fsck_problem problem_table[] = { N_("Resize @i not valid. "), PROMPT_RECREATE, 0 }, + /* Meta_bg and resize_inode are not compatible, disable resize_inode*/ + { PR_0_DISABLE_RESIZE_INODE, + N_("Resize_@i and meta_bg features are enabled. Those features are\n" + "not compatible. Resize @i should be disabled. "), + PROMPT_FIX, 0 }, + /* Superblock last mount time is in the future */ { PR_0_FUTURE_SB_LAST_MOUNT, N_("@S last mount time (%t,\n\tnow = %T) is in the future.\n"), diff --git a/e2fsck/problem.h b/e2fsck/problem.h index 7db122ab..2c79169e 100644 --- a/e2fsck/problem.h +++ b/e2fsck/problem.h @@ -285,6 +285,9 @@ struct problem_context { /* Inode count in the superblock incorrect */ #define PR_0_INODE_COUNT_BIG 0x000050 +/* Meta_bg and resize_inode are not compatible, remove resize_inode*/ +#define PR_0_DISABLE_RESIZE_INODE 0x000051 + /* * Pass 1 errors */ diff --git a/e2fsck/super.c b/e2fsck/super.c index eb7ab0d1..e5932be6 100644 --- a/e2fsck/super.c +++ b/e2fsck/super.c @@ -436,6 +436,14 @@ void check_resize_inode(e2fsck_t ctx) clear_problem_context(&pctx); + if (ext2fs_has_feature_resize_inode(fs->super) && + ext2fs_has_feature_meta_bg(fs->super) && + fix_problem(ctx, PR_0_DISABLE_RESIZE_INODE, &pctx)) { + ext2fs_clear_feature_resize_inode(fs->super); + fs->super->s_reserved_gdt_blocks = 0; + ext2fs_mark_super_dirty(fs); + } + /* * If the resize inode feature isn't set, then * s_reserved_gdt_blocks must be zero.
Previous e2fsprogs versions allowed to create a file system with both resize_inode and meta_bg enabled. This was fixed by upstream commit 42e77d5d ("libext2fs: don't create filesystems with meta_bg and resize_inode") However e2fsck still does not recognize the conflict and will attempt to clear and recreate resize_inode if it's corrupted due to this incompatible feature combination, though it will create it in the same wrong layout. Fix it by teaching e2fsck to recognize resize_inode and meta_bg conflict and fixing it by disabling and clearing resize inode. Signed-off-by: Lukas Czerner <lczerner@redhat.com> --- e2fsck/problem.c | 6 ++++++ e2fsck/problem.h | 3 +++ e2fsck/super.c | 8 ++++++++ 3 files changed, 17 insertions(+)