[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220831115517.qolsk27xh5djei7h@quack3>
Date: Wed, 31 Aug 2022 13:55:17 +0200
From: Jan Kara <jack@...e.cz>
To: Jason Yan <yanaijie@...wei.com>
Cc: tytso@....edu, adilger.kernel@...ger.ca, jack@...e.cz,
ritesh.list@...il.com, lczerner@...hat.com,
linux-ext4@...r.kernel.org
Subject: Re: [PATCH 09/13] ext4: factor out ext4_compat_feature_check()
On Tue 30-08-22 20:04:07, Jason Yan wrote:
> Factor out ext4_compat_feature_check(). No functional change.
>
> Signed-off-by: Jason Yan <yanaijie@...wei.com>
> ---
> fs/ext4/super.c | 144 ++++++++++++++++++++++++++----------------------
> 1 file changed, 77 insertions(+), 67 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 96cf23787bba..1e7d6eb6a3aa 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4607,6 +4607,82 @@ static int ext4_handle_csum(struct super_block *sb, struct ext4_super_block *es)
> return 0;
> }
>
> +static int ext4_compat_feature_check(struct super_block *sb,
> + struct ext4_super_block *es,
> + int silent)
And here maybe ext4_check_feature_compatibility() might be a better name
because "compat_feature" is a name of a specific subset of ext4 features so
using it in function name is a bit confusing. Otherwise feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
> +{
> + if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV &&
> + (ext4_has_compat_features(sb) ||
> + ext4_has_ro_compat_features(sb) ||
> + ext4_has_incompat_features(sb)))
> + ext4_msg(sb, KERN_WARNING,
> + "feature flags set on rev 0 fs, "
> + "running e2fsck is recommended");
> +
> + if (es->s_creator_os == cpu_to_le32(EXT4_OS_HURD)) {
> + set_opt2(sb, HURD_COMPAT);
> + if (ext4_has_feature_64bit(sb)) {
> + ext4_msg(sb, KERN_ERR,
> + "The Hurd can't support 64-bit file systems");
> + return -EINVAL;
> + }
> +
> + /*
> + * ea_inode feature uses l_i_version field which is not
> + * available in HURD_COMPAT mode.
> + */
> + if (ext4_has_feature_ea_inode(sb)) {
> + ext4_msg(sb, KERN_ERR,
> + "ea_inode feature is not supported for Hurd");
> + return -EINVAL;
> + }
> + }
> +
> + if (IS_EXT2_SB(sb)) {
> + if (ext2_feature_set_ok(sb))
> + ext4_msg(sb, KERN_INFO, "mounting ext2 file system "
> + "using the ext4 subsystem");
> + else {
> + /*
> + * If we're probing be silent, if this looks like
> + * it's actually an ext[34] filesystem.
> + */
> + if (silent && ext4_feature_set_ok(sb, sb_rdonly(sb)))
> + return -EINVAL;
> + ext4_msg(sb, KERN_ERR, "couldn't mount as ext2 due "
> + "to feature incompatibilities");
> + return -EINVAL;
> + }
> + }
> +
> + if (IS_EXT3_SB(sb)) {
> + if (ext3_feature_set_ok(sb))
> + ext4_msg(sb, KERN_INFO, "mounting ext3 file system "
> + "using the ext4 subsystem");
> + else {
> + /*
> + * If we're probing be silent, if this looks like
> + * it's actually an ext4 filesystem.
> + */
> + if (silent && ext4_feature_set_ok(sb, sb_rdonly(sb)))
> + return -EINVAL;
> + ext4_msg(sb, KERN_ERR, "couldn't mount as ext3 due "
> + "to feature incompatibilities");
> + return -EINVAL;
> + }
> + }
> +
> + /*
> + * Check feature flags regardless of the revision level, since we
> + * previously didn't change the revision level when setting the flags,
> + * so there is a chance incompat flags are set on a rev 0 filesystem.
> + */
> + if (!ext4_feature_set_ok(sb, (sb_rdonly(sb))))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> {
> struct buffer_head *bh, **group_desc;
> @@ -4761,73 +4837,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> sb->s_flags = (sb->s_flags & ~SB_POSIXACL) |
> (test_opt(sb, POSIX_ACL) ? SB_POSIXACL : 0);
>
> - if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV &&
> - (ext4_has_compat_features(sb) ||
> - ext4_has_ro_compat_features(sb) ||
> - ext4_has_incompat_features(sb)))
> - ext4_msg(sb, KERN_WARNING,
> - "feature flags set on rev 0 fs, "
> - "running e2fsck is recommended");
> -
> - if (es->s_creator_os == cpu_to_le32(EXT4_OS_HURD)) {
> - set_opt2(sb, HURD_COMPAT);
> - if (ext4_has_feature_64bit(sb)) {
> - ext4_msg(sb, KERN_ERR,
> - "The Hurd can't support 64-bit file systems");
> - goto failed_mount;
> - }
> -
> - /*
> - * ea_inode feature uses l_i_version field which is not
> - * available in HURD_COMPAT mode.
> - */
> - if (ext4_has_feature_ea_inode(sb)) {
> - ext4_msg(sb, KERN_ERR,
> - "ea_inode feature is not supported for Hurd");
> - goto failed_mount;
> - }
> - }
> -
> - if (IS_EXT2_SB(sb)) {
> - if (ext2_feature_set_ok(sb))
> - ext4_msg(sb, KERN_INFO, "mounting ext2 file system "
> - "using the ext4 subsystem");
> - else {
> - /*
> - * If we're probing be silent, if this looks like
> - * it's actually an ext[34] filesystem.
> - */
> - if (silent && ext4_feature_set_ok(sb, sb_rdonly(sb)))
> - goto failed_mount;
> - ext4_msg(sb, KERN_ERR, "couldn't mount as ext2 due "
> - "to feature incompatibilities");
> - goto failed_mount;
> - }
> - }
> -
> - if (IS_EXT3_SB(sb)) {
> - if (ext3_feature_set_ok(sb))
> - ext4_msg(sb, KERN_INFO, "mounting ext3 file system "
> - "using the ext4 subsystem");
> - else {
> - /*
> - * If we're probing be silent, if this looks like
> - * it's actually an ext4 filesystem.
> - */
> - if (silent && ext4_feature_set_ok(sb, sb_rdonly(sb)))
> - goto failed_mount;
> - ext4_msg(sb, KERN_ERR, "couldn't mount as ext3 due "
> - "to feature incompatibilities");
> - goto failed_mount;
> - }
> - }
> -
> - /*
> - * Check feature flags regardless of the revision level, since we
> - * previously didn't change the revision level when setting the flags,
> - * so there is a chance incompat flags are set on a rev 0 filesystem.
> - */
> - if (!ext4_feature_set_ok(sb, (sb_rdonly(sb))))
> + if (ext4_compat_feature_check(sb, es, silent))
> goto failed_mount;
>
> if (le16_to_cpu(sbi->s_es->s_reserved_gdt_blocks) > (blocksize / 4)) {
> --
> 2.31.1
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists