[PATCH 1/2] btrfs-progs: fix fsck-test/037 skip corrupt FREE_SPACE_BITMAP

[PATCH 1/2] btrfs-progs: fix fsck-test/037 skip corrupt FREE_SPACE_BITMAP

[Anand Jain]

We don't have the FREE_SPACE_BITMAP in the default filesystem,
skip the sub test by checking if the objectid is null for the
FREE_SPACE_BITMAP. The null objectid check for the FREE_SPACE_EXTENT
is not actually required, it is added to maintain similar flow in
the code above it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/fsck-tests/037-freespacetree-repair/test.sh | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/fsck-tests/037-freespacetree-repair/test.sh b/tests/fsck-tests/037-freespacetree-repair/test.sh
index d7ee0f217a09..39cdd5c8b89a 100755
--- a/tests/fsck-tests/037-freespacetree-repair/test.sh
+++ b/tests/fsck-tests/037-freespacetree-repair/test.sh
@@ -29,6 +29,9 @@ corrupt_fst_item()
 		objectid=$("$TOP/btrfs" inspect-internal dump-tree -t 10 "$TEST_DEV" | \
 			grep -o "[[:digit:]]* FREE_SPACE_BITMAP [[:digit:]]*" | \
 			cut -d' ' -f1 | tail -2 | head -1)
+		if [[ $objectid == "" ]]; then
+			return 1
+		fi
 		offset=$("$TOP/btrfs" inspect-internal dump-tree -t 10 "$TEST_DEV" | \
 			grep -o "[[:digit:]]* FREE_SPACE_BITMAP [[:digit:]]*" | \
 			cut -d' ' -f3 | tail -2 | head -1)
@@ -38,6 +41,9 @@ corrupt_fst_item()
 		objectid=$("$TOP/btrfs" inspect-internal dump-tree -t 10 "$TEST_DEV" | \
 			grep -o "[[:digit:]]* FREE_SPACE_EXTENT [[:digit:]]*" | \
 			cut -d' ' -f1 | tail -2 | head -1)
+		if [[ $objectid == "" ]]; then
+			return 1
+		fi
 		offset=$("$TOP/btrfs" inspect-internal dump-tree -t 10 "$TEST_DEV" | \
 			grep -o "[[:digit:]]* FREE_SPACE_EXTENT [[:digit:]]*" | \
 			cut -d' ' -f3 | tail -2 | head -1)
@@ -48,6 +54,8 @@ corrupt_fst_item()
 
 	run_check "$TOP/btrfs-corrupt-block" -r 10 -K "$objectid,$type,$offset" \
 		-f offset "$TEST_DEV"
+
+	return 0
 }
 
 if ! [ -f "/sys/fs/btrfs/features/free_space_tree" ]; then
@@ -69,8 +77,7 @@ done
 run_check_umount_test_dev
 
 # now corrupt one of the bitmap items
-corrupt_fst_item "bitmap"
-check_image "$TEST_DEV"
+corrupt_fst_item "bitmap" && check_image "$TEST_DEV"
 
 # change the freespace such that we now have at least one free_space_extent
 # object
@@ -80,5 +87,4 @@ run_check $SUDO_HELPER fallocate -l 50m "$TEST_MNT/file"
 run_check_umount_test_dev
 
 # now corrupt an extent
-corrupt_fst_item "extent"
-check_image "$TEST_DEV"
+corrupt_fst_item "extent" && check_image "$TEST_DEV"
-- 
1.8.3.1

[PATCH 2/2] btrfs-progs: fix misc-test/029 provide device for mount

[Anand Jain]

The mount fails with 'file exists' error. Fix it by providing the device
name.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 tests/misc-tests/029-send-p-different-mountpoints/test.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/misc-tests/029-send-p-different-mountpoints/test.sh b/tests/misc-tests/029-send-p-different-mountpoints/test.sh
index a478b3d26495..e34402d9ec06 100755
--- a/tests/misc-tests/029-send-p-different-mountpoints/test.sh
+++ b/tests/misc-tests/029-send-p-different-mountpoints/test.sh
@@ -19,8 +19,10 @@ run_mayfail $SUDO_HELPER mkdir -p "$SUBVOL_MNT" ||
 run_check_mkfs_test_dev
 run_check_mount_test_dev
 
+# The sed part is to replace double forward-slash with single forward-slash
+lodev=$(losetup  | grep $(echo $TEST_DEV | sed 's/\/\//\//') | awk '{print $1}')
 run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$TEST_MNT/subv1"
