Re: [LTP] [PATCH 16/17] lib: tst_test: Remove now unused needs_drivers

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril, Li,

> Hi!
> > > --- a/include/tst_test.h
> > > +++ b/include/tst_test.h
> > > @@ -498,10 +498,6 @@ struct tst_fs {
> > >   *                  to the test temporary directory from the LTP datafiles
> > >   *                  directory.
> > >   *
> > > - * @needs_drivers: A NULL terminated array of kernel modules required to run
> > > - *                 the test. The module has to be build in or present in order
> > > - *                 for the test to run.
> > > - *
> > >   * @save_restore: A {} terminated array of /proc or /sys files that should
> > >   *                saved at the start of the test and restored at the end. See
> > >   *                tst_sys_conf_save() and :ref:`struct tst_path_val` for details.

> > As we add runtime_check and module_check, I think we also need to update
> > the descrition comments of @needs_kconfigs in tst_test.h.


> What about something like this:

> diff --git a/include/tst_test.h b/include/tst_test.h
> index 752688c45..f2a5378cd 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -511,6 +511,13 @@ struct tst_fs {
>   *                  and parenthesis are supported, e.g.
>   *                  "CONFIG_X86_INTEL_UMIP=y | CONFIG_X86_UIMP=y" is evaluated
>   *                  to true if at least one of the options is present.
> + *                  For some config options their presence in the config file
> + *                  is not enough to assure the functionality is present or
> + *                  enabled. E.g. config option may be set to 'm' but the
> + *                  module may not be installed or functionality may be
> + *                  disabled on the kernel command line. The kconfig library
> + *                  has a set of additonal checks for a few config options that

s/additonal/additional/

> + *                  are used in the tests and need additional checks.

(Please consider configuring spell checker in your editor setup, or run make
check as I enabled in our checkpatch.pl configuration check for typos.)

AI would reword it, feel free to use some parts or ignore:

	For some config options their presence in the config file is not enough to assure the functionality is present or enabled.
	E.g. config option may be set to 'm' but the module may not be installed or functionality may be disabled on the kernel command line.

	The kconfig library has a set of additional checks for a few config options that are used in the tests and need additional checks.
	Please rebase before adding the text (I fixed typo in include/tst_test.h).

The removal is obviously ok.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp