Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE

["Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com>]

On 1/13/25 21:09, Ritesh Harjani (IBM) wrote:
> "Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com> writes:
>
>> On 1/13/25 01:11, Ritesh Harjani (IBM) wrote:
>>> "Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com> writes:
>>>
>>>> Bug Description:
>>>>
>>>> _test_mount function is failing with the following error:
>>>> ./common/rc: line 4716: _xfs_prepare_for_eio_shutdown: command not found
> Please notice the error that you are seeing here ^^^
>
>>>> check: failed to mount /dev/loop0 on /mnt1/test
>>>>
>>>> when the second section in local.config file is xfs and the first section
>>>> is non-xfs.
>>>>
>>>> It can be easily reproduced with the following local.config file
>>>>
>>>> [s2]
>>>> export FSTYP=ext4
>>>> export TEST_DEV=/dev/loop0
>>>> export TEST_DIR=/mnt1/test
>>>> export SCRATCH_DEV=/dev/loop1
>>>> export SCRATCH_MNT=/mnt1/scratch
>>>>
>>>> [s1]
>>>> export FSTYP=xfs
>>>> export TEST_DEV=/dev/loop0
>>>> export TEST_DIR=/mnt1/test
>>>> export SCRATCH_DEV=/dev/loop1
>>>> export SCRATCH_MNT=/mnt1/scratch
>>>>
>>>> ./check selftest/001
>>>>
>>>> Root cause:
>>>> When _test_mount() is executed for the second section, the FSTYPE has
>>>> already changed but the new fs specific common/$FSTYP has not yet
>>>> been done. Hence _xfs_prepare_for_eio_shutdown() is not found and
>>>> the test run fails.
>>>>
>>>> Fix:
>>>> call _source_specific_fs $FSTYP at the correct call site of  _test_mount()
>>> You should add the Fixes: tag too. Based on your description I guess
>>> this should be the tag?
>>>
>>> Fixes: 1a49022fab9b4 ("fstests: always use fail-at-unmount semantics for XFS")
> Please look into the above commit. The above patch introduced function
> "_prepare_for_eio_shutdown()" in _test_mount(), which is what we are
> getting the error for (for XFS i.e. _xfs_prepare_for_eio_shutdown()
> command not found). Right?
Okay, now I got the logic. I was thinking of the commit that added the 
entire "if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ];" block 
(where the sourcing should have been there)
>
> Ok, why don't revert the above commit and see if the revert fixes the
> issue for you.
Yes, I can check that.
>
> https://www.kernel.org/doc/html/v4.10/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

Thanks, I will read this.

--NR

>
> -ritesh
>
>> Shouldn't this be the following?
>>
>> commit f8e4f532f18d7517430d9849bfc042305d7f7f4d (HEAD)
>> Author: Lukas Czerner <lczerner@redhat.com>
>> Date:   Fri Apr 4 17:18:15 2014 +1100
>>
>>       check: Allow to recreate TEST_DEV
>>
>>       Add config option RECREATE_TEST_DEV to allow to recreate file system on
>>       the TEST_DEV device. Permitted values are true and false.
>>
>>       If RECREATE_TEST_DEV is set to true the TEST_DEV device will be
>>       unmounted and FSTYP file system will be created on it. Afterwards it
>>       will be mounted to TEST_DIR again with the default, or specified mount
>>       options.
>>
>>       Also recreate the file system if FSTYP differs from the previous
>>       section.
>>
>>> I agree with today the problem was in _test_mount(), tomorrow it could
>>> be _test_mkfs, hence we could source the new FSTYP config file before
>>> calling _test_mkfs().
>>>
>>> With the fixes tag added, please feel free to add:
>>>
>>> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>>>
>>>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>>>> ---
>>>>    check | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/check b/check
>>>> index 607d2456..8cdbb68f 100755
>>>> --- a/check
>>>> +++ b/check
>>>> @@ -776,6 +776,7 @@ function run_section()
>>>>    	if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
>>>>    		echo "RECREATING    -- $FSTYP on $TEST_DEV"
>>>>    		_test_unmount 2> /dev/null
>>>> +		[[ "$OLD_FSTYP" != "$FSTYP" ]] && _source_specific_fs $FSTYP
>>>>    		if ! _test_mkfs >$tmp.err 2>&1
>>>>    		then
>>>>    			echo "our local _test_mkfs routine ..."
>>>> -- 
>>>> 2.34.1
>> -- 
>> Nirjhar Roy
>> Linux Kernel Developer
>> IBM, Bangalore

-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore