[CRIU] [PATCH] check/zdtm: Introduce fine-grained feature testing

Christopher Covington cov at codeaurora.org
Tue Jan 13 12:01:21 PST 2015


On 01/13/2015 01:17 PM, Pavel Emelyanov wrote:
> Right now we state that CRIU works on 3.11 and above kernels and, at the
> same time, have support for a couple of new features like aio, tun, timerfd
> etc. available in later kernels. Since these new features do not break
> generic operations we do not require them in the kernel strictly.
> 
> However, in the zdtm tests it's very important to know exactly what can
> and what cannot be tested. Right now this is done in a tough manner -- if
> the kernel is not 3.11 or criu check fails for _any_ reason we treat the
> kernel as being "bad" and throw out a set of tests.
> 
> I propose to test some individual features and form the list of tests
> in a more fine-grained manner.
> 
> This patch only fixes the AIO, mnt_id, tun and posix-timers tests. Next
> I will add checks and fixes for user-namespaces tests.

It looks like some tests are always run, but they might skip (like sse00)
while other tests may be omitted from the list and not run (like aio). It
might be nice if one or the other approach was used consistently. In my
opinion putting skips in the C code results in prettier code than
conditionally appending to test lists in shell, but on the other hand it does
seem more efficient to call criu check once rather than for every single test
that is feature dependent. Do you think we can and should convert the tests to
one or the other approach? If so, which one?

My motivation is that in my arm and aarch64 environments, tests like sse00
that are supposed to skip end up failing (although it could be the fault of my
environment which is an installed version of the tests rather than in the
source tree with the patch to do everything with zdtm.sh and not need make).

00:05:18.685:  3613: SKIP: sse00.c:84: Unsupported arch
00:05:18.688:  3612: ERR: test.c:176: Test exited with unexpectedly with code 0

Thanks,
Chris

> Signed-off-by: Pavel Emelyanov <xemul at parallels.com>
> ---
>  cr-check.c        | 39 ++++++++++++++++++++++++++++++++++++++-
>  crtools.c         |  5 +++++
>  include/crtools.h |  1 +
>  include/tun.h     |  2 +-
>  test/zdtm.sh      | 43 ++++++++++++++++++++++++++++++++++---------
>  tun.c             |  4 ++--
>  6 files changed, 81 insertions(+), 13 deletions(-)
> 
> diff --git a/cr-check.c b/cr-check.c
> index 0c4dcf4..51d25ae 100644
> --- a/cr-check.c
> +++ b/cr-check.c
> @@ -675,6 +675,8 @@ static int check_aio_remap(void)
>  	return 0;
>  }
>  
> +static int (*chk_feature)(void);
> +
>  int cr_check(void)
>  {
>  	struct ns_id ns = { .pid = getpid(), .nd = &mnt_ns_desc };
> @@ -700,6 +702,11 @@ int cr_check(void)
>  	if (mntinfo == NULL)
>  		return -1;
>  
> +	if (chk_feature) {
> +		ret = chk_feature();
> +		goto out;
> +	}
> +
>  	ret |= check_map_files();
>  	ret |= check_sock_diag();
>  	ret |= check_ns_last_pid();
> @@ -718,13 +725,43 @@ int cr_check(void)
>  	ret |= check_ptrace_peeksiginfo();
>  	ret |= check_mem_dirty_track();
>  	ret |= check_posix_timers();
> -	ret |= check_tun();
> +	ret |= check_tun_cr(0);
>  	ret |= check_timerfd();
>  	ret |= check_mnt_id();
>  	ret |= check_aio_remap();
>  
> +out:
>  	if (!ret)
>  		pr_msg("Looks good.\n");
>  
>  	return ret;
>  }
> +
> +static int check_tun(void)
> +{
> +	/*
> +	 * In case there's no TUN support at all we
> +	 * should report error. Unlike this plain criu
> +	 * check would report "Looks good" in this case
> +	 * since C/R effectively works, just not for TUN.
> +	 */
> +	return check_tun_cr(-1);
> +}
> +
> +int check_add_feature(char *feat)
> +{
> +	if (!strcmp(feat, "mnt_id"))
> +		chk_feature = check_mnt_id;
> +	else if (!strcmp(feat, "aio_remap"))
> +		chk_feature = check_aio_remap;
> +	else if (!strcmp(feat, "timerfd"))
> +		chk_feature = check_timerfd;
> +	else if (!strcmp(feat, "tun"))
> +		chk_feature = check_tun;
> +	else {
> +		pr_err("Unknown feature %s\n", feat);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> diff --git a/crtools.c b/crtools.c
> index 3cc4954..e235faa 100644
> --- a/crtools.c
> +++ b/crtools.c
> @@ -191,6 +191,7 @@ int main(int argc, char *argv[], char *envp[])
>  		{ "manage-cgroups", no_argument, 0, 1060},
>  		{ "cgroup-root", required_argument, 0, 1061},
>  		{ "inherit-fd", required_argument, 0, 1062},
> +		{ "feature", required_argument, 0, 1063},
>  		{ },
>  	};
>  
> @@ -400,6 +401,10 @@ int main(int argc, char *argv[], char *envp[])
>  			if (inherit_fd_add(optarg) < 0)
>  				return 1;
>  			break;
> +		case 1063:
> +			if (check_add_feature(optarg) < 0)
> +				return 1;
> +			break;
>  		case 'M':
>  			{
>  				char *aux;
> diff --git a/include/crtools.h b/include/crtools.h
> index f4b09a3..176e8b3 100644
> --- a/include/crtools.h
> +++ b/include/crtools.h
> @@ -30,5 +30,6 @@ extern bool may_dump(struct proc_status_creds *);
>  struct _CredsEntry;
>  extern bool may_restore(struct _CredsEntry *);
>  extern bool cr_user_is_root(void);
> +extern int check_add_feature(char *arg);
>  
>  #endif /* __CR_CRTOOLS_H__ */
> diff --git a/include/tun.h b/include/tun.h
> index 5dbabec..d70f8f2 100644
> --- a/include/tun.h
> +++ b/include/tun.h
> @@ -11,6 +11,6 @@ extern const struct fdtype_ops tunfile_dump_ops;
>  extern int dump_tun_link(NetDeviceEntry *nde, struct cr_imgset *fds);
>  extern int restore_one_tun(NetDeviceEntry *nde, int nlsk);
>  extern struct collect_image_info tunfile_cinfo;
> -extern int check_tun(void);
> +extern int check_tun_cr(int no_tun_err);
>  
>  #endif /* __CR_TUN_H__ */
> diff --git a/test/zdtm.sh b/test/zdtm.sh
> index e212ee6..d8933b6 100755
> --- a/test/zdtm.sh
> +++ b/test/zdtm.sh
> @@ -11,6 +11,8 @@ ARCH=`uname -m | sed			\
>  
>  ZP="zdtm/live"
>  
> +source $(readlink -f `dirname $0`/env.sh) || exit 1
> +
>  TEST_LIST="
>  static/pipe00
>  static/pipe01
> @@ -178,7 +180,6 @@ static/shm
>  static/msgque
>  static/sem
>  transition/ipc
> -ns/static/tun
>  static/netns-nf
>  static/netns
>  static/cgroup00
> @@ -189,14 +190,9 @@ static/remap_dead_pid
>  "
>  
>  TEST_CR_KERNEL="
> -ns/static/tun
> -static/timerfd
> -static/aio00
>  "
>  
> -cat /proc/self/fdinfo/1 | grep -q mnt_id
> -if [ $? -eq 0 ]; then
> -	TEST_LIST="$TEST_LIST
> +TEST_MNTNS="
>  ns/static/mntns_open
>  ns/static/mntns_link_remap
>  ns/static/mntns_link_ghost
> @@ -204,10 +200,41 @@ ns/static/mntns_shared_bind
>  ns/static/mntns_shared_bind02
>  ns/static/mntns_root_bind
>  "
> +
> +TEST_AIO="
> +static/aio00
> +"
> +
> +TEST_TIMERFD="
> +static/timerfd
> +"
> +
> +TEST_TUN="
> +ns/static/tun
> +"
> +
> +$CRIU check --feature "mnt_id"
> +if [ $? -eq 0 ]; then
> +	TEST_LIST="$TEST_LIST$TEST_MNTNS"
>  else
>  	export ZDTM_NOSUBNS=1
>  fi
>  
> +$CRIU check --feature "aio_remap"
> +if [ $? -eq 0 ]; then
> +	TEST_LIST="$TEST_LIST$TEST_AIO"
> +fi
> +
> +$CRIU check --feature "timerfd"
> +if [ $? -eq 0 ]; then
> +	TEST_LIST="$TEST_LIST$TEST_TIMERFD"
> +fi
> +
> +$CRIU check --feature "tun"
> +if [ $? -eq 0 ]; then
> +	TEST_LIST="$TEST_LIST$TEST_TUN"
> +fi
> +
>  BLACKLIST_FOR_USERNS="
>  ns/static/maps01
>  ns/static/mlock_setuid
> @@ -257,8 +284,6 @@ ns/static/mntns_shared_bind02
>  ns/static/mntns_root_bind
>  "
>  
> -source $(readlink -f `dirname $0`/env.sh) || exit 1
> -
>  can_cr_userns() {
>  	[ ! -f /proc/self/ns/user ] && return 1
>  	$CRIU check | fgrep -q 'PR_SET_MM_MAP is not supported' && return 1
> diff --git a/tun.c b/tun.c
> index 389a66c..0f860a9 100644
> --- a/tun.c
> +++ b/tun.c
> @@ -50,7 +50,7 @@
>  
>  #define TUN_DEV_GEN_PATH	"/dev/net/tun"
>  
> -int check_tun(void)
> +int check_tun_cr(int no_tun_err)
>  {
>  	int fd, idx = 13, ret;
>  
> @@ -62,7 +62,7 @@ int check_tun(void)
>  	fd = open(TUN_DEV_GEN_PATH, O_RDWR);
>  	if (fd < 0) {
>  		pr_perror("Can't check tun support");
> -		return 0;
> +		return no_tun_err;
>  	}
>  
>  	ret = ioctl(fd, TUNSETIFINDEX, &idx);
> 


-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


More information about the CRIU mailing list