[PATCH 1/2] btrfs-progs: Fix false alert about EXTENT_DATA shouldn't be hole

[PATCH 1/2] btrfs-progs: Fix false alert about EXTENT_DATA shouldn't be hole

[Qu Wenruo]

Since incompat feature NO_HOLES still allow us to have explicit hole
file extent, current check is too restrict and will cause false alert
like:

root 5 EXTENT_DATA[257, 0] shouldn't be hole

Fix it by removing the restrict hole file extent check.

Reported-by: Henk Slager <eye1tm@gmail.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-check.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index c052f66e..7bd57677 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -4841,11 +4841,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 	}
 
 	/* Check EXTENT_DATA hole */
-	if (no_holes && is_hole) {
-		err |= FILE_EXTENT_ERROR;
-		error("root %llu EXTENT_DATA[%llu %llu] shouldn't be hole",
-		      root->objectid, fkey->objectid, fkey->offset);
-	} else if (!no_holes && *end != fkey->offset) {
+	if (!no_holes && *end != fkey->offset) {
 		err |= FILE_EXTENT_ERROR;
 		error("root %llu EXTENT_DATA[%llu %llu] interrupt",
 		      root->objectid, fkey->objectid, fkey->offset);
-- 
2.13.1

[PATCH 2/2] btrfs-progs: Add test case to check hole file extents with NO_HOLES flag

[Qu Wenruo]

Add test case which we have NO_HOLES incompat flag while still have
hole file extent.

This can be created by enabling NO_HOLES feature on an existing
filesystem, which lowmem mode would cause false alert for it.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 tests/fsck-tests/025-file-extents/test.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tests/fsck-tests/025-file-extents/test.sh b/tests/fsck-tests/025-file-extents/test.sh
index e4bc4247..66cfbaab 100755
--- a/tests/fsck-tests/025-file-extents/test.sh
+++ b/tests/fsck-tests/025-file-extents/test.sh
@@ -5,6 +5,7 @@ source "$TOP/tests/common"
 
 check_prereq btrfs
 check_prereq mkfs.btrfs
+check_prereq btrfstune
 check_global_prereq dd
 check_global_prereq fallocate
 
@@ -38,5 +39,21 @@ test_compressed_inline_extent()
 	run_check "$TOP/btrfs" check "$TEST_DEV"
 }
 
+# Hole file extent with NO_HOLES incompat flag
+# Lowmem mode will cause false alert as it doesn't allow any hole file extent
+# exist, while we can set NO_HOLES at anytime we want, it's definitely a false
+# alert
+test_hole_extent_with_no_holes_flag()
+{
+	run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+	run_check_mount_test_dev
+
+	run_check $SUDO_HELPER truncate -s 16K "$TEST_MNT/tmp"
+	run_check_umount_test_dev
+	run_check $SUDO_HELPER "$TOP/btrfstune" -n "$TEST_DEV"
+	run_check "$TOP/btrfs" check "$TEST_DEV"
+}
+
 test_paritical_write_into_prealloc
 test_compressed_inline_extent
+test_hole_extent_with_no_holes_flag
-- 
2.13.1

Re: [PATCH 1/2] btrfs-progs: Fix false alert about EXTENT_DATA shouldn't be hole

[Henk Slager]

On 16-06-17 03:43, Qu Wenruo wrote:
> Since incompat feature NO_HOLES still allow us to have explicit hole
> file extent, current check is too restrict and will cause false alert
> like:
>
> root 5 EXTENT_DATA[257, 0] shouldn't be hole
>
> Fix it by removing the restrict hole file extent check.
>
> Reported-by: Henk Slager <eye1tm@gmail.com>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>  cmds-check.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/cmds-check.c b/cmds-check.c
> index c052f66e..7bd57677 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -4841,11 +4841,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
>  	}
>  
>  	/* Check EXTENT_DATA hole */
> -	if (no_holes && is_hole) {
> -		err |= FILE_EXTENT_ERROR;
> -		error("root %llu EXTENT_DATA[%llu %llu] shouldn't be hole",
> -		      root->objectid, fkey->objectid, fkey->offset);
> -	} else if (!no_holes && *end != fkey->offset) {
> +	if (!no_holes && *end != fkey->offset) {
>  		err |= FILE_EXTENT_ERROR;
>  		error("root %llu EXTENT_DATA[%llu %llu] interrupt",
>  		      root->objectid, fkey->objectid, fkey->offset);


Thanks for the patch, I applied it on v4.11 btrfs-progs and re-ran the check:
# btrfs check -p --readonly /dev/mapper/smr

on filesystem mentioned in:
https://www.spinics.net/lists/linux-btrfs/msg66374.html

and now the "shouldn't be hole" errors don't show up anymore.

Tested-by: Henk Slager <eye1tm@gmail.com>

Re: [PATCH 1/2] btrfs-progs: Fix false alert about EXTENT_DATA shouldn't be hole

[David Sterba]

On Mon, Jun 19, 2017 at 01:26:20PM +0200, Henk Slager wrote:
> On 16-06-17 03:43, Qu Wenruo wrote:
> > Since incompat feature NO_HOLES still allow us to have explicit hole
> > file extent, current check is too restrict and will cause false alert
> > like:
> >
> > root 5 EXTENT_DATA[257, 0] shouldn't be hole
> >
> > Fix it by removing the restrict hole file extent check.
> >
> > Reported-by: Henk Slager <eye1tm@gmail.com>
> > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> > ---
> >  cmds-check.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/cmds-check.c b/cmds-check.c
> > index c052f66e..7bd57677 100644
> > --- a/cmds-check.c
> > +++ b/cmds-check.c
> > @@ -4841,11 +4841,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
> >  	}
> >  
> >  	/* Check EXTENT_DATA hole */
> > -	if (no_holes && is_hole) {
> > -		err |= FILE_EXTENT_ERROR;
> > -		error("root %llu EXTENT_DATA[%llu %llu] shouldn't be hole",
> > -		      root->objectid, fkey->objectid, fkey->offset);
> > -	} else if (!no_holes && *end != fkey->offset) {
> > +	if (!no_holes && *end != fkey->offset) {
> >  		err |= FILE_EXTENT_ERROR;
> >  		error("root %llu EXTENT_DATA[%llu %llu] interrupt",
> >  		      root->objectid, fkey->objectid, fkey->offset);
> 
> 
> Thanks for the patch, I applied it on v4.11 btrfs-progs and re-ran the check:
> # btrfs check -p --readonly /dev/mapper/smr
> 
> on filesystem mentioned in:
> https://www.spinics.net/lists/linux-btrfs/msg66374.html
> 
> and now the "shouldn't be hole" errors don't show up anymore.
> 
> Tested-by: Henk Slager <eye1tm@gmail.com>

Thank you both, patch applied. I might also release a 4.11.x release
with this fix included.

Re: [PATCH 1/2] btrfs-progs: Fix false alert about EXTENT_DATA shouldn't be hole

[Henk Slager]

On Mon, Jun 19, 2017 at 1:26 PM, Henk Slager <eye1tm@gmail.com> wrote:
> On 16-06-17 03:43, Qu Wenruo wrote:
>> Since incompat feature NO_HOLES still allow us to have explicit hole
>> file extent, current check is too restrict and will cause false alert
>> like:
>>
>> root 5 EXTENT_DATA[257, 0] shouldn't be hole
>>
>> Fix it by removing the restrict hole file extent check.
>>
>> Reported-by: Henk Slager <eye1tm@gmail.com>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> ---
>>  cmds-check.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/cmds-check.c b/cmds-check.c
>> index c052f66e..7bd57677 100644
>> --- a/cmds-check.c
>> +++ b/cmds-check.c
>> @@ -4841,11 +4841,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
>>       }
>>
>>       /* Check EXTENT_DATA hole */
>> -     if (no_holes && is_hole) {
>> -             err |= FILE_EXTENT_ERROR;
>> -             error("root %llu EXTENT_DATA[%llu %llu] shouldn't be hole",
>> -                   root->objectid, fkey->objectid, fkey->offset);
>> -     } else if (!no_holes && *end != fkey->offset) {
>> +     if (!no_holes && *end != fkey->offset) {
>>               err |= FILE_EXTENT_ERROR;
>>               error("root %llu EXTENT_DATA[%llu %llu] interrupt",
>>                     root->objectid, fkey->objectid, fkey->offset);
>
>
> Thanks for the patch, I applied it on v4.11 btrfs-progs and re-ran the check:
> # btrfs check -p --readonly /dev/mapper/smr
While looking at new kernel+progs releases  and my script logs, I see
I made copy-paste error; of course it is the lowmem mode:
# btrfs check -p --mode lowmem --readonly /dev/mapper/smr
> on filesystem mentioned in:
> https://www.spinics.net/lists/linux-btrfs/msg66374.html
>
> and now the "shouldn't be hole" errors don't show up anymore.
>
> Tested-by: Henk Slager <eye1tm@gmail.com>
>