[PATCH v8 4/5] Implement testing framework for pidns

[Dmitry V. Levin]

On Sun, Aug 09, 2020 at 04:13:05PM +0200, Ákos Uzonyi wrote:
[...]
> --- a/tests/init.sh
> +++ b/tests/init.sh
> @@ -387,6 +387,37 @@ test_prog_set()
>  	test_pure_prog_set "$@" < "$srcdir/$NAME.in"
>  }
>  
> +test_pidns_run_strace()
> +{
> +	local parent_pid
> +
> +	check_prog tail
> +	check_prog cut
> +	check_prog grep
> +
> +	run_prog > /dev/null
> +	run_strace --pidns-translation -f $@ $args > "$EXP"
> +
> +	# filter out logs made by the parent or init process of the pidns test
> +	parent_pid="$(tail -n 2 $LOG | head -n 1 | cut -d' ' -f1)"
> +	init_pid="$(tail -n 1 $LOG | cut -d' ' -f1)"
> +	grep -E -v "^($parent_pid|$init_pid) " "$LOG" > "$OUT"
> +	match_diff "$OUT" "$EXP"
> +}

init_pid also has to be declared as local here.

> +
> +test_pidns()
> +{
> +	# ioctl(NS_GET_PARENT) is added in Linux 4.9
> +	require_min_kernel_version_or_skip 4.9

Is this check still needed given that check_ns_ioctl() already performs
a runtime check?

[...]
> +		if (read(strace_ids_pipe[0], pidns_strace_ids,
> +		    sizeof(pidns_strace_ids)) < 0)
> +			perror_msg_and_fail("read");

If this read call is expected to return sizeof(pidns_strace_ids),
should the return value be tested against that instead?

[...]
> +	if (write(strace_ids_pipe[1], pidns_strace_ids,
> +	    sizeof(pidns_strace_ids)) < 0)
> +		perror_msg_and_fail("write");

Likewise, if this write call is expected to return sizeof(pidns_strace_ids),
should the return value be tested against that instead?

> +static void
> +create_init_process(void)
> +{
> +	int child_pipe[2];
> +	if (pipe(child_pipe) < 0)
> +		perror_msg_and_fail("pipe");
> +
> +	pid_t pid = fork();
> +	if (pid < 0)
> +		perror_msg_and_fail("fork");
> +
> +	if (!pid) {
> +		close(child_pipe[1]);
> +		_exit(read(child_pipe[0], &child_pipe[1], sizeof(int)) == 0);
> +	}
> +
> +	close(child_pipe[0]);
> +}

If nothing is going to be written to child_pipe[1], then this read call is
expected to return 0, followed by _exit(1).  The choice of exit code seems
strange to me.

> +
> +void
> +check_ns_ioctl(void)
> +{
> +	int fd = open("/proc/self/ns/pid", O_RDONLY);
> +	if (fd < 0)
> +		perror_msg_and_fail("opening /proc/self/ns/pid");

What if /proc/self/ns/pid is not available, e.g. /proc is not mounted
or the kernel doesn't implement it?
I suggest to do perror_msg_and_skip instead, at least in case of ENOENT.


-- 
ldv