[Devel] [PATCH VZ10 v9 3/8] selftests/ve: Update ve_ns_owner_test
Vladimir Riabchun
vladimir.riabchun at virtuozzo.com
Fri Sep 18 16:28:01 MSK 2026
Sashiko review:
On 9/15/26 19:22, Vladimir Riabchun wrote:
> 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 at virtuozzo.com>
> ---
> tools/testing/selftests/ve/ve_ns_owner_test.c | 33 ++++++++++---------
> 1 file changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/selftests/ve/ve_ns_owner_test.c b/tools/testing/selftests/ve/ve_ns_owner_test.c
> index 82c31dff4b1b..c3fa7d7b7f9c 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 some value >= 1 (mounts copied into the
> * new mntns are accounted to the new ve).
> *
> * 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);
Sashiko says:
This is a pre-existing issue, but the logic in check_new_ve_owner to verify that
've.mnt_avail_nr' strictly decreases seems unsound, potentially causing
intermittent test failures.
Could the test environment allow for external factors that might not ensure
this decrement happens logically?
Is there a possibility that concurrent operations could influence the
've.mnt_avail_nr' in a way that disrupts this test's reliability?
No, this is our namespace and nobody should touch it. If value is different, a bug
is somewhere.
> }
>
> 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
> - * the clone/unshare populates the new mntns under this ve, i.e.
> - * mnt_nr rises strictly above zero.
> + * mnt_avail_nr counter should be VE_MOUNTS_MAX. Each test below
> + * verifies that the clone/unshare populates the new mntns
> + * under this ve, i.e. mnt_avail_nr strictly decreases.
> */
> - snprintf(path, sizeof(path), "%d/ve.mnt_nr", self->ctid);
> - ASSERT_EQ(read_u64_at(self->cgv2_fd, path, &initial_mnt_nr), 0);
> - ASSERT_EQ(initial_mnt_nr, 0);
> + snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", self->ctid);
> + ASSERT_EQ(read_u64_at(self->cgv2_fd, path, &initial_mnt_avail_nr), 0);
> + ASSERT_EQ(initial_mnt_avail_nr, VE_MOUNTS_MAX);
> };
>
> FIXTURE_TEARDOWN(ve_ns_owner)
--
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo
More information about the Devel
mailing list