[PATCH v2 1/5] xfs_repair: refuse to run if we don't recognize version or feature flags

[Fanael Linithien]

On 2015-05-27 17:27, Eric Sandeen wrote:
>
> I wonder if something like:
>
> 	/* Look for V5 feature flags we don't know about */
> 	if (XFS_SB_VERSION_NUM(sb) >= XFS_SB_VERSION_5 &&
> 	    (xfs_sb_has_ro_compat_feature(sb, XFS_SB_FEAT_RO_COMPAT_UNKNOWN) ||
> 	     xfs_sb_has_incompat_feature(sb, XFS_SB_FEAT_INCOMPAT_UNKNOWN) ||
> 	     xfs_sb_has_compat_feature(sb, XFS_SB_FEAT_COMPAT_UNKNOWN))) {
> 		printf("unknown feature flags 0x%x/0x%x/0x%x\n",
> 			sb->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_UNKNOWN,
> 			sb->sb_features_incompat & XFS_SB_FEAT_INCOMPAT_UNKNOWN,
> 			sb->sb_features_compat & XFS_SB_FEAT_COMPAT_UNKNOWN);
> 		...
>
> would suffice, given that the user will need to read code to unerstand the hex
> values, anyway.
> 	
> Hm, and as Fanael also pointed out, "compat" features ... should be ok, right,
> and can be removed from the exclusions?

I'm not entirely sure silently ignoring unknown compat features in 
xfs_repair is a good idea. Consider this ext2 example: xattr support is 
a compat flag. It's okay to rw mount a FS with xattrs on some ancient 
(or non-Linux) kernel without xattr support — everything will be fine, 
even though there's no way to access them — but if the fsck tool doesn't 
understand them, it wouldn't be able to diagnose xattr corruption.

I'd either warn the user about unknown compat features, telling them 
they're on their own if something in the FS is still broken; or barf 
outright.