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

Pavel Emelyanov xemul at parallels.com
Wed Jan 14 02:06:41 PST 2015


On 01/13/2015 11:01 PM, Christopher Covington wrote:

>> 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?

Heh, I didn't know sse00 behaved like that :) But it does.

Regarding which way to use. A test can only refuse to work if it finds out
that the required kernel/cpu functionality is missing. On the other hand
criu itself may refuse to work on some test if there's no required kernel
support for either dump or restore.

So we have two types of checks:

1. is some functionality available at all
2. do we have kernel support to C/R one

Right now I've chosen the strategy to make _both_ checks in the criu, because
checking whether we can C/R something can only be done after checking that we
have this something at all. Like it's done for tun and user-namespaces.

> 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

Well, this particular case can be ... fixed by adding sse tests to the arch
specific list TEST_LIST_ARCH in zdtm.sh.

> 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);
>>
> 
> 



More information about the CRIU mailing list