* [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE
@ 2025-01-12 15:21 Nirjhar Roy (IBM)
2025-01-12 19:41 ` Ritesh Harjani
2025-01-13 5:59 ` Zorro Lang
0 siblings, 2 replies; 11+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-01-12 15:21 UTC (permalink / raw)
To: fstests
Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang,
nirjhar.roy.lists
Bug Description:
_test_mount function is failing with the following error:
./common/rc: line 4716: _xfs_prepare_for_eio_shutdown: command not found
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()
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE
2025-01-12 15:21 [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE Nirjhar Roy (IBM)
@ 2025-01-12 19:41 ` Ritesh Harjani
2025-01-13 5:59 ` Nirjhar Roy (IBM)
2025-01-13 5:59 ` Zorro Lang
1 sibling, 1 reply; 11+ messages in thread
From: Ritesh Harjani @ 2025-01-12 19:41 UTC (permalink / raw)
To: Nirjhar Roy (IBM), fstests
Cc: linux-ext4, linux-xfs, ojaswin, djwong, zlang, nirjhar.roy.lists
"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
> 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")
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE
2025-01-12 15:21 [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE Nirjhar Roy (IBM)
2025-01-12 19:41 ` Ritesh Harjani
@ 2025-01-13 5:59 ` Zorro Lang
2025-01-13 8:52 ` Nirjhar Roy (IBM)
1 sibling, 1 reply; 11+ messages in thread
From: Zorro Lang @ 2025-01-13 5:59 UTC (permalink / raw)
To: Nirjhar Roy (IBM)
Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong,
zlang
On Sun, Jan 12, 2025 at 03:21:51PM +0000, Nirjhar Roy (IBM) wrote:
> Bug Description:
>
> _test_mount function is failing with the following error:
> ./common/rc: line 4716: _xfs_prepare_for_eio_shutdown: command not found
> 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()
>
> 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
The _source_specific_fs is called when importing common/rc file:
# check for correct setup and source the $FSTYP specific functions now
_source_specific_fs $FSTYP
From the code logic of check script:
if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
echo "RECREATING -- $FSTYP on $TEST_DEV"
_test_unmount 2> /dev/null
if ! _test_mkfs >$tmp.err 2>&1
then
echo "our local _test_mkfs routine ..."
cat $tmp.err
echo "check: failed to mkfs \$TEST_DEV using specified options"
status=1
exit
fi
if ! _test_mount
then
echo "check: failed to mount $TEST_DEV on $TEST_DIR"
status=1
exit
fi
# TEST_DEV has been recreated, previous FSTYP derived from
# TEST_DEV could be changed, source common/rc again with
# correct FSTYP to get FSTYP specific configs, e.g. common/xfs
. common/rc
^^^^^^^^^^^
we import common/rc at here.
So I'm wondering if we can move this line upward, to fix the problem you
hit (and don't bring in regression :) Does that help?
Thanks,
Zorro
> if ! _test_mkfs >$tmp.err 2>&1
> then
> echo "our local _test_mkfs routine ..."
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE
2025-01-12 19:41 ` Ritesh Harjani
@ 2025-01-13 5:59 ` Nirjhar Roy (IBM)
2025-01-13 15:39 ` Ritesh Harjani
0 siblings, 1 reply; 11+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-01-13 5:59 UTC (permalink / raw)
To: Ritesh Harjani (IBM), fstests
Cc: linux-ext4, linux-xfs, ojaswin, djwong, zlang
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
>> 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")
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE
2025-01-13 5:59 ` Zorro Lang
@ 2025-01-13 8:52 ` Nirjhar Roy (IBM)
2025-01-13 13:11 ` Zorro Lang
0 siblings, 1 reply; 11+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-01-13 8:52 UTC (permalink / raw)
To: Zorro Lang
Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong,
zlang
On 1/13/25 11:29, Zorro Lang wrote:
> On Sun, Jan 12, 2025 at 03:21:51PM +0000, Nirjhar Roy (IBM) wrote:
>> Bug Description:
>>
>> _test_mount function is failing with the following error:
>> ./common/rc: line 4716: _xfs_prepare_for_eio_shutdown: command not found
>> 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()
>>
>> 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
> The _source_specific_fs is called when importing common/rc file:
>
> # check for correct setup and source the $FSTYP specific functions now
> _source_specific_fs $FSTYP
>
> From the code logic of check script:
>
> if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
> echo "RECREATING -- $FSTYP on $TEST_DEV"
> _test_unmount 2> /dev/null
> if ! _test_mkfs >$tmp.err 2>&1
> then
> echo "our local _test_mkfs routine ..."
> cat $tmp.err
> echo "check: failed to mkfs \$TEST_DEV using specified options"
> status=1
> exit
> fi
> if ! _test_mount
> then
> echo "check: failed to mount $TEST_DEV on $TEST_DIR"
> status=1
> exit
> fi
> # TEST_DEV has been recreated, previous FSTYP derived from
> # TEST_DEV could be changed, source common/rc again with
> # correct FSTYP to get FSTYP specific configs, e.g. common/xfs
> . common/rc
> ^^^^^^^^^^^
> we import common/rc at here.
>
> So I'm wondering if we can move this line upward, to fix the problem you
> hit (and don't bring in regression :) Does that help?
>
> Thanks,
> Zorro
Okay so we can move ". common/rc" upward and then remove the following
from "check" file:
if ! _test_mount
then
echo "check: failed to mount $TEST_DEV on $TEST_DIR"
status=1
exit
fi
since . common/rc will call init_rc() in the end, which does a
_test_mount(). Do you agree with this (Zorro and Ritesh)?
I can make the changes and send a v2?
--NR
>
>
>> if ! _test_mkfs >$tmp.err 2>&1
>> then
>> echo "our local _test_mkfs routine ..."
>> --
>> 2.34.1
>>
>>
--
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE
2025-01-13 8:52 ` Nirjhar Roy (IBM)
@ 2025-01-13 13:11 ` Zorro Lang
2025-01-13 15:33 ` Ritesh Harjani
2025-01-15 5:07 ` Nirjhar Roy (IBM)
0 siblings, 2 replies; 11+ messages in thread
From: Zorro Lang @ 2025-01-13 13:11 UTC (permalink / raw)
To: Nirjhar Roy (IBM)
Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong,
zlang
On Mon, Jan 13, 2025 at 02:22:20PM +0530, Nirjhar Roy (IBM) wrote:
>
> On 1/13/25 11:29, Zorro Lang wrote:
> > On Sun, Jan 12, 2025 at 03:21:51PM +0000, Nirjhar Roy (IBM) wrote:
> > > Bug Description:
> > >
> > > _test_mount function is failing with the following error:
> > > ./common/rc: line 4716: _xfs_prepare_for_eio_shutdown: command not found
> > > 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()
> > >
> > > 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
> > The _source_specific_fs is called when importing common/rc file:
> >
> > # check for correct setup and source the $FSTYP specific functions now
> > _source_specific_fs $FSTYP
> >
> > From the code logic of check script:
> >
> > if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
> > echo "RECREATING -- $FSTYP on $TEST_DEV"
> > _test_unmount 2> /dev/null
> > if ! _test_mkfs >$tmp.err 2>&1
> > then
> > echo "our local _test_mkfs routine ..."
> > cat $tmp.err
> > echo "check: failed to mkfs \$TEST_DEV using specified options"
> > status=1
> > exit
> > fi
> > if ! _test_mount
> > then
> > echo "check: failed to mount $TEST_DEV on $TEST_DIR"
> > status=1
> > exit
> > fi
> > # TEST_DEV has been recreated, previous FSTYP derived from
> > # TEST_DEV could be changed, source common/rc again with
> > # correct FSTYP to get FSTYP specific configs, e.g. common/xfs
> > . common/rc
> > ^^^^^^^^^^^
> > we import common/rc at here.
> >
> > So I'm wondering if we can move this line upward, to fix the problem you
> > hit (and don't bring in regression :) Does that help?
> >
> > Thanks,
> > Zorro
>
> Okay so we can move ". common/rc" upward and then remove the following from
> "check" file:
>
> if ! _test_mount
> then
> echo "check: failed to mount $TEST_DEV on $TEST_DIR"
> status=1
> exit
> fi
>
> since . common/rc will call init_rc() in the end, which does a
> _test_mount(). Do you agree with this (Zorro and Ritesh)?
>
> I can make the changes and send a v2?
Hmm... the _init_rc doesn't do _test_mkfs, so you might need to do
". common/rc" after "_test_mkfs", rather than "_test_unmount".
By checking the _init_rc, I think it can replace the _test_mount you metioned
above. Some details might need more testing, to make sure we didn't miss
anything wrong:)
Any review points from others?
Thanks,
Zorro
>
> --NR
>
> >
> >
> > > if ! _test_mkfs >$tmp.err 2>&1
> > > then
> > > echo "our local _test_mkfs routine ..."
> > > --
> > > 2.34.1
> > >
> > >
> --
> Nirjhar Roy
> Linux Kernel Developer
> IBM, Bangalore
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE
2025-01-13 13:11 ` Zorro Lang
@ 2025-01-13 15:33 ` Ritesh Harjani
2025-01-15 5:10 ` Nirjhar Roy (IBM)
2025-01-15 5:07 ` Nirjhar Roy (IBM)
1 sibling, 1 reply; 11+ messages in thread
From: Ritesh Harjani @ 2025-01-13 15:33 UTC (permalink / raw)
To: Zorro Lang, Nirjhar Roy (IBM)
Cc: fstests, linux-ext4, linux-xfs, ojaswin, djwong, zlang
Zorro Lang <zlang@redhat.com> writes:
> On Mon, Jan 13, 2025 at 02:22:20PM +0530, Nirjhar Roy (IBM) wrote:
>>
>> On 1/13/25 11:29, Zorro Lang wrote:
>> > On Sun, Jan 12, 2025 at 03:21:51PM +0000, Nirjhar Roy (IBM) wrote:
>> > > Bug Description:
>> > >
>> > > _test_mount function is failing with the following error:
>> > > ./common/rc: line 4716: _xfs_prepare_for_eio_shutdown: command not found
>> > > 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()
>> > >
>> > > 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
>> > The _source_specific_fs is called when importing common/rc file:
>> >
>> > # check for correct setup and source the $FSTYP specific functions now
>> > _source_specific_fs $FSTYP
>> >
>> > From the code logic of check script:
>> >
>> > if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
>> > echo "RECREATING -- $FSTYP on $TEST_DEV"
>> > _test_unmount 2> /dev/null
>> > if ! _test_mkfs >$tmp.err 2>&1
>> > then
>> > echo "our local _test_mkfs routine ..."
>> > cat $tmp.err
>> > echo "check: failed to mkfs \$TEST_DEV using specified options"
>> > status=1
>> > exit
>> > fi
>> > if ! _test_mount
>> > then
>> > echo "check: failed to mount $TEST_DEV on $TEST_DIR"
>> > status=1
>> > exit
>> > fi
>> > # TEST_DEV has been recreated, previous FSTYP derived from
>> > # TEST_DEV could be changed, source common/rc again with
>> > # correct FSTYP to get FSTYP specific configs, e.g. common/xfs
>> > . common/rc
>> > ^^^^^^^^^^^
>> > we import common/rc at here.
>> >
>> > So I'm wondering if we can move this line upward, to fix the problem you
>> > hit (and don't bring in regression :) Does that help?
>> >
>> > Thanks,
>> > Zorro
>>
>> Okay so we can move ". common/rc" upward and then remove the following from
>> "check" file:
>>
>> if ! _test_mount
>> then
>> echo "check: failed to mount $TEST_DEV on $TEST_DIR"
>> status=1
>> exit
>> fi
>>
>> since . common/rc will call init_rc() in the end, which does a
>> _test_mount(). Do you agree with this (Zorro and Ritesh)?
>>
>> I can make the changes and send a v2?
>
> Hmm... the _init_rc doesn't do _test_mkfs,
Yes, I had noticed that problem. So I felt sourcing fs specific file
before _test_mkfs should be ok.
> so you might need to do ". common/rc" after "_test_mkfs", rather than "_test_unmount".
>
> By checking the _init_rc, I think it can replace the _test_mount you metioned
> above. Some details might need more testing, to make sure we didn't miss
> anything wrong:)
If moving . common/rc above _test_mount works, than that is a better
approach than sourcing fs specific config file twice.
-ritesh
>
> Any review points from others?
>
> Thanks,
> Zorro
>
>>
>> --NR
>>
>> >
>> >
>> > > if ! _test_mkfs >$tmp.err 2>&1
>> > > then
>> > > echo "our local _test_mkfs routine ..."
>> > > --
>> > > 2.34.1
>> > >
>> > >
>> --
>> Nirjhar Roy
>> Linux Kernel Developer
>> IBM, Bangalore
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE
2025-01-13 5:59 ` Nirjhar Roy (IBM)
@ 2025-01-13 15:39 ` Ritesh Harjani
2025-01-15 5:06 ` Nirjhar Roy (IBM)
0 siblings, 1 reply; 11+ messages in thread
From: Ritesh Harjani @ 2025-01-13 15:39 UTC (permalink / raw)
To: Nirjhar Roy (IBM), fstests; +Cc: linux-ext4, linux-xfs, ojaswin, djwong, zlang
"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?
Ok, why don't revert the above commit and see if the revert fixes the
issue for you.
https://www.kernel.org/doc/html/v4.10/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
-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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE
2025-01-13 15:39 ` Ritesh Harjani
@ 2025-01-15 5:06 ` Nirjhar Roy (IBM)
0 siblings, 0 replies; 11+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-01-15 5:06 UTC (permalink / raw)
To: Ritesh Harjani (IBM), fstests
Cc: linux-ext4, linux-xfs, ojaswin, djwong, zlang
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE
2025-01-13 13:11 ` Zorro Lang
2025-01-13 15:33 ` Ritesh Harjani
@ 2025-01-15 5:07 ` Nirjhar Roy (IBM)
1 sibling, 0 replies; 11+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-01-15 5:07 UTC (permalink / raw)
To: Zorro Lang
Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong,
zlang
On 1/13/25 18:41, Zorro Lang wrote:
> On Mon, Jan 13, 2025 at 02:22:20PM +0530, Nirjhar Roy (IBM) wrote:
>> On 1/13/25 11:29, Zorro Lang wrote:
>>> On Sun, Jan 12, 2025 at 03:21:51PM +0000, Nirjhar Roy (IBM) wrote:
>>>> Bug Description:
>>>>
>>>> _test_mount function is failing with the following error:
>>>> ./common/rc: line 4716: _xfs_prepare_for_eio_shutdown: command not found
>>>> 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()
>>>>
>>>> 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
>>> The _source_specific_fs is called when importing common/rc file:
>>>
>>> # check for correct setup and source the $FSTYP specific functions now
>>> _source_specific_fs $FSTYP
>>>
>>> From the code logic of check script:
>>>
>>> if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
>>> echo "RECREATING -- $FSTYP on $TEST_DEV"
>>> _test_unmount 2> /dev/null
>>> if ! _test_mkfs >$tmp.err 2>&1
>>> then
>>> echo "our local _test_mkfs routine ..."
>>> cat $tmp.err
>>> echo "check: failed to mkfs \$TEST_DEV using specified options"
>>> status=1
>>> exit
>>> fi
>>> if ! _test_mount
>>> then
>>> echo "check: failed to mount $TEST_DEV on $TEST_DIR"
>>> status=1
>>> exit
>>> fi
>>> # TEST_DEV has been recreated, previous FSTYP derived from
>>> # TEST_DEV could be changed, source common/rc again with
>>> # correct FSTYP to get FSTYP specific configs, e.g. common/xfs
>>> . common/rc
>>> ^^^^^^^^^^^
>>> we import common/rc at here.
>>>
>>> So I'm wondering if we can move this line upward, to fix the problem you
>>> hit (and don't bring in regression :) Does that help?
>>>
>>> Thanks,
>>> Zorro
>> Okay so we can move ". common/rc" upward and then remove the following from
>> "check" file:
>>
>> if ! _test_mount
>> then
>> echo "check: failed to mount $TEST_DEV on $TEST_DIR"
>> status=1
>> exit
>> fi
>>
>> since . common/rc will call init_rc() in the end, which does a
>> _test_mount(). Do you agree with this (Zorro and Ritesh)?
>>
>> I can make the changes and send a v2?
> Hmm... the _init_rc doesn't do _test_mkfs, so you might need to do
> ". common/rc" after "_test_mkfs", rather than "_test_unmount".
Yes, we should place ". common/rc" after, _test_mkfs.
>
> By checking the _init_rc, I think it can replace the _test_mount you metioned
> above. Some details might need more testing, to make sure we didn't miss
> anything wrong:)
Yes, makes sense.
>
> Any review points from others?
>
> Thanks,
> Zorro
>
>> --NR
>>
>>>
>>>> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE
2025-01-13 15:33 ` Ritesh Harjani
@ 2025-01-15 5:10 ` Nirjhar Roy (IBM)
0 siblings, 0 replies; 11+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-01-15 5:10 UTC (permalink / raw)
To: Ritesh Harjani (IBM), Zorro Lang
Cc: fstests, linux-ext4, linux-xfs, ojaswin, djwong, zlang
On 1/13/25 21:03, Ritesh Harjani (IBM) wrote:
> Zorro Lang <zlang@redhat.com> writes:
>
>> On Mon, Jan 13, 2025 at 02:22:20PM +0530, Nirjhar Roy (IBM) wrote:
>>> On 1/13/25 11:29, Zorro Lang wrote:
>>>> On Sun, Jan 12, 2025 at 03:21:51PM +0000, Nirjhar Roy (IBM) wrote:
>>>>> Bug Description:
>>>>>
>>>>> _test_mount function is failing with the following error:
>>>>> ./common/rc: line 4716: _xfs_prepare_for_eio_shutdown: command not found
>>>>> 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()
>>>>>
>>>>> 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
>>>> The _source_specific_fs is called when importing common/rc file:
>>>>
>>>> # check for correct setup and source the $FSTYP specific functions now
>>>> _source_specific_fs $FSTYP
>>>>
>>>> From the code logic of check script:
>>>>
>>>> if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
>>>> echo "RECREATING -- $FSTYP on $TEST_DEV"
>>>> _test_unmount 2> /dev/null
>>>> if ! _test_mkfs >$tmp.err 2>&1
>>>> then
>>>> echo "our local _test_mkfs routine ..."
>>>> cat $tmp.err
>>>> echo "check: failed to mkfs \$TEST_DEV using specified options"
>>>> status=1
>>>> exit
>>>> fi
>>>> if ! _test_mount
>>>> then
>>>> echo "check: failed to mount $TEST_DEV on $TEST_DIR"
>>>> status=1
>>>> exit
>>>> fi
>>>> # TEST_DEV has been recreated, previous FSTYP derived from
>>>> # TEST_DEV could be changed, source common/rc again with
>>>> # correct FSTYP to get FSTYP specific configs, e.g. common/xfs
>>>> . common/rc
>>>> ^^^^^^^^^^^
>>>> we import common/rc at here.
>>>>
>>>> So I'm wondering if we can move this line upward, to fix the problem you
>>>> hit (and don't bring in regression :) Does that help?
>>>>
>>>> Thanks,
>>>> Zorro
>>> Okay so we can move ". common/rc" upward and then remove the following from
>>> "check" file:
>>>
>>> if ! _test_mount
>>> then
>>> echo "check: failed to mount $TEST_DEV on $TEST_DIR"
>>> status=1
>>> exit
>>> fi
>>>
>>> since . common/rc will call init_rc() in the end, which does a
>>> _test_mount(). Do you agree with this (Zorro and Ritesh)?
>>>
>>> I can make the changes and send a v2?
>> Hmm... the _init_rc doesn't do _test_mkfs,
> Yes, I had noticed that problem. So I felt sourcing fs specific file
> before _test_mkfs should be ok.
>
>> so you might need to do ". common/rc" after "_test_mkfs", rather than "_test_unmount".
>>
>> By checking the _init_rc, I think it can replace the _test_mount you metioned
>> above. Some details might need more testing, to make sure we didn't miss
>> anything wrong:)
> If moving . common/rc above _test_mount works, than that is a better
> approach than sourcing fs specific config file twice.
Yes, moving the ". common/rc" just after _test_mkfs and removing the
_test_mount after fixes it the issue. I will do additional testing
before sending a v2.
--NR
>
>
> -ritesh
>
>> Any review points from others?
>>
>> Thanks,
>> Zorro
>>
>>> --NR
>>>
>>>>
>>>>> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-01-15 5:10 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-12 15:21 [PATCH] check: Fix fs specfic imports when $FSTYPE!=$OLD_FSTYPE Nirjhar Roy (IBM)
2025-01-12 19:41 ` Ritesh Harjani
2025-01-13 5:59 ` Nirjhar Roy (IBM)
2025-01-13 15:39 ` Ritesh Harjani
2025-01-15 5:06 ` Nirjhar Roy (IBM)
2025-01-13 5:59 ` Zorro Lang
2025-01-13 8:52 ` Nirjhar Roy (IBM)
2025-01-13 13:11 ` Zorro Lang
2025-01-13 15:33 ` Ritesh Harjani
2025-01-15 5:10 ` Nirjhar Roy (IBM)
2025-01-15 5:07 ` Nirjhar Roy (IBM)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).