Re: [LTP] [PATCH 1/2] swapon01: Test on all filesystems

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> Test on all filesystems to increase coverage.  btrfs and tmpfs
> currently does not support swap file, but keep it in case this get
> changed in the future.
> 
> Testing on all filesystems usually requests root, but we don't require
> it with .needs_root = 1. But using swapon() requires it as well, thus
> keep it defined.
> 
> Also detect the support on the same file as which is being tested.

For the previous iteration of this patch Li wasn't sure if this adds
enough value since there isn't ton of filesystem specific code involved
unless we actually start writing to the swapfile.

And as the patch is I would agree with that. What may make sense I think
is to require certain filesystem to support swap, i.e. fail the test in
the case that we haven't managed to create and enable the swapfile where
it's supposed to work. That would guard about accidental breakages, as
it is the test would not catch these because it woult TCONF in the
setup.

> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> IMHO it's not a big slowdown thus I keep btrfs and tmpfs.
> They might get support one day.
> 
>  testcases/kernel/syscalls/swapon/swapon01.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/swapon/swapon01.c b/testcases/kernel/syscalls/swapon/swapon01.c
> index b5c3f359c..6b7f96cf7 100644
> --- a/testcases/kernel/syscalls/swapon/swapon01.c
> +++ b/testcases/kernel/syscalls/swapon/swapon01.c
> @@ -9,6 +9,9 @@
>   * Checks that swapon() succeds with swapfile.
>   */
>  
> +#define MNTPOINT	"mntpoint"
> +#define SWAP_FILE	MNTPOINT"/swapfile01"
> +
>  #include <unistd.h>
>  #include <errno.h>
>  #include <stdlib.h>
> @@ -18,14 +21,14 @@
>  
>  static void verify_swapon(void)
>  {
> -	TEST(tst_syscall(__NR_swapon, "./swapfile01", 0));
> +	TEST(tst_syscall(__NR_swapon, SWAP_FILE, 0));
>  
>  	if (TST_RET == -1) {
>  		tst_res(TFAIL | TTERRNO, "Failed to turn on swapfile");
>  	} else {
>  		tst_res(TPASS, "Succeeded to turn on swapfile");
>  		/*we need to turn this swap file off for -i option */
> -		if (tst_syscall(__NR_swapoff, "./swapfile01") != 0) {
> +		if (tst_syscall(__NR_swapoff, SWAP_FILE) != 0) {
>  			tst_brk(TBROK | TERRNO, "Failed to turn off swapfile,"
>  				" system reboot after execution of LTP "
>  				"test suite is recommended.");
> @@ -35,13 +38,15 @@ static void verify_swapon(void)
>  
>  static void setup(void)
>  {
> -	is_swap_supported("./tstswap");
> -	make_swapfile("swapfile01", 0);
> +	is_swap_supported(SWAP_FILE);
> +	make_swapfile(SWAP_FILE, 0);
>  }
>  
>  static struct tst_test test = {
> -	.needs_root = 1,
> -	.needs_tmpdir = 1,
> +	.mntpoint = MNTPOINT,
> +	.mount_device = 1,
> +	.needs_root = 1, /* for swapon() */
> +	.all_filesystems = 1,
>  	.test_all = verify_swapon,
>  	.setup = setup
>  };
> -- 
> 2.42.0
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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