[Ocfs2-tools-devel] [patch 1/7] Adds some macros in the header files.

[Mark Fasheh]

On Tue, Jun 12, 2007 at 02:43:34PM +0800, tao.ma at oracle.com wrote:
> Add macro "OCFS2_FEATURE_INCOMPAT_LOCAL_MOUNT" in ocfs2_fs.h to protect users
> from inadvertently mounting the fs after an aborted run without fsck-ing.
> 
> Index: new.ocfs2-tools/libocfs2/include/ocfs2.h
> ===================================================================
> --- new.ocfs2-tools.orig/libocfs2/include/ocfs2.h	2007-06-11 13:51:55.000000000 -0400
> +++ new.ocfs2-tools/libocfs2/include/ocfs2.h	2007-06-11 14:11:51.000000000 -0400
> @@ -76,7 +76,8 @@
>  #define OCFS2_LIB_FEATURE_INCOMPAT_SUPP		(OCFS2_FEATURE_INCOMPAT_SUPP | \
>  						 OCFS2_FEATURE_INCOMPAT_HEARTBEAT_DEV | \
>  						 OCFS2_FEATURE_INCOMPAT_RESIZE_INPROG | \
> -						 OCFS2_FEATURE_INCOMPAT_LOCAL_MOUNT)
> +						 OCFS2_FEATURE_INCOMPAT_LOCAL_MOUNT   | \
> +						 OCFS2_FEATURE_INCOMPAT_REMOVE_SLOT_INPROG)
>  
>  #define OCFS2_LIB_FEATURE_RO_COMPAT_SUPP	OCFS2_FEATURE_RO_COMPAT_SUPP
>  
> Index: new.ocfs2-tools/libocfs2/include/ocfs2_fs.h
> ===================================================================
> --- new.ocfs2-tools.orig/libocfs2/include/ocfs2_fs.h	2007-06-11 13:51:55.000000000 -0400
> +++ new.ocfs2-tools/libocfs2/include/ocfs2_fs.h	2007-06-11 14:11:51.000000000 -0400
> @@ -109,6 +109,12 @@
>  /* Support for sparse allocation in b-trees */
>  #define OCFS2_FEATURE_INCOMPAT_SPARSE_ALLOC	0x0010
>  
> +/* tunefs sets this incompat flag before starting slot remove and clears it
> + * at the end. This flag protects users from inadvertently mounting the fs
> + * after an aborted run without fsck-ing.
> + */
> +#define OCFS2_FEATURE_INCOMPAT_REMOVE_SLOT_INPROG	0x0020

How many more tunefs-specific feature incompat flags are we going to need?

The tunefs flags are starting to pollute the incompat namespace, which wasn't designed
with this sort of use in mind. Then when we get the ability to make these
changes without an incompat flag, they'll just sit there...

We can do better, and help the user at the same time. Please just make this
into something like:

#define OCFS2_FEATURE_INCOMPAT_TUNEFS_INPROG    0x0020

and allocate a small field from the free space on our superblock to flag
exactly which tunefs operation is in progress.

This way we preserve the rest of the INCOMPAT bits for actual _file_
_system_ _features_ and tunefs.ocfs2 can put as rich a set of information in
it's little area as needed.

A nice side effect is that the kernel module knows whether an incompat is a
tunefs crash or not from now until forever. With that knowledge, we can even
print a nice little message for the poor user who tried to mount after a
failed tunefs.ocfs2:

if (osb->s_feature_incompat & (OCFS2_FEATURE_INCOMPAT_TUNEFS_INPROG|OCFS2_FEATURE_INCOMPAT_RESIZE_INPROG))
	printk("Unable to mount because of a failed tunefs. Please run tunefs.ocfs2 or fsck.ocfs2 to recover the volume.\n");

or some such message. This is much more informative (and probably less
alarming) than:

"couldn't mount because of unsupported optional features (0x0020)"

Which is what the user would be seeing if we keep piling on incompat flags
for tunefs.

Surely, we already lost some of this by having OCFS2_FEATURE_INCOMPAT_RESIZE_INPROG
in the 1st place, but there's zero reason to continue down this path.
	--Mark

--
Mark Fasheh
Senior Software Developer, Oracle
mark.fasheh at oracle.com