public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v2 1/4] lib/tst_test.c: add 'needs_drivers' option with tst_check_drivers cmd
Date: Thu, 13 Sep 2018 14:27:12 +0200	[thread overview]
Message-ID: <20180913122712.GA18036@rei.lan> (raw)
In-Reply-To: <1534764229-26993-1-git-send-email-alexey.kodanev@oracle.com>

Hi!
> +2.2.26 Checking kernel for the driver support
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Someties test needs certain drivers to be available in the kernel and must
> +end with TCONF if any are missing. For this task there is the '.needs_drivers'
> +option which accepts NULL-terminated array of the drivers names.

Maybe we should explicitly say here that this works for both drivers
that are compiled-in and modules, since otherwise this may confuse the
users, especially since we talk about modprobe in the next paragraph.

I would write something as:

"Some tests may need specific kernel drivers, either compiled in, or
loaded as a module. If .need_drivers points to a NULL-terminated array
of kernel module names these are all checked and the test exits with
TCONF on first missing driver."

> +Since it relies on modprobe command, the check will be skipped if the command
> +itself is not available on the system.
> +
>  2.3 Writing a testcase in shell
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
> diff --git a/include/tst_kernel.h b/include/tst_kernel.h
> index 5d5c04c..88941e1 100644
> --- a/include/tst_kernel.h
> +++ b/include/tst_kernel.h
> @@ -23,4 +23,12 @@
>   */
>  int tst_kernel_bits(void);
>  
> +/**
> + * Checks support for the kernel driver.
> + *
> + * @param name The name of the driver.
> + * @return Returns 0 if the kernel has the driver or modprobe is missing.
> + */
> +int tst_check_driver(const char *name);
> +
>  #endif	/* TST_KERNEL_H__ */
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 98dacf3..55c1418 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -170,6 +170,9 @@ struct tst_test {
>  
>  	/* NULL terminated array of resource file names */
>  	const char *const *resource_files;
> +
> +	/* NULL terminated array of needed kernel drivers */
> +	const char * const *needs_drivers;
>  };
>  
>  /*
> diff --git a/lib/tst_kernel.c b/lib/tst_kernel.c
> index 42d64cb..ad00d2d 100644
> --- a/lib/tst_kernel.c
> +++ b/lib/tst_kernel.c
> @@ -45,3 +45,12 @@ int tst_kernel_bits(void)
>  
>  	return kernel_bits;
>  }
> +
> +int tst_check_driver(const char *name)
> +{
> +	const char * const argv[] = { "modprobe", name, NULL };
> +	int res = tst_run_cmd_(NULL, argv, "/dev/null", "/dev/null", 1);
> +
> +	/* 255 - it looks like modprobe not available */
> +	return (res == 255) ? 0 : res;
> +}
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 2f3d357..dcc4088 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -767,6 +767,15 @@ static void do_setup(int argc, char *argv[])
>  	if (tst_test->min_kver)
>  		check_kver();
>  
> +	if (tst_test->needs_drivers) {
> +		const char *name;
> +		int i;
> +
> +		for (i = 0; (name = tst_test->needs_drivers[i]); ++i)
> +			if (tst_check_driver(name))
> +				tst_brk(TCONF, "%s driver not available", name);
> +	}
> +
>  	if (tst_test->format_device)
>  		tst_test->needs_device = 1;
>  
> diff --git a/testcases/lib/.gitignore b/testcases/lib/.gitignore
> index a9034e4..d83a48e 100644
> --- a/testcases/lib/.gitignore
> +++ b/testcases/lib/.gitignore
> @@ -1,5 +1,6 @@
>  /tst_sleep
>  /tst_random
> +/tst_check_drivers
>  /tst_checkpoint
>  /tst_rod
>  /tst_kvcmp
> diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile
> index 3547e16..e1dea3b 100644
> --- a/testcases/lib/Makefile
> +++ b/testcases/lib/Makefile
> @@ -28,6 +28,6 @@ INSTALL_TARGETS		:= *.sh
>  
>  MAKE_TARGETS		:= tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
>  			   tst_device tst_net_iface_prefix tst_net_ip_prefix tst_net_vars\
> -			   tst_getconf tst_supported_fs
> +			   tst_getconf tst_supported_fs tst_check_drivers
>  
>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/lib/tst_check_drivers.c b/testcases/lib/tst_check_drivers.c
> new file mode 100644
> index 0000000..c1d879d
> --- /dev/null
> +++ b/testcases/lib/tst_check_drivers.c
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* Copyright (c) 2018 Oracle and/or its affiliates. All Rights Reserved. */
> +
> +#include <stdio.h>
> +#include "tst_kernel.h"
> +
> +int main(int argc, const char *argv[])
> +{
> +	const char *name;
> +	int i;
> +
> +	if (argc < 2) {
> +		fprintf(stderr, "Please provide kernel driver list\n");
> +		return 1;
> +	}
> +
> +	for (i = 1; (name = argv[i]); ++i)
> +		if (tst_check_driver(name)) {
> +			fprintf(stderr, "%s", name);
> +			return 1;
> +		}

I would be happier if we put a curly braces around large blocks inside
of other blocks as LKML prefers to do, since this piece of code is
starting to look a bit like python...


Other than that this looks very good.

-- 
Cyril Hrubis
chrubis@suse.cz

  parent reply	other threads:[~2018-09-13 12:27 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-20 11:23 [LTP] [PATCH v2 1/4] lib/tst_test.c: add 'needs_drivers' option with tst_check_drivers cmd Alexey Kodanev
2018-08-20 11:23 ` [LTP] [PATCH v2 2/4] lib/tst_test.sh: add TST_NEEDS_DRIVERS parameter Alexey Kodanev
2018-08-22 15:42   ` Petr Vorel
2018-09-13 12:35   ` Cyril Hrubis
2018-09-14 11:54     ` Alexey Kodanev
2018-08-20 11:23 ` [LTP] [PATCH v2 3/4] lib/tst_test.sh: add TST_RTNL_CHK() helper function Alexey Kodanev
2018-09-13 13:00   ` Petr Vorel
2018-08-20 11:23 ` [LTP] [PATCH v2 4/4] network/ipsec: replace ipsec_try() with TST_RTNL_CHK() Alexey Kodanev
2018-08-22 15:45   ` Petr Vorel
2018-09-13 13:02   ` Petr Vorel
2018-08-22 15:41 ` [LTP] [PATCH v2 1/4] lib/tst_test.c: add 'needs_drivers' option with tst_check_drivers cmd Petr Vorel
2018-08-27 10:44   ` Alexey Kodanev
2018-09-06 17:53 ` Petr Vorel
2018-09-13 12:27 ` Cyril Hrubis [this message]
2018-09-14 11:41   ` Alexey Kodanev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180913122712.GA18036@rei.lan \
    --to=chrubis@suse.cz \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox