Re: [PATCH] btrfs/254: fix test failure due to already unmounted scratch device

[Anand Jain <anand.jain@oracle.com>]

On 28/2/25 13:20, Zorro Lang wrote:
> On Thu, Feb 27, 2025 at 01:25:18PM +0000, fdmanana@kernel.org wrote:
>> From: Filipe Manana <fdmanana@suse.com>
>>
>> If there are no failures in the middle of test while the 3rd scratch
>> device is mounted (at $seq_mnt), the unmount call in the _cleanup
>> function will result in a test failure since the unmount already
>> happened, making the test fail:
>>
>>    $ ./check btrfs/254
>>    FSTYP         -- btrfs
>>    PLATFORM      -- Linux/x86_64 debian0 6.14.0-rc4-btrfs-next-188+ #1 SMP PREEMPT_DYNAMIC Wed Feb 26 17:38:41 WET 2025
>>    MKFS_OPTIONS  -- /dev/sdc
>>    MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
>>
>>    btrfs/254 2s ... - output mismatch (see /home/fdmanana/git/hub/xfstests/results//btrfs/254.out.bad)
>>        --- tests/btrfs/254.out	2024-10-07 12:36:15.532225987 +0100
>>        +++ /home/fdmanana/git/hub/xfstests/results//btrfs/254.out.bad	2025-02-27 12:53:30.848728429 +0000
>>        @@ -3,3 +3,4 @@
>>         	Total devices <NUM> FS bytes used <SIZE>
>>         	devid <DEVID> size <SIZE> used <SIZE> path SCRATCH_DEV
>>         	*** Some devices missing
>>        +umount: /home/fdmanana/btrfs-tests/dev/254.mnt: not mounted.
>>        ...
>>        (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/254.out /home/fdmanana/git/hub/xfstests/results//btrfs/254.out.bad'  to see the entire diff)
>>
>>    HINT: You _MAY_ be missing kernel fix:
>>          770c79fb6550 btrfs: harden identification of a stale device
>>
>>    Ran: btrfs/254
>>    Failures: btrfs/254
>>    Failed 1 of 1 tests
>>
>> This is a recent regression because the _unmount function used to redirect
>> stdout and stderr to $seqres.full, but not anymore since the recent commit
>> identified in the Fixes tag below. So redirect stdout and stderr of the
>> call to _unmount in the _cleanup function to /dev/null.
>>
>> Fixes: f43da58ef936 ("unmount: resume logging of stdout and stderr for filtering")
>> Signed-off-by: Filipe Manana <fdmanana@suse.com>
>> ---
>>   tests/btrfs/254 | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/btrfs/254 b/tests/btrfs/254
>> index 33fdf059..e1b4fb01 100755
>> --- a/tests/btrfs/254
>> +++ b/tests/btrfs/254
>> @@ -21,7 +21,7 @@ _cleanup()
>>   {
>>   	cd /
>>   	rm -f $tmp.*
>> -	_unmount $seq_mnt
> 
> Oh this change was merged with the _unmount update together...
> 

>> +	_unmount $seq_mnt > /dev/null 2>&1
> 
> Sure, makes sense to me.
> 

Zorro,

The redirection part is missing in your for-next and needs to be fixed.

Thx.
Anand

> Reviewed-by: Zorro Lang <zlang@redhat.com>
> 
>>   	rm -rf $seq_mnt > /dev/null 2>&1
>>   	cleanup_dmdev
>>   }
>> -- 
>> 2.45.2
>>
>>
>