| Message ID | 20231221013914.3108026-1-song@kernel.org (mailing list archive) |
|---|---|
| State | Under Review, archived |
| Headers | show |
| Series | [mdadm] tests: Gate tests for linear flavor with variable LINEAR | expand |
On 21.12.2023 02:39, Song Liu wrote: > linear flavor is being removed in the kernel [1], so tests for the linear > flavor will fail. Gate tests for linear flavor with LINEAR=yes, so that we > can still run these tests for older kernels. > > [1] https://lore.kernel.org/linux-raid/20231214222107.2016042-1-song@kernel.org/ > Signed-off-by: Song Liu <song@kernel.org> > --- > tests/00linear | 5 +++++ > tests/00names | 8 +++++++- > tests/00raid0 | 4 ++++ > tests/00readonly | 8 +++++++- > tests/02lineargrow | 5 +++++ > tests/03assem-incr | 8 +++++++- > tests/03r0assem | 4 ++++ > tests/04r0update | 6 ++++++ > 8 files changed, 45 insertions(+), 3 deletions(-) > > diff --git a/tests/00linear b/tests/00linear > index e3ac6555c9dd..5a1160851af2 100644 > --- a/tests/00linear > +++ b/tests/00linear > @@ -1,6 +1,11 @@ > > # create a simple linear > > +if [ "$LINEAR" != "yes" ]; then > + echo -ne 'skipping... ' > + exit 0 > +fi > + > mdadm -CR $md0 -l linear -n3 $dev0 $dev1 $dev2 > check linear > testdev $md0 3 $mdsize2_l 1 > diff --git a/tests/00names b/tests/00names > index 7a066d8fb2b7..d996befc5e8b 100644 > --- a/tests/00names > +++ b/tests/00names > @@ -4,7 +4,13 @@ set -x -e > conf=$targetdir/mdadm.conf > echo "CREATE names=yes" > $conf > > -for i in linear raid0 raid1 raid4 raid5 raid6 > +levels=(raid0 raid1 raid4 raid5 raid6) > + > +if [ "$LINEAR" == "yes" ]; then > + levels+=( linear ) > +fi > + > +for i in ${levels[@]} > do > mdadm -CR --config $conf /dev/md/$i -l $i -n 4 $dev4 $dev3 $dev2 $dev1 > check $i > diff --git a/tests/00raid0 b/tests/00raid0 > index 9b8896cbdc52..6407c320fd65 100644 > --- a/tests/00raid0 > +++ b/tests/00raid0 > @@ -16,6 +16,10 @@ check raid0 > testdev $md0 5 $size 512 > mdadm -S $md0 > > +if [ "$LINEAR" != "yes" ]; then > + echo -ne 'skipping... ' > + exit 0 > +fi > > # now same again with different chunk size > for chunk in 4 32 256 > diff --git a/tests/00readonly b/tests/00readonly > index afe243b3a0b0..80b63629e4f9 100644 > --- a/tests/00readonly > +++ b/tests/00readonly > @@ -1,8 +1,14 @@ > #!/bin/bash > > +levels=(raid0 raid1 raid4 raid5 raid6 raid10) > + > +if [ "$LINEAR" == "yes" ]; then > + levels+=( linear ) > +fi > + > for metadata in 0.9 1.0 1.1 1.2 > do > - for level in linear raid0 raid1 raid4 raid5 raid6 raid10 > + for level in ${levels[@]} > do > if [[ $metadata == "0.9" && $level == "raid0" ]]; > then > diff --git a/tests/02lineargrow b/tests/02lineargrow > index 595bf9f20802..d17e2326d13f 100644 > --- a/tests/02lineargrow > +++ b/tests/02lineargrow > @@ -1,6 +1,11 @@ > > # create a liner array, and add more drives to to. > > +if [ "$LINEAR" != "yes" ]; then > + echo -ne 'skipping... ' > + exit 0 > +fi > + > for e in 0.90 1 1.1 1.2 > do > case $e in > diff --git a/tests/03assem-incr b/tests/03assem-incr > index f10a1a48ee5c..38880a7fed10 100644 > --- a/tests/03assem-incr > +++ b/tests/03assem-incr > @@ -6,7 +6,13 @@ set -x -e > # Here just test that a partly "-I" assembled array can > # be completed with "-A" > > -for l in 0 1 5 linear > +levels=(raid0 raid1 raid5) > + > +if [ "$LINEAR" == "yes" ]; then > + levels+=( linear ) > +fi > + > +for l in ${levels[@]} > do > mdadm -CR $md0 -l $l -n5 $dev0 $dev1 $dev2 $dev3 $dev4 --assume-clean > mdadm -S md0 > diff --git a/tests/03r0assem b/tests/03r0assem > index 44df06456233..f7c29e8c1ab6 100644 > --- a/tests/03r0assem > +++ b/tests/03r0assem > @@ -64,6 +64,10 @@ mdadm --assemble --scan --config=$conf $md2 > $tst > mdadm -S $md2 > > +if [ "$LINEAR" != "yes" ]; then > + echo -ne 'skipping... ' > + exit 0 > +fi > > ### Now for version 0... > > diff --git a/tests/04r0update b/tests/04r0update > index b95efb06c761..c495f34a0a79 100644 > --- a/tests/04r0update > +++ b/tests/04r0update > @@ -1,5 +1,11 @@ > > # create a raid0, re-assemble with a different super-minor > + > +if [ "$LINEAR" != "yes" ]; then > + echo -ne 'skipping... ' > + exit 0 > +fi > + > mdadm -CR -e 0.90 $md0 -llinear -n3 $dev0 $dev1 $dev2 > testdev $md0 3 $mdsize0 1 > minor1=`mdadm -E $dev0 | sed -n -e 's/.*Preferred Minor : //p'` Hi Song, this approach looks a bit dirty to me as it's omitting what's already in the test suite. I would prefer adding additional param rather than setting environment variable, so test execution flow stays unified (as far as I'm aware we do not use flags for now). Adding param is also a good excuse to explain why linear is not tested by default in "--help". Another thing is "--raidtype=linear" option, is probably redundant now. Thanks, Mateusz
On Thu, Dec 28, 2023 at 6:28 AM Mateusz Kusiak <mateusz.kusiak@linux.intel.com> wrote: [...] > > # create a raid0, re-assemble with a different super-minor > + > +if [ "$LINEAR" != "yes" ]; then > + echo -ne 'skipping... ' > + exit 0 > +fi > + > mdadm -CR -e 0.90 $md0 -llinear -n3 $dev0 $dev1 $dev2 > testdev $md0 3 $mdsize0 1 > minor1=`mdadm -E $dev0 | sed -n -e 's/.*Preferred Minor : //p'` > > Hi Song, this approach looks a bit dirty to me as it's omitting what's already in the test suite. I would prefer adding additional param rather than setting environment variable, so test execution flow stays unified (as far as I'm aware we do not use flags for now). Adding param is also a good excuse to explain why linear is not tested by default in "--help". We have something similar to this in tests/00multipath, so I used this pattern. Thanks, Song > > Another thing is "--raidtype=linear" option, is probably redundant now. > > Thanks, Mateusz
On Fri, 29 Dec 2023 00:12:43 -0800 Song Liu <song@kernel.org> wrote: > On Thu, Dec 28, 2023 at 6:28 AM Mateusz Kusiak > <mateusz.kusiak@linux.intel.com> wrote: > [...] > > > > # create a raid0, re-assemble with a different super-minor > > + > > +if [ "$LINEAR" != "yes" ]; then > > + echo -ne 'skipping... ' > > + exit 0 > > +fi > > + > > mdadm -CR -e 0.90 $md0 -llinear -n3 $dev0 $dev1 $dev2 > > testdev $md0 3 $mdsize0 1 > > minor1=`mdadm -E $dev0 | sed -n -e 's/.*Preferred Minor : //p'` > > > > Hi Song, this approach looks a bit dirty to me as it's omitting what's > > already in the test suite. I would prefer adding additional param rather > > than setting environment variable, so test execution flow stays unified (as > > far as I'm aware we do not use flags for now). Adding param is also a good > > excuse to explain why linear is not tested by default in "--help". > > We have something similar to this in tests/00multipath, so I used > this pattern. Hi Song, Please note that MULTIPATH env variable is handled inside testing framework. This variable is set to yes if multipath module is available. It can be also disabled by --disable-multipath. It is described in help. it is invoked by main() -> do_setup() -> check_env(). Now we need to handle both linear and multipath same way. There are two options: - disable them by default and add --enable-multipath and --enable-linear options, - make linear to work same as multipath now, add --disable-linear flag. I don't have strong preference. Both ways are acceptable for me. You disabled testing those levels by default. It looks good to me but we need to explain it little in message to give user clue why those levels are skipped. Test result should be possible straightforward for less experienced users so please expand messages, like: "Skipping, \$LINEAR set to \"$LINEAR\"" It i just an example, I didn't test it. The final message depends on option you will choose. Mdadm is a kind of user interface for MD driver. I expect that not only developers are using this test suite. We need to keep it easy to use, messages should be meaningful and helpful. > > > > Another thing is "--raidtype=linear" option, is probably redundant now. > > Env variable are better implemented now than this --raidtype option. There is more work to do to make --raidtype fully valid because --raidtype uses filtering by test name. Test may define set of levels used, even if the name doesn't point to any level. So yes, I think that we can eventually remove --raidtype=linear as it is not really useful but I give it up to Song. Thanks, Mariusz
Hi Mariusz, Thanks for these explanations. On Fri, Jan 5, 2024 at 1:58 AM Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com> wrote: > [...] > > It i just an example, I didn't test it. The final message depends on > option you will choose. > > Mdadm is a kind of user interface for MD driver. I expect that not only > developers are using this test suite. We need to keep it easy to use, messages > should be meaningful and helpful. > > > > > > > Another thing is "--raidtype=linear" option, is probably redundant now. > > > > > Env variable are better implemented now than this --raidtype option. There is > more work to do to make --raidtype fully valid because --raidtype uses > filtering by test name. Test may define set of levels used, even if the name > doesn't point to any level. So yes, I think that we can eventually remove > --raidtype=linear as it is not really useful but I give it up to Song. How about we add something like the following on top of this patch? Thanks, Song diff --git i/test w/test index b244453b1cec..49a36c3b8ef2 100755 --- i/test +++ w/test @@ -140,6 +140,7 @@ do_help() { --raidtype= raid0|linear|raid1|raid456|raid10|ddf|imsm --disable-multipath Disable any tests involving multipath --disable-integrity Disable slow tests of RAID[56] consistency + --disable-linear Disable any tests involving linear --logdir=directory Directory to save all logfiles in --save-logs Usually use with --logdir together --keep-going | --no-error Don't stop on error, ie. run all tests @@ -255,6 +256,9 @@ parse_args() { --disable-integrity ) unset INTEGRITY ;; + --disable-linear ) + unset LINEAR + ;; --dev=* ) case ${i##*=} in loop ) diff --git i/tests/func.sh w/tests/func.sh index 5053b0121f1d..d7561f3c20cf 100644 --- i/tests/func.sh +++ w/tests/func.sh @@ -123,6 +123,14 @@ check_env() { modprobe multipath 2> /dev/null grep -sq 'Personalities : .*multipath' /proc/mdstat && MULTIPATH="yes" + + # Check whether to run linear tests + modprobe linear 2> /dev/null + grep -sq 'Personalities : .*linear' /proc/mdstat && + LINEAR="yes" + if [ "$LINEAR" != "yes" ]; then + echo "test: skipping tests for linear, which is removed in upstream 6.8+ kernels" + fi } do_setup() {
On Fri, 5 Jan 2024 15:18:07 -0800 Song Liu <song@kernel.org> wrote: > Hi Mariusz, > > Thanks for these explanations. > > On Fri, Jan 5, 2024 at 1:58 AM Mariusz Tkaczyk > <mariusz.tkaczyk@linux.intel.com> wrote: > > > [...] > > > > It i just an example, I didn't test it. The final message depends on > > option you will choose. > > > > Mdadm is a kind of user interface for MD driver. I expect that not only > > developers are using this test suite. We need to keep it easy to use, > > messages should be meaningful and helpful. > > > > > > > > > > Another thing is "--raidtype=linear" option, is probably redundant now. > > > > > > > > Env variable are better implemented now than this --raidtype option. There > > is more work to do to make --raidtype fully valid because --raidtype uses > > filtering by test name. Test may define set of levels used, even if the name > > doesn't point to any level. So yes, I think that we can eventually remove > > --raidtype=linear as it is not really useful but I give it up to Song. > > How about we add something like the following on top of this patch? > > Thanks, > Song > > diff --git i/test w/test > index b244453b1cec..49a36c3b8ef2 100755 > --- i/test > +++ w/test > @@ -140,6 +140,7 @@ do_help() { > --raidtype= > raid0|linear|raid1|raid456|raid10|ddf|imsm > --disable-multipath Disable any tests > involving multipath > --disable-integrity Disable slow tests of > RAID[56] consistency > + --disable-linear Disable any tests involving linear > --logdir=directory Directory to save all logfiles in > --save-logs Usually use with --logdir together > --keep-going | --no-error Don't stop on error, ie. > run all tests > @@ -255,6 +256,9 @@ parse_args() { > --disable-integrity ) > unset INTEGRITY > ;; > + --disable-linear ) > + unset LINEAR > + ;; > --dev=* ) > case ${i##*=} in > loop ) > diff --git i/tests/func.sh w/tests/func.sh > index 5053b0121f1d..d7561f3c20cf 100644 > --- i/tests/func.sh > +++ w/tests/func.sh > @@ -123,6 +123,14 @@ check_env() { > modprobe multipath 2> /dev/null > grep -sq 'Personalities : .*multipath' /proc/mdstat && > MULTIPATH="yes" > + > + # Check whether to run linear tests > + modprobe linear 2> /dev/null > + grep -sq 'Personalities : .*linear' /proc/mdstat && > + LINEAR="yes" > + if [ "$LINEAR" != "yes" ]; then > + echo "test: skipping tests for linear, which is > removed in upstream 6.8+ kernels" > + fi > } > > do_setup() { Approach looks good to me but multipath case handling is missed. Can we add a message for Multipath? Thanks! Mariusz
diff --git a/tests/00linear b/tests/00linear index e3ac6555c9dd..5a1160851af2 100644 --- a/tests/00linear +++ b/tests/00linear @@ -1,6 +1,11 @@ # create a simple linear +if [ "$LINEAR" != "yes" ]; then + echo -ne 'skipping... ' + exit 0 +fi + mdadm -CR $md0 -l linear -n3 $dev0 $dev1 $dev2 check linear testdev $md0 3 $mdsize2_l 1 diff --git a/tests/00names b/tests/00names index 7a066d8fb2b7..d996befc5e8b 100644 --- a/tests/00names +++ b/tests/00names @@ -4,7 +4,13 @@ set -x -e conf=$targetdir/mdadm.conf echo "CREATE names=yes" > $conf -for i in linear raid0 raid1 raid4 raid5 raid6 +levels=(raid0 raid1 raid4 raid5 raid6) + +if [ "$LINEAR" == "yes" ]; then + levels+=( linear ) +fi + +for i in ${levels[@]} do mdadm -CR --config $conf /dev/md/$i -l $i -n 4 $dev4 $dev3 $dev2 $dev1 check $i diff --git a/tests/00raid0 b/tests/00raid0 index 9b8896cbdc52..6407c320fd65 100644 --- a/tests/00raid0 +++ b/tests/00raid0 @@ -16,6 +16,10 @@ check raid0 testdev $md0 5 $size 512 mdadm -S $md0 +if [ "$LINEAR" != "yes" ]; then + echo -ne 'skipping... ' + exit 0 +fi # now same again with different chunk size for chunk in 4 32 256 diff --git a/tests/00readonly b/tests/00readonly index afe243b3a0b0..80b63629e4f9 100644 --- a/tests/00readonly +++ b/tests/00readonly @@ -1,8 +1,14 @@ #!/bin/bash +levels=(raid0 raid1 raid4 raid5 raid6 raid10) + +if [ "$LINEAR" == "yes" ]; then + levels+=( linear ) +fi + for metadata in 0.9 1.0 1.1 1.2 do - for level in linear raid0 raid1 raid4 raid5 raid6 raid10 + for level in ${levels[@]} do if [[ $metadata == "0.9" && $level == "raid0" ]]; then diff --git a/tests/02lineargrow b/tests/02lineargrow index 595bf9f20802..d17e2326d13f 100644 --- a/tests/02lineargrow +++ b/tests/02lineargrow @@ -1,6 +1,11 @@ # create a liner array, and add more drives to to. +if [ "$LINEAR" != "yes" ]; then + echo -ne 'skipping... ' + exit 0 +fi + for e in 0.90 1 1.1 1.2 do case $e in diff --git a/tests/03assem-incr b/tests/03assem-incr index f10a1a48ee5c..38880a7fed10 100644 --- a/tests/03assem-incr +++ b/tests/03assem-incr @@ -6,7 +6,13 @@ set -x -e # Here just test that a partly "-I" assembled array can # be completed with "-A" -for l in 0 1 5 linear +levels=(raid0 raid1 raid5) + +if [ "$LINEAR" == "yes" ]; then + levels+=( linear ) +fi + +for l in ${levels[@]} do mdadm -CR $md0 -l $l -n5 $dev0 $dev1 $dev2 $dev3 $dev4 --assume-clean mdadm -S md0 diff --git a/tests/03r0assem b/tests/03r0assem index 44df06456233..f7c29e8c1ab6 100644 --- a/tests/03r0assem +++ b/tests/03r0assem @@ -64,6 +64,10 @@ mdadm --assemble --scan --config=$conf $md2 $tst mdadm -S $md2 +if [ "$LINEAR" != "yes" ]; then + echo -ne 'skipping... ' + exit 0 +fi ### Now for version 0... diff --git a/tests/04r0update b/tests/04r0update index b95efb06c761..c495f34a0a79 100644 --- a/tests/04r0update +++ b/tests/04r0update @@ -1,5 +1,11 @@ # create a raid0, re-assemble with a different super-minor + +if [ "$LINEAR" != "yes" ]; then + echo -ne 'skipping... ' + exit 0 +fi + mdadm -CR -e 0.90 $md0 -llinear -n3 $dev0 $dev1 $dev2 testdev $md0 3 $mdsize0 1 minor1=`mdadm -E $dev0 | sed -n -e 's/.*Preferred Minor : //p'`
linear flavor is being removed in the kernel [1], so tests for the linear flavor will fail. Gate tests for linear flavor with LINEAR=yes, so that we can still run these tests for older kernels. [1] https://lore.kernel.org/linux-raid/20231214222107.2016042-1-song@kernel.org/ Signed-off-by: Song Liu <song@kernel.org> --- tests/00linear | 5 +++++ tests/00names | 8 +++++++- tests/00raid0 | 4 ++++ tests/00readonly | 8 +++++++- tests/02lineargrow | 5 +++++ tests/03assem-incr | 8 +++++++- tests/03r0assem | 4 ++++ tests/04r0update | 6 ++++++ 8 files changed, 45 insertions(+), 3 deletions(-)