-run_check $SUDO_HELPER mount -t btrfs -o subvol=subv1 "$TEST_DEV" "$SUBVOL_MNT"
+run_check $SUDO_HELPER mount -t btrfs -o subvol=subv1 "$lodev" "$SUBVOL_MNT"
 
 run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$TEST_MNT/test-subvol"
 run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot -r \
-- 
1.8.3.1

Re: [PATCH 2/2] btrfs-progs: fix misc-test/029 provide device for mount

[David Sterba]

On Wed, Mar 25, 2020 at 04:32:09PM +0800, Anand Jain wrote:
> The mount fails with 'file exists' error. Fix it by providing the device
> name.

Can you be more specific about the environment where it fails? The test
passes for me.

> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  tests/misc-tests/029-send-p-different-mountpoints/test.sh | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/misc-tests/029-send-p-different-mountpoints/test.sh b/tests/misc-tests/029-send-p-different-mountpoints/test.sh
> index a478b3d26495..e34402d9ec06 100755
> --- a/tests/misc-tests/029-send-p-different-mountpoints/test.sh
> +++ b/tests/misc-tests/029-send-p-different-mountpoints/test.sh
> @@ -19,8 +19,10 @@ run_mayfail $SUDO_HELPER mkdir -p "$SUBVOL_MNT" ||
>  run_check_mkfs_test_dev
>  run_check_mount_test_dev
>  
> +# The sed part is to replace double forward-slash with single forward-slash
> +lodev=$(losetup  | grep $(echo $TEST_DEV | sed 's/\/\//\//') | awk '{print $1}')

There's a simpler way to canonicalize a path, eg using readlink or
realpath.

And I don't see why would two slashes appear in a path. IIRC a path
starting with two slashes is standardized as a network path and
recognized by VFS but why this is a concern for the testsuite?

Re: [PATCH 2/2] btrfs-progs: fix misc-test/029 provide device for mount

[Anand Jain]

On 3/26/20 11:46 PM, David Sterba wrote:
> On Wed, Mar 25, 2020 at 04:32:09PM +0800, Anand Jain wrote:
>> The mount fails with 'file exists' error. Fix it by providing the device
>> name.
> 
> Can you be more specific about the environment where it fails? The test
> passes for me.
> 

I am running it as

/btrfs-progs$ make TEST=029\* test-misc
     [TEST]   misc-tests.sh
     [TEST/misc]   029-send-p-different-mountpoints
failed: mount -t btrfs -o subvol=subv1 /btrfs-progs/tests//test.img 
/btrfs-progs/tests/misc-tests/029-send-p-different-mountpoints/subvol_mnt
test failed for case 029-send-p-different-mountpoints
make: *** [test-misc] Error 1

OR

make test

I wonder if there is any other/better way to run them?


>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   tests/misc-tests/029-send-p-different-mountpoints/test.sh | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/misc-tests/029-send-p-different-mountpoints/test.sh b/tests/misc-tests/029-send-p-different-mountpoints/test.sh
>> index a478b3d26495..e34402d9ec06 100755
>> --- a/tests/misc-tests/029-send-p-different-mountpoints/test.sh
>> +++ b/tests/misc-tests/029-send-p-different-mountpoints/test.sh
>> @@ -19,8 +19,10 @@ run_mayfail $SUDO_HELPER mkdir -p "$SUBVOL_MNT" ||
>>   run_check_mkfs_test_dev
>>   run_check_mount_test_dev
>>   
>> +# The sed part is to replace double forward-slash with single forward-slash
>> +lodev=$(losetup  | grep $(echo $TEST_DEV | sed 's/\/\//\//') | awk '{print $1}')
> 
> There's a simpler way to canonicalize a path, eg using readlink or
> realpath.
> 

  Err. yep. I will fix.

> And I don't see why would two slashes appear in a path. IIRC a path
> starting with two slashes is standardized as a network path and
> recognized by VFS but why this is a concern for the testsuite?
> 

The mount finds the path given is an image file and tries to add another
loop device to it (as I understand the above error), now I wonder
it should fail in your environment as well. If my understand is only
true.

Thanks, Anand

[PATCH 2/2 v2] btrfs-progs: fix misc-test/029 provide device for mount

[Anand Jain]

The mount fails with 'file exists' error. Fix it by providing the device
name.

$ make TEST=029\* test-misc
    [TEST]   misc-tests.sh
    [TEST/misc]   029-send-p-different-mountpoints
failed: mount -t btrfs -o subvol=subv1 /btrfs-progs/tests//test.img /btrfs-progs/tests/misc-tests/029-send-p-different-mountpoints/subvol_mnt
test failed for case 029-send-p-different-mountpoints
make: *** [test-misc] Error 1

