Re: [LTP] [PATCH v3] Refactoring dio_sparse.c using LTP API

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
This version is nearly perfect.
> -		case 'a':
> -			alignment = strtol(optarg, &endp, 0);
> -			alignment = scale_by_kmg(alignment, *endp);
> -			break;

The only missing piece is that it does not support the -a option anymore
and if you grep the runtest files:

cd ltp/runtest; git grep dio_sparse

You will see that there are several entries that actually pass the
alignment to the test. I do not think that having the memory aligned
more than the minimal requirement makes any difference, but we still
have to fix the entries if we decide to get rid of that parameter.


Also the minimal aligment requirement is not page size but the result
from ioctl(2) BLKSSZGET which is the blocksize for the underlying
device and while the page size is likely multiple of that we should
really use the ioctl(), we already use that in the aiocp.c test.

> -		case 'w':
> -			writesize = strtol(optarg, &endp, 0);
> -			writesize = scale_by_kmg(writesize, *endp);
> -			break;
> -		case 's':
> -			filesize = strtol(optarg, &endp, 0);
> -			filesize = scale_by_kmg(filesize, *endp);
> -			break;
> -		case 'o':
> -			offset = strtol(optarg, &endp, 0);
> -			offset = scale_by_kmg(offset, *endp);
> -			break;
> -		case 'n':
> -			num_children = atoi(optarg);
> -			if (num_children > NUM_CHILDREN) {
> -				fprintf(stderr,
> -					"number of children limited to %d\n",
> -					NUM_CHILDREN);
> -				num_children = NUM_CHILDREN;
> -			}
> -			break;
> -		case '?':
> -			usage();
> -			break;
> -		}
> -	}
> -
> -	setup();
> -	tst_resm(TINFO, "Dirtying free blocks");
> -	dirty_freeblocks(filesize);
> -
> -	fd = SAFE_OPEN(cleanup, filename,
> -		O_DIRECT | O_WRONLY | O_CREAT | O_EXCL, 0600);
> -	SAFE_FTRUNCATE(cleanup, fd, filesize);
> -
> -	tst_resm(TINFO, "Starting I/O tests");
> -	signal(SIGTERM, SIG_DFL);
> -	for (i = 0; i < num_children; i++) {
> -		switch (pid[i] = fork()) {
> -		case 0:
> -			SAFE_CLOSE(NULL, fd);
> -			read_sparse(filename, filesize);
> -			break;
> -		case -1:
> -			while (i-- > 0)
> -				kill(pid[i], SIGTERM);
> -
> -			tst_brkm(TBROK | TERRNO, cleanup, "fork()");
> -		default:
> -			continue;
> -		}
> -	}
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -
> -	ret = dio_sparse(fd, alignment, writesize, filesize, offset);
> -
> -	tst_resm(TINFO, "Killing childrens(s)");
>  
> -	for (i = 0; i < num_children; i++)
> -		kill(pid[i], SIGTERM);
> +	fd = SAFE_OPEN(filename, O_DIRECT | O_WRONLY | O_CREAT, 0666);
> +	SAFE_FTRUNCATE(fd, filesize);
>  
> -	for (i = 0; i < num_children; i++) {
> -		int status;
> -		pid_t p;
> +	*run_child = 1;
>  
> -		p = waitpid(pid[i], &status, 0);
> -		if (p < 0) {
> -			tst_resm(TBROK | TERRNO, "waitpid()");
> -		} else {
> -			if (WIFEXITED(status) && WEXITSTATUS(status) == 10)
> -				children_errors++;
> +	for (i = 0; i < numchildren; i++) {
> +		if (!SAFE_FORK()) {
> +			io_read(filename, filesize, run_child);
> +			return;
>  		}
>  	}
>  
> -	if (children_errors)
> -		tst_resm(TFAIL, "%i children(s) exited abnormally",
> -			 children_errors);
> +	dio_sparse(fd, filesize, writesize, offset);
>  
> -	if (!children_errors && !ret)
> -		tst_resm(TPASS, "Test passed");
> +	if (SAFE_WAITPID(-1, &status, WNOHANG))
> +		tst_res(TFAIL, "Non zero bytes read");
> +	else
> +		tst_res(TPASS, "All bytes read were zeroed");
>  
> -	cleanup();
> -	tst_exit();
> +	*run_child = 0;
>  }
>  
> -static void setup(void)
> -{
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -	tst_tmpdir();
> -}
> -
> -static void cleanup(void)
> -{
> -	if (fd > 0 && close(fd))
> -		tst_resm(TWARN | TERRNO, "Failed to close file");
> -
> -	tst_rmdir();
> -}
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.options = options,
> +	.needs_tmpdir = 1,
> +	.forks_child = 1,
> +};
> -- 
> 2.34.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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