Re: [PATCH 1/3] btrfs: Add incompat flags check for btrfs_check_super_valid()

[Anand Jain]

On 04/20/2018 11:15 PM, David Sterba wrote:

On Fri, Apr 20, 2018 at 10:32:03PM +0800, Anand Jain wrote:

On 04/19/2018 05:38 PM, Qu Wenruo wrote:

Although we have already checked incompat flags manually before really
mounting it, we could still enhance btrfs_check_super_valid() to check
incompat flags for later write time super block validation check.

This patch adds such incompat flags check for btrfs_check_super_valid(),
currently it won't be triggered, but provides the basis for later write
time check.

Signed-off-by: Qu Wenruo <w...@suse.com>
---
   fs/btrfs/disk-io.c | 13 +++++++++++++
   1 file changed, 13 insertions(+)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 60caa68c3618..ec123158f051 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4104,6 +4104,19 @@ static int btrfs_check_super_valid(struct btrfs_fs_info 
*fs_info)
                ret = -EINVAL;
        }

+ /*

+        * Before calling btrfs_check_super_valid() we have already checked
+        * incompat flags. So if we developr new incompat flags, it's must be
+        * some corruption.
+        */
+       if (btrfs_super_incompat_flags(sb) & ~BTRFS_FEATURE_INCOMPAT_SUPP) {
+               btrfs_err(fs_info,
+               "corrupted incompat flags detected 0x%llx, supported 0x%llx",

2707         features = btrfs_super_incompat_flags(disk_super) &
2708                 ~BTRFS_FEATURE_INCOMPAT_SUPP;
2709         if (features) {
2710                 btrfs_err(fs_info,
2711                     "cannot mount because of unsupported optional features 
(%llx)",
2712                     features);
2713                 err = -EINVAL;
2714                 goto fail_alloc;
2715         }

So there's a "user experience" change, now that you pointed out the

other check.

 Its a regression.

If a filesystem with incompat bits set will be mounted,
this will say 'you have corrupted filesystem', which is not IMO what we
want to tell.

Tough the extended btrfs_check_super_valid could catch the corrupted
incompat bits, what we need at mount time is wording from the 2nd
message ("cannot mount unsuppported features").

I think that there should be a separate function that does the
pre-commit checks, calls btrfs_check_super_valid and also validates the
incompat bit.

 We can still print it as 'unsupported optional features', which would
 imply corruption in the non-mount context.

Thanks, Anand

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html