[PATCH] allow tune2fs to set/clear resize_inode

[PATCH] allow tune2fs to set/clear resize_inode

[Eric Sandeen]

(this patch has been carried in Red Hat / Fedora rpms for a while, not 
sure why it never got sent upstream... clearing this allows for mounting 
filesystems with a resize_inode on older systems)

-----------------

Allow tune2fs to set & clear resize_inode; requires fsck afterwards.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Addresses-Red-Hat-Bugzilla: #167816

Index: e2fsprogs-git/misc/tune2fs.c
===================================================================
--- e2fsprogs-git.orig/misc/tune2fs.c
+++ e2fsprogs-git/misc/tune2fs.c
@@ -96,6 +96,7 @@ static void usage(void)
 
 static __u32 ok_features[3] = {
 	EXT3_FEATURE_COMPAT_HAS_JOURNAL |
+		EXT2_FEATURE_COMPAT_RESIZE_INODE |
 		EXT2_FEATURE_COMPAT_DIR_INDEX,	/* Compat */
 	EXT2_FEATURE_INCOMPAT_FILETYPE,		/* Incompat */
 	EXT2_FEATURE_RO_COMPAT_SPARSE_SUPER	/* R/O compat */
@@ -283,6 +284,7 @@ static void update_feature_set(ext2_fils
 {
 	int sparse, old_sparse, filetype, old_filetype;
 	int journal, old_journal, dxdir, old_dxdir;
+	int resize_inode, old_resize_inode;
 	struct ext2_super_block *sb= fs->super;
 	__u32	old_compat, old_incompat, old_ro_compat;
 
@@ -298,6 +300,8 @@ static void update_feature_set(ext2_fils
 		EXT3_FEATURE_COMPAT_HAS_JOURNAL;
 	old_dxdir = sb->s_feature_compat &
 		EXT2_FEATURE_COMPAT_DIR_INDEX;
+	old_resize_inode = sb->s_feature_compat &
+		EXT2_FEATURE_COMPAT_RESIZE_INODE;
 	if (e2p_edit_feature(features, &sb->s_feature_compat,
 			     ok_features)) {
 		fprintf(stderr, _("Invalid filesystem option set: %s\n"),
@@ -312,6 +316,8 @@ static void update_feature_set(ext2_fils
 		EXT3_FEATURE_COMPAT_HAS_JOURNAL;
 	dxdir = sb->s_feature_compat &
 		EXT2_FEATURE_COMPAT_DIR_INDEX;
+	resize_inode = sb->s_feature_compat &
+		EXT2_FEATURE_COMPAT_RESIZE_INODE;
 	if (old_journal && !journal) {
 		if ((mount_flags & EXT2_MF_MOUNTED) &&
 		    !(mount_flags & EXT2_MF_READONLY)) {
@@ -358,7 +364,8 @@ static void update_feature_set(ext2_fils
 	     sb->s_feature_incompat))
 		ext2fs_update_dynamic_rev(fs);
 	if ((sparse != old_sparse) ||
-	    (filetype != old_filetype)) {
+	    (filetype != old_filetype) ||
+	    (resize_inode != old_resize_inode)) {
 		sb->s_state &= ~EXT2_VALID_FS;
 		printf("\n%s\n", _(please_fsck));
 	}
Index: e2fsprogs-git/misc/tune2fs.8.in
===================================================================
--- e2fsprogs-git.orig/misc/tune2fs.8.in
+++ e2fsprogs-git/misc/tune2fs.8.in
@@ -392,12 +392,16 @@ option.
 .TP
 .B sparse_super
 Limit the number of backup superblocks to save space on large filesystems.
+.TP
+.B resize_inode
+Reserve space so the block group descriptor table may grow in the future.
 .RE
 .IP
 After setting or clearing 
-.B sparse_super
-and 
-.B filetype 
+.BR sparse_super ,
+.BR filetype ,
+or
+.B resize_inode
 filesystem features,
 .BR e2fsck (8)
 must be run on the filesystem to return the filesystem to a consistent state.

Re: [PATCH] allow tune2fs to set/clear resize_inode

[Andreas Dilger]

On Nov 05, 2007  14:45 -0600, Eric Sandeen wrote:
> (this patch has been carried in Red Hat / Fedora rpms for a while, not 
> sure why it never got sent upstream... clearing this allows for mounting 
> filesystems with a resize_inode on older systems)
>
> Allow tune2fs to set & clear resize_inode; requires fsck afterwards.

Two things that are a bit confusing:
- since RESIZE_INODE is a COMPAT feature, I don't see how this affects
  mounting the filesystem on older systems?  Is there a bug somewhere?
- why not use something like remove_journal_inode() (or create a new
  helper function like ext2fs_unlink(), and move the kill_file_by_inode()
  and release_blocks_proc() into lib/ext2fs as ext2fs_delete_inode())
  to avoid the need for an e2fsck?

>  	if ((sparse != old_sparse) ||
> -	    (filetype != old_filetype)) {
> +	    (filetype != old_filetype) ||
> +	    (resize_inode != old_resize_inode)) {
>  		sb->s_state &= ~EXT2_VALID_FS;
>  		printf("\n%s\n", _(please_fsck));
>  	}

I don't know that it is so easy to enable RESIZE_INODE on an existing
filesystem as just setting the feature flag and running e2fsck?  The
reserved group descriptor blocks will potentially conflict with the
bitmaps and inode tables.

What is needed is an ext2prepare-like step that involves resize2fs code
to move the file/dir blocks and then the move inode table, as if the
filesystem were going to be resized to the new maximum resize limit,
and then create the resize inode but do not actually add new blocks/groups
at the end of the filesystem.

Cheers, Andreas
--
Andreas Dilger
Sr. Software Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: [PATCH] allow tune2fs to set/clear resize_inode

[Eric Sandeen]

Andreas Dilger wrote:
> On Nov 05, 2007  14:45 -0600, Eric Sandeen wrote:
>> (this patch has been carried in Red Hat / Fedora rpms for a while, not 
>> sure why it never got sent upstream... clearing this allows for mounting 
>> filesystems with a resize_inode on older systems)
>>
>> Allow tune2fs to set & clear resize_inode; requires fsck afterwards.
> 
> Two things that are a bit confusing:
> - since RESIZE_INODE is a COMPAT feature, I don't see how this affects
>   mounting the filesystem on older systems?  Is there a bug somewhere?

Hmm there is a RHEL3 bug, long since closed with a patch like this one
(actually less fleshed out)... but you're right, this should work.

(quick test)

...and it does.  Grrr, sorry, assumed the patch was there for a reason!

I'll retract this patch, I don't think it's needed.  Sorry for the noise.

-Eric

Re: [PATCH] allow tune2fs to set/clear resize_inode

[Theodore Tso]

On Tue, Nov 06, 2007 at 09:12:55AM +0800, Andreas Dilger wrote:
> I don't know that it is so easy to enable RESIZE_INODE on an existing
> filesystem as just setting the feature flag and running e2fsck?  The
> reserved group descriptor blocks will potentially conflict with the
> bitmaps and inode tables.

Yes, it isn't so easy, and yup, it will potentially conflict with the
bitmap and inode tables --- and this is something which e2fsck does
*not* handle well.  

> What is needed is an ext2prepare-like step that involves resize2fs code
> to move the file/dir blocks and then the move inode table, as if the
> filesystem were going to be resized to the new maximum resize limit,
> and then create the resize inode but do not actually add new blocks/groups
> at the end of the filesystem.

Yeah, the plan was to eventually add ext2prepare-like code into
tune2fs, using the undo I/O manager for safety.  But that's been
relatively low priority.

BTW, I've gotten ~2 bug reports from Debian users claiming that
ext2prepare had trashed their filesystem.  I don't have any clean
evidence about whether it was a userspace error or some kind of bug in
ext2prepare, possibly conflicting with some new ext3 feature that
we've since added that ext2prepare doesn't properly account for
(extended attributes, maybe?).  

I have not had time to look into it, but thought has crossed my mind
that a quick hack would be to splice the undo manager into
ext2prepare, have it run e2fsck, and if it fails, do a rollback,
create an e2image file, and then instruct the user to send in a bug
report.  :-)

							- Ted

Re: [PATCH] allow tune2fs to set/clear resize_inode

[Andreas Dilger]

On Nov 06, 2007  13:51 -0500, Theodore Tso wrote:
> On Tue, Nov 06, 2007 at 09:12:55AM +0800, Andreas Dilger wrote:
> > What is needed is an ext2prepare-like step that involves resize2fs code
> > to move the file/dir blocks and then the move inode table, as if the
> > filesystem were going to be resized to the new maximum resize limit,
> > and then create the resize inode but do not actually add new blocks/groups
> > at the end of the filesystem.
> 
> Yeah, the plan was to eventually add ext2prepare-like code into
> tune2fs, using the undo I/O manager for safety.  But that's been
> relatively low priority.
> 
> BTW, I've gotten ~2 bug reports from Debian users claiming that
> ext2prepare had trashed their filesystem.  I don't have any clean
> evidence about whether it was a userspace error or some kind of bug in
> ext2prepare, possibly conflicting with some new ext3 feature that
> we've since added that ext2prepare doesn't properly account for
> (extended attributes, maybe?).  
> 
> I have not had time to look into it, but thought has crossed my mind
> that a quick hack would be to splice the undo manager into
> ext2prepare, have it run e2fsck, and if it fails, do a rollback,
> create an e2image file, and then instruct the user to send in a bug
> report.  :-)

I don't think it would be very easy to splice undo manager into ext2prepare.
I'd rather see time spent to make resize2fs handle the prepare functionality
and then ext2resize can be entirely obsoleted.

Aneesh, adding undo manager to resize2fs would be an excellent use of that
library, and going from resize2fs to "resize2fs --prepare-only" (or whatever)
would be trivial I think.  Is that something you'd be interested to work on?

Cheers, Andreas
--
Andreas Dilger
Sr. Software Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: [PATCH] allow tune2fs to set/clear resize_inode

[Theodore Tso]

On Mon, Nov 05, 2007 at 02:45:41PM -0600, Eric Sandeen wrote:
> (this patch has been carried in Red Hat / Fedora rpms for a while, not 
> sure why it never got sent upstream... clearing this allows for mounting 
> filesystems with a resize_inode on older systems)

I just checked in the following into the e2fsprogs maint branch.

       	       	      		     - Ted

commit 037914e28f69110020195fe9f9ea56e5274ff2c0
Author: Theodore Ts'o <tytso@mit.edu>
Date:   Tue Feb 26 17:31:06 2008 -0500

    tune2fs: Add support to clear the resize_inode feature
    
    This requires an fsck aftwards.  We don't allow setting the
    resize_inode feature because extensive work to tune2fs or e2fsck to
    safely relocate blocks is necessary in order to reserve the blocks
    needed by the resize inode.
    
    Addresses-Red-Hat-Bugzilla: #167816
    
    Signed-off-by: Eric Sandeen <sandeen@redhat.com>
    Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

diff --git a/misc/tune2fs.8.in b/misc/tune2fs.8.in
index 435556f..40cdb96 100644
--- a/misc/tune2fs.8.in
+++ b/misc/tune2fs.8.in
@@ -429,12 +429,19 @@ option.
 .TP
 .B sparse_super
 Limit the number of backup superblocks to save space on large filesystems.
+.TP
+.B resize_inode
+Reserve space so the block group descriptor table may grow in the
+future.
+.B Tune2fs 
+only supports clearing this filesystem feature.
 .RE
 .IP
 After setting or clearing 
-.B sparse_super
-and 
-.B filetype 
+.BR sparse_super ,
+.BR filetype ,
+or
+.B resize_inode
 filesystem features,
 .BR e2fsck (8)
 must be run on the filesystem to return the filesystem to a consistent state.
diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index ee7af4b..4529e24 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -116,6 +116,7 @@ static __u32 ok_features[3] = {
 
 static __u32 clear_ok_features[3] = {
 	EXT3_FEATURE_COMPAT_HAS_JOURNAL |
+		EXT2_FEATURE_COMPAT_RESIZE_INODE |
 		EXT2_FEATURE_COMPAT_DIR_INDEX,	/* Compat */
 	EXT2_FEATURE_INCOMPAT_FILETYPE,		/* Incompat */
 	EXT2_FEATURE_RO_COMPAT_SPARSE_SUPER	/* R/O compat */
@@ -389,7 +390,9 @@ static void update_feature_set(ext2_filsys fs, char *features)
 	if (FEATURE_CHANGED(E2P_FEATURE_RO_INCOMPAT,
 			    EXT2_FEATURE_RO_COMPAT_SPARSE_SUPER) ||
 	    FEATURE_CHANGED(E2P_FEATURE_INCOMPAT,
-			    EXT2_FEATURE_INCOMPAT_FILETYPE)) {
+			    EXT2_FEATURE_INCOMPAT_FILETYPE) ||
+	    FEATURE_CHANGED(E2P_FEATURE_COMPAT,
+			    EXT2_FEATURE_COMPAT_RESIZE_INODE)) {
 		sb->s_state &= ~EXT2_VALID_FS;
 		printf("\n%s\n", _(please_fsck));
 	}

Re: [PATCH] allow tune2fs to set/clear resize_inode

[Eric Sandeen]

Theodore Tso wrote:
> On Mon, Nov 05, 2007 at 02:45:41PM -0600, Eric Sandeen wrote:
>> (this patch has been carried in Red Hat / Fedora rpms for a while, not 
>> sure why it never got sent upstream... clearing this allows for mounting 
>> filesystems with a resize_inode on older systems)
> 
> I just checked in the following into the e2fsprogs maint branch.

Hm, I had actually retracted this patch because as a COMPAT feature it
should have worked, right?  But, I suppose it's harmless. :)

Thanks,
-Eric