====== RUN CHECK mount -t btrfs -o subvol=subv1
/btrfs-progs/tests//test.img
/btrfs-progs/tests/misc-tests/029-send-p-different-mountpoints/subvol_mnt
mount: mount /dev/loop1 on
/btrfs-progs/tests/misc-tests/029-send-p-different-mountpoints/subvol_mnt
failed: File exists
failed: mount -t btrfs -o subvol=subv1 /btrfs-progs/tests//test.img
/btrfs-progs/tests/misc-tests/029-send-p-different-mountpoints/subvol_mnt
test failed for case 029-send-p-different-mountpoints

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---

v2: use readlink to sanitize the TEST_DEV path
    update change log

 tests/misc-tests/029-send-p-different-mountpoints/test.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/misc-tests/029-send-p-different-mountpoints/test.sh b/tests/misc-tests/029-send-p-different-mountpoints/test.sh
index a478b3d26495..0ab98f93416d 100755
--- a/tests/misc-tests/029-send-p-different-mountpoints/test.sh
+++ b/tests/misc-tests/029-send-p-different-mountpoints/test.sh
@@ -19,8 +19,9 @@ run_mayfail $SUDO_HELPER mkdir -p "$SUBVOL_MNT" ||
 run_check_mkfs_test_dev
 run_check_mount_test_dev
 
+lodev=$(losetup  | grep $(readlink -f ${TEST_DEV}) | awk '{print $1}')
 run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$TEST_MNT/subv1"
-run_check $SUDO_HELPER mount -t btrfs -o subvol=subv1 "$TEST_DEV" "$SUBVOL_MNT"
+run_check $SUDO_HELPER mount -t btrfs -o subvol=subv1 "$lodev" "$SUBVOL_MNT"
 
 run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$TEST_MNT/test-subvol"
 run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot -r \
-- 
1.8.3.1

Re: [PATCH 2/2] btrfs-progs: fix misc-test/029 provide device for mount

[David Sterba]

On Fri, Mar 27, 2020 at 12:15:45PM +0800, Anand Jain wrote:
> 
> 
> On 3/26/20 11:46 PM, David Sterba wrote:
> > On Wed, Mar 25, 2020 at 04:32:09PM +0800, Anand Jain wrote:
> >> The mount fails with 'file exists' error. Fix it by providing the device
> >> name.
> > 
> > Can you be more specific about the environment where it fails? The test
> > passes for me.
> > 
> 
> I am running it as
> 
> /btrfs-progs$ make TEST=029\* test-misc
>      [TEST]   misc-tests.sh
>      [TEST/misc]   029-send-p-different-mountpoints
> failed: mount -t btrfs -o subvol=subv1 /btrfs-progs/tests//test.img 
> /btrfs-progs/tests/misc-tests/029-send-p-different-mountpoints/subvol_mnt
> test failed for case 029-send-p-different-mountpoints
> make: *** [test-misc] Error 1

That command works fine and the path of image has double slashes too:

$ make TEST=029\* test-misc
    [TEST]   misc-tests.sh
    [TEST/misc]   029-send-p-different-mountpoints

From the log, the first run of mount:

RUN CHECK root_helper mount -t btrfs -o loop /btrfs-progs/tests//test.img /btrfs-progs/tests//mnt

> >> --- a/tests/misc-tests/029-send-p-different-mountpoints/test.sh
> >> +++ b/tests/misc-tests/029-send-p-different-mountpoints/test.sh
> >> @@ -19,8 +19,10 @@ run_mayfail $SUDO_HELPER mkdir -p "$SUBVOL_MNT" ||
> >>   run_check_mkfs_test_dev
> >>   run_check_mount_test_dev
> >>   
> >> +# The sed part is to replace double forward-slash with single forward-slash
> >> +lodev=$(losetup  | grep $(echo $TEST_DEV | sed 's/\/\//\//') | awk '{print $1}')
> > 
> > There's a simpler way to canonicalize a path, eg using readlink or
> > realpath.
> 
>   Err. yep. I will fix.
> 
> > And I don't see why would two slashes appear in a path. IIRC a path
> > starting with two slashes is standardized as a network path and
> > recognized by VFS but why this is a concern for the testsuite?
> 
> The mount finds the path given is an image file and tries to add another
> loop device to it (as I understand the above error), now I wonder
> it should fail in your environment as well. If my understand is only
> true.

Instead of patching the tests, the path to TEST_DEV should be
canonicalized in prepare_test_dev or initialization of TEST_DEV should
not put the slash at the end in the test driver scripts.

tests/misc-tests.sh:
...
TEST_TOP="$TOP/tests/"