On Fri, 29 Dec 2023 00:12:43 -0800 Song Liu <song@xxxxxxxxxx> wrote: > On Thu, Dec 28, 2023 at 6:28 AM Mateusz Kusiak > <mateusz.kusiak@xxxxxxxxxxxxxxx> 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
