[LTP] [PATCH] block_dev: Convert to new API

[Cyril Hrubis]

Hi!
>  .../block/block_dev_kernel/ltp_block_dev.c         |  1 -
>  .../block/block_dev_user/block_dev.c               | 78 +++++++++-------------
>  2 files changed, 32 insertions(+), 47 deletions(-)
> 
> diff --git a/testcases/kernel/device-drivers/block/block_dev_kernel/ltp_block_dev.c b/testcases/kernel/device-drivers/block/block_dev_kernel/ltp_block_dev.c
> index 17047c0d5ae3f6556f3fa4b0eb2a17a86e5f05a6..8a3ee4ff0bac6ed46c7c09c5752b6fec3ef01a40 100644
> --- a/testcases/kernel/device-drivers/block/block_dev_kernel/ltp_block_dev.c
> +++ b/testcases/kernel/device-drivers/block/block_dev_kernel/ltp_block_dev.c
> @@ -12,7 +12,6 @@
>  #include <linux/module.h>
>  #include <linux/device.h>
>  #include <linux/fs.h>
> -#include <linux/genhd.h>
>  #include <linux/blkdev.h>

Technically LTP is supposed to support kernel 4.4 and newer, looking
into kernel git the genhd.h content was moved into blkdev.h in 5.18 so
ideally we should make it work for both. I suppose that we can make use
of one of the macros that were moved into blkdev.h and do something as:

#include <linux/blkdev.h>
#ifndef DISK_NAME_LEN
# include <linux/genhd.h>
#endif

>  MODULE_AUTHOR("Márton Németh <nm127@freemail.hu>");
> diff --git a/testcases/kernel/device-drivers/block/block_dev_user/block_dev.c b/testcases/kernel/device-drivers/block/block_dev_user/block_dev.c
> index 543c36795cc3b2776c59141023e03ff2c58bd36a..f795fb05b9af71f6a3c748936a15ac3a652db5e3 100644
> --- a/testcases/kernel/device-drivers/block/block_dev_user/block_dev.c
> +++ b/testcases/kernel/device-drivers/block/block_dev_user/block_dev.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved.
>   *
> @@ -26,74 +27,59 @@
>  #include <unistd.h>
>  #include <string.h>
>  
> -#include "test.h"
> -#include "safe_macros.h"
> -#include "old_module.h"
> +#include "tst_test.h"
> +#include "tst_module.h"
> +
> +#define MODULE_NAME "ltp_block_dev"
> +#define MODULE_NAME_KO	MODULE_NAME ".ko"
>  
>  char *TCID = "block_dev";

Isn't this unused as well?

> -int TST_TOTAL = 9;
>  
> -static const char module_name[]	= "ltp_block_dev.ko";
>  static const char dev_result[]	= "/sys/devices/ltp_block_dev/result";
>  static const char dev_tcase[]	= "/sys/devices/ltp_block_dev/tcase";
> -static int module_loaded;
>  
> -static int run_all_testcases;
> -static const option_t options[] = {
> -	{"a", &run_all_testcases, NULL},
> +static int module_loaded;
> +static char* run_all_testcases;

The whitespaces around the asterisk here are wrong. Have fixed all the
errors and warnings reported by 'make check'?

> +static struct tst_option options[] = {
> +	{"a",  &run_all_testcases, "-a\tRun all test-cases (can crash the kernel)"},
>  	{NULL, NULL, NULL}
>  };
>  
>  static void cleanup(void)
>  {
>  	if (module_loaded)
> -		tst_module_unload(NULL, module_name);
> -}
> -
> -static void help(void)
> -{
> -	printf("  -a      Run all test-cases (can crash the kernel)\n");
> -}
> -
> -void setup(int argc, char *argv[])
> -{
> -	tst_parse_opts(argc, argv, options, help);
> -
> -	tst_require_root();
> -
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> +		tst_module_unload(MODULE_NAME_KO);
>  }
>  
> -static void test_run(void)
> +static void run(unsigned int n)
>  {
> -	int off = 0;
>  	/*
>  	 * test-cases #8 and #9 can crash the kernel.
>  	 * We have to wait for kernel fix where register_blkdev() &
>  	 * unregister_blkdev() checks the input device name parameter
>  	 * against NULL pointer.
>  	 */

Is this still true?

> -	if (!run_all_testcases)
> -		off = 2;
> -
> -	tst_module_load(cleanup, module_name, NULL);
> -	module_loaded = 1;
> -
> -	int i, pass = 0;
> -	for (i = 0; i < TST_TOTAL - off; ++i) {
> -		SAFE_FILE_PRINTF(cleanup, dev_tcase, "%d", i + 1);
> -		SAFE_FILE_SCANF(cleanup, dev_result, "%d", &pass);
> -		tst_resm((pass) ? TPASS : TFAIL, "Test-case '%d'", i + 1);
> +	n++;
> +	if (!run_all_testcases && (n == 8 || n == 9)) {
> +		tst_res(TCONF, "Skipped n = %d", n);
> +		return;
>  	}
> -}
> -
> -int main(int argc, char *argv[])
> -{
> -	setup(argc, argv);
> -
> -	test_run();
>  
> -	cleanup();
> +	if (!module_loaded) {
> +		tst_module_load(MODULE_NAME_KO, NULL);
> +		module_loaded = 1;
> +	}
>  
> -	tst_exit();
> +	int pass = 0;
> +	SAFE_FILE_PRINTF(dev_tcase, "%d", n);
> +	SAFE_FILE_SCANF(dev_result, "%d", &pass);
> +	tst_res((pass) ? TPASS : TFAIL, "Test-case '%d'", n);

It would be way better if we printed a menaningful information about the
test here as well, but that would probably require having a third file
exported by the test module and should ideally be done in an subsequent
patch.

>  }
> +
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.cleanup = cleanup,
> +	.test = run,
> +	.tcnt = 9,
> +	.options = options,
> +};
> 
> ---
> base-commit: 47aff4decc81ac837fd745278def6883fc2f197b
> change-id: 20241018-block_dev-c322152e03ad
> 
> Best regards,
> -- 
> Ricardo B. Marliere <rbm@suse.com>
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz