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

2018-04-20 Thread Anand Jain



On 04/21/2018 10:43 AM, Qu Wenruo wrote:



On 2018年04月21日 10:38, Anand Jain wrote:



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 
---
    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.


In that case such wording is not obvious enough to info it's super block
corruption.


 If the device is already mounted. Then its a corruption.

Thanks, Anand


So I still prefer current way of checking incompact flags and csum types
manually at mount time before btrfs_validate_super().

David's idea of a separate function to do mount time check and gives
better prompt is pretty good.
Especially for incompat features, as incompat features will increase in
the future and for older kernel info such feature as corruption is not
acceptable.

But currently, there are only 2 members we need to check at mount time
(csum_type and incompat features), and only one caller, current code
looks good enough for its purpose.

Thanks,
Qu



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



--
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


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

2018-04-20 Thread Qu Wenruo


On 2018年04月21日 10:38, Anand Jain wrote:
> 
> 
> 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 
 ---
    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.

In that case such wording is not obvious enough to info it's super block
corruption.

So I still prefer current way of checking incompact flags and csum types
manually at mount time before btrfs_validate_super().

David's idea of a separate function to do mount time check and gives
better prompt is pretty good.
Especially for incompat features, as incompat features will increase in
the future and for older kernel info such feature as corruption is not
acceptable.

But currently, there are only 2 members we need to check at mount time
(csum_type and incompat features), and only one caller, current code
looks good enough for its purpose.

Thanks,
Qu

> 
> 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



signature.asc
Description: OpenPGP digital signature


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

2018-04-20 Thread 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 
---
   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


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

2018-04-20 Thread David Sterba
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 
> > ---
> >   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. 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.
--
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


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

2018-04-20 Thread Anand Jain



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 
---
  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",
+ btrfs_super_incompat_flags(sb),
+ BTRFS_FEATURE_INCOMPAT_SUPP);
+   ret = -EINVAL;
+   }
+
/*
 * The generation is a global counter, we'll trust it more than the 
others
 * but it's still possible that it's the one that's wrong.



This patch should move the existing check in the open_ctree() line 2707
into the btrfs_check_super_valid(), we don't need duplicate checks.

disk-io.c

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 }


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


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

2018-04-20 Thread David Sterba
On Thu, Apr 19, 2018 at 07:24:48PM +0300, Nikolay Borisov wrote:
> I agree with David, just make it btrfs_validate_super

Ack.
--
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


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

2018-04-19 Thread Nikolay Borisov


On 19.04.2018 18:31, David Sterba wrote:
> On Thu, Apr 19, 2018 at 07:10:30PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2018年04月19日 18:59, Nikolay Borisov wrote:
>>>
>>>
>>> On 19.04.2018 12:38, 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 
>>>
>>> Reviewed-by: Nikolay Borisov 
>>>
 ---
  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)
>>>
>>> nit: Thinking out loud here - wouldn't it make more sense to name the
>>> function btrfs_validate_super. check_super_valid sounds a bit cumbersome
>>> to me. What do you think ?
>>
>> Indeed, I also like to remove the btrfs_ prefix since it's a static
>> function.
>> validate_super() looks much better.
> 
> It's not necessary to remove the btrfs_ prefix from all static
> functions, sometimes the functions appear on stacks or mixed with other
> subystem helpers that have generic names. The prefix makes it clear that
> it's our function.

I agree with David, just make it btrfs_validate_super
> 
--
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


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

2018-04-19 Thread David Sterba
On Thu, Apr 19, 2018 at 07:10:30PM +0800, Qu Wenruo wrote:
> 
> 
> On 2018年04月19日 18:59, Nikolay Borisov wrote:
> > 
> > 
> > On 19.04.2018 12:38, 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 
> > 
> > Reviewed-by: Nikolay Borisov 
> > 
> >> ---
> >>  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)
> > 
> > nit: Thinking out loud here - wouldn't it make more sense to name the
> > function btrfs_validate_super. check_super_valid sounds a bit cumbersome
> > to me. What do you think ?
> 
> Indeed, I also like to remove the btrfs_ prefix since it's a static
> function.
> validate_super() looks much better.

It's not necessary to remove the btrfs_ prefix from all static
functions, sometimes the functions appear on stacks or mixed with other
subystem helpers that have generic names. The prefix makes it clear that
it's our function.
--
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


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

2018-04-19 Thread Qu Wenruo


On 2018年04月19日 18:59, Nikolay Borisov wrote:
> 
> 
> On 19.04.2018 12:38, 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 
> 
> Reviewed-by: Nikolay Borisov 
> 
>> ---
>>  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)
> 
> nit: Thinking out loud here - wouldn't it make more sense to name the
> function btrfs_validate_super. check_super_valid sounds a bit cumbersome
> to me. What do you think ?

Indeed, I also like to remove the btrfs_ prefix since it's a static
function.
validate_super() looks much better.

Thanks,
Qu

>>  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
> s/developr/detect ?
>> + * 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",
>> +  btrfs_super_incompat_flags(sb),
>> +  BTRFS_FEATURE_INCOMPAT_SUPP);
>> +ret = -EINVAL;
>> +}
>> +
>>  /*
>>   * The generation is a global counter, we'll trust it more than the 
>> others
>>   * but it's still possible that it's the one that's wrong.
>>
> --
> 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


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

2018-04-19 Thread Nikolay Borisov


On 19.04.2018 12:38, 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 

Reviewed-by: Nikolay Borisov 

> ---
>  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)

nit: Thinking out loud here - wouldn't it make more sense to name the
function btrfs_validate_super. check_super_valid sounds a bit cumbersome
to me. What do you think ?
>   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
s/developr/detect ?
> +  * 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",
> +   btrfs_super_incompat_flags(sb),
> +   BTRFS_FEATURE_INCOMPAT_SUPP);
> + ret = -EINVAL;
> + }
> +
>   /*
>* The generation is a global counter, we'll trust it more than the 
> others
>* but it's still possible that it's the one that's wrong.
> 
--
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


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

2018-04-19 Thread Qu Wenruo
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 
---
 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",
+ btrfs_super_incompat_flags(sb),
+ BTRFS_FEATURE_INCOMPAT_SUPP);
+   ret = -EINVAL;
+   }
+
/*
 * The generation is a global counter, we'll trust it more than the 
others
 * but it's still possible that it's the one that's wrong.
-- 
2.17.0

--
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