Re: [Devel] [PATCH VZ10 v6 4/9] selftests/ve: Update ve_ns_owner_test

[Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>]

> Mount accounting interface has changed, now it shows the number
> of available mounts.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> 
> Feature: per-ve failcounters
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
>
> diff --git a/tools/testing/selftests/ve/ve_ns_owner_test.c b/tools/testing/selftests/ve/ve_ns_owner_test.c
> index 82c31dff4b1b..adcbc4051442 100644
> --- a/tools/testing/selftests/ve/ve_ns_owner_test.c
> +++ b/tools/testing/selftests/ve/ve_ns_owner_test.c
> @@ -18,7 +18,7 @@
>   * with a new netns and mntns, the parent reads the new ve's counters
>   * via cgroupfs and asserts they reflect the just-created namespaces:
>   *   - ve.netns_avail_nr drops by exactly one (the new netns);
> - *   - ve.mnt_nr is strictly greater than zero (mounts copied into the
> + *   - ve.mnt_avail_nr drops by exactly one (mounts copied into the
>   *     new mntns are accounted to the new ve).
Does it really drop by exactly one? If we copy mnt namespace, we copy
the whole mount tree not just one mount so we can possibly drop my more
than one.
>   *
>   * We never assert against the parent ve's counters: those are shared
> @@ -50,6 +50,7 @@
>   * any spurious accounting against the parent ve would overflow it.
>   */
>  #define VE_NETNS_MAX		3
> +#define VE_MOUNTS_MAX		4096
>  
>  /*
>   * Synchronisation across the clone() boundary: child does its setup,
> @@ -106,7 +107,7 @@ static int clone_child_func(void *arg)
>  /*
>   * Before fix:
>   *   - clone path: ve.netns_avail_nr stays at VE_NETNS_MAX and
> - *     ve.mnt_nr stays at 0 because copy_net_ns()/copy_mnt_ns()
> + *     ve.mnt_avail_nr stays at VE_MOUNTS_MAX because copy_net_ns()/copy_mnt_ns()
>   *     charged the parent ve via get_exec_env().
>   *   - unshare path: the syscall itself returned -EINVAL, so this
>   *     check was unreachable.
> @@ -117,16 +118,16 @@ static int clone_child_func(void *arg)
>  static void check_new_ve_owner(struct __test_metadata *_metadata,
>  			       int cgv2_fd, int ctid)
>  {
> -	unsigned long long avail, mnt;
> -	char path[PATH_MAX];
> +	unsigned long long avail_netns, avail_mnt;
> +	char path[64];
>  
>  	snprintf(path, sizeof(path), "%d/ve.netns_avail_nr", ctid);
> -	ASSERT_EQ(read_u64_at(cgv2_fd, path, &avail), 0);
> -	EXPECT_EQ(avail, VE_NETNS_MAX - 1);
> +	ASSERT_EQ(read_u64_at(cgv2_fd, path, &avail_netns), 0);
> +	EXPECT_EQ(avail_netns, VE_NETNS_MAX - 1);
>  
> -	snprintf(path, sizeof(path), "%d/ve.mnt_nr", ctid);
> -	ASSERT_EQ(read_u64_at(cgv2_fd, path, &mnt), 0);
> -	EXPECT_GT(mnt, 0);
> +	snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", ctid);
> +	ASSERT_EQ(read_u64_at(cgv2_fd, path, &avail_mnt), 0);
> +	EXPECT_LT(avail_mnt, VE_MOUNTS_MAX);
>  }
>  
>  FIXTURE(ve_ns_owner)
> @@ -137,7 +138,7 @@ FIXTURE(ve_ns_owner)
>  
>  FIXTURE_SETUP(ve_ns_owner)
>  {
> -	unsigned long long initial_mnt_nr;
> +	unsigned long long initial_mnt_avail_nr;
>  	char val[16];
>  	char path[PATH_MAX];
>  
> @@ -174,13 +175,13 @@ FIXTURE_SETUP(ve_ns_owner)
>  
>  	/*
>  	 * The new ve cgroup has not been entered by anything yet, so its
> -	 * mnt_nr counter must start at 0. Each test below verifies that
> +	 * mnt_avail_nr counter be VE_MOUNTS_MAX. Each test below verifies that
NIT: must be/ should be

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>