Re: [LTP] [PATCH] lib: Add checking of setup params in kernel command line

* Re: [LTP] [PATCH] lib: Add checking of setup params in kernel command line
@ 2022-09-15 11:49 zhaogongyi via ltp
  2022-09-15 13:01 ` Cyril Hrubis
  0 siblings, 1 reply; 4+ messages in thread
From: zhaogongyi via ltp @ 2022-09-15 11:49 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

Hi!

> 
> Hi!
> > There are some test cases that need to check the configuration of
> > setup params, so we add tst_kernel_cmdline_check() to check it in
> > do_setup().
> 
> Can you elaborate on that? Which tests and what for?

In linux kernel, there are many setup params configured throng command line, for example, PSI is not enabled by default,
We need to set psi=1 in command line to enable it.

As far as I know, there is no testcase referenced setup param now. it can be considered as a possible use in future.

> 
> > Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
> > ---
> >  include/tst_kernel.h |  9 +++++++++
> >  include/tst_test.h   |  3 +++
> >  lib/tst_kernel.c     | 21 +++++++++++++++++++++
> >  lib/tst_test.c       |  9 +++++++++
> >  4 files changed, 42 insertions(+)
> >
> > diff --git a/include/tst_kernel.h b/include/tst_kernel.h index
> > 9e17bb71e..874ec3bf4 100644
> > --- a/include/tst_kernel.h
> > +++ b/include/tst_kernel.h
> > @@ -20,4 +20,13 @@ int tst_kernel_bits(void);
> >   */
> >  int tst_check_driver(const char *driver);
> >
> > +/*
> > + * Checks if there is a configuration in the kernel command line.
> > + *
> > + * @param cmd The setup param that need to be checked in command
> line.
> > + * @return Returns 0 if the kernel configured the setup param in
> command line.
> > + * -1 when the setup param is not configured.
> > + */
> > +int tst_kernel_cmdline_check(const char *cmd);
> > +
> >  #endif	/* TST_KERNEL_H__ */
> > diff --git a/include/tst_test.h b/include/tst_test.h index
> > ac52f268c..ba7d538b7 100644
> > --- a/include/tst_test.h
> > +++ b/include/tst_test.h
> > @@ -312,6 +312,9 @@ struct tst_test {
> >
> >  	/* {} terminated array of required CGroup controllers */
> >  	const char *const *needs_cgroup_ctrls;
> > +
> > +	/* NULL terminated array of required kernel cmdlines. */
> > +	const char *const *needs_kernel_cmdlines;
> >  };
> >
> >  /*
> > diff --git a/lib/tst_kernel.c b/lib/tst_kernel.c index
> > ecf4b917e..1e6d18e55 100644
> > --- a/lib/tst_kernel.c
> > +++ b/lib/tst_kernel.c
> > @@ -24,6 +24,8 @@
> >  #include "tst_kernel.h"
> >  #include "old_safe_stdio.h"
> >
> > +#define COMMAND_LINE_SIZE 4096
> > +
> >  static int get_kernel_bits_from_uname(struct utsname *buf)  {
> >  	if (uname(buf)) {
> > @@ -189,3 +191,22 @@ int tst_check_driver(const char *driver)
> >
> >  	return ret;
> >  }
> > +
> > +int tst_kernel_cmdline_check(const char *cmd) {
> > +	char cmdline[COMMAND_LINE_SIZE];
> > +
> > +	FILE *f = fopen("/proc/cmdline", "r");
> > +	if (f == NULL) {
> > +		tst_brkm(TBROK | TERRNO, NULL, "Open /proc/cmdline failed");
> > +		return -1;
> > +	}
> > +
> > +	while (fscanf(f, "%s", cmdline) > 0) {
> > +		if (!strcmp(cmdline, cmd))
> > +			return 0;
> > +	}
> > +
> > +	tst_brkm(TCONF, NULL, "Setup param '%s' is not configured", cmd);
> > +	return -1;
> > +}
> 
> If nothing else this call should really take the NULL terminated array as
> parameter and check for all flags so that we don't have to parse the file
> eac time we check for a flag.

> 
> Also there is no point in doing return from a function that calls tst_brkm().
> The function should just return void in this case.
> 
> > diff --git a/lib/tst_test.c b/lib/tst_test.c index
> > 8ccde1629..7a842c5ae 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -1125,6 +1125,15 @@ static void do_setup(int argc, char *argv[])
> >  	if (tst_test->min_kver)
> >  		check_kver();
> >
> > +	if (tst_test->needs_kernel_cmdlines)
> > +	{
> > +		const char *cmd;
> > +		int i;
> > +
> > +		for (i = 0; (cmd = tst_test->needs_kernel_cmdlines[i]); ++i)
> > +			tst_kernel_cmdline_check(cmd);
> > +	}
> > +
> >  	if (tst_test->supported_archs
> && !tst_is_on_arch(tst_test->supported_archs))
> >  		tst_brk(TCONF, "This arch '%s' is not supported for test!",
> > tst_arch.name);
> >
> > --
> > 2.17.1
> >
> >
> > --
> > Mailing list info: https://lists.linux.it/listinfo/ltp
> 
> --
> Cyril Hrubis
> chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 4+ messages in thread