[Devel] [PATCH vz10 v3 8/9] selftests/ve: factor common helpers into ve_selftest.h
Mirian Shilakadze
mirian.shilakadze at virtuozzo.com
Fri Jul 17 14:41:26 MSK 2026
On 7/17/26 12:28 PM, Vladimir Riabchun wrote:
>
>
> On 17.07.2026 01:25, Mirian Shilakadze wrote:
>> The ve selftests each keep their own copies of the same small pieces: a
>> private cgroup2 mount, an openat based file write helper, a u64 file
>> reader, a cgroup join helper, VE cgroup create and destroy, the
>> CLONE_NEWVE fallback, the ctid search range, and the controller set a
>> VE cgroup needs enabled. The new ve.proc_permissions test needs them
>> too, so move them into a shared ve_selftest.h, the helpers made static
>> inline so they can live in a header, and drop the copies from
>> ve_ns_owner_test.
>
> It would be really cool to move ve_devcg_bpf and ve_printk tests to
> ve directory as well and reuse these nice helpers. Maybe in a separate
> task.
Agreed, that would be nice. I'd keep it as a separate follow up so this
series stays focused. Will file a task for it.
>>
>> No functional change to the tests. The moved code keeps the same
>> behaviour bar cosmetic reformatting. The shared destroy_ve additionally
>> retries briefly on a busy cgroup, which only makes teardown more robust.
>>
>> https://virtuozzo.atlassian.net/browse/VSTOR-135286
>> Signed-off-by: Mirian Shilakadze <mirian.shilakadze at virtuozzo.com>
>> ---
>> tools/testing/selftests/ve/ve_ns_owner_test.c | 111 +------------
>> tools/testing/selftests/ve/ve_selftest.h | 151 ++++++++++++++++++
>> 2 files changed, 158 insertions(+), 104 deletions(-)
>> create mode 100644 tools/testing/selftests/ve/ve_selftest.h
>>
>> diff --git a/tools/testing/selftests/ve/ve_ns_owner_test.c b/tools/
>> testing/selftests/ve/ve_ns_owner_test.c
>> index b5b4f10be72b..82c31dff4b1b 100644
>> --- a/tools/testing/selftests/ve/ve_ns_owner_test.c
>> +++ b/tools/testing/selftests/ve/ve_ns_owner_test.c
>> @@ -43,13 +43,7 @@
>> #include <errno.h>
>> #include "../kselftest_harness.h"
>> -
>> -#define CTID_MIN 108
>> -#define CTID_MAX 200
>> -
>> -#ifndef CLONE_NEWVE
>> -#define CLONE_NEWVE 0x00000040
>> -#endif
>> +#include "ve_selftest.h"
>> /*
>> * Make ve.netns_avail_nr movements easy to detect: a small cap means
>> @@ -57,81 +51,6 @@
>> */
>> #define VE_NETNS_MAX 3
>> -static int write_file_at(int dirfd, const char *path, const char *val)
>> -{
>> - int fd, ret;
>> - size_t len = strlen(val);
>> -
>> - fd = openat(dirfd, path, O_WRONLY);
>> - if (fd < 0)
>> - return -1;
>> -
>> - ret = write(fd, val, len);
>> - close(fd);
>> - return (ret == (int)len) ? 0 : -1;
>> -}
>> -
>> -static int read_u64_at(int dirfd, const char *path, unsigned long
>> long *out)
>> -{
>> - char buf[32] = {0};
>> - int fd, ret;
>> -
>> - fd = openat(dirfd, path, O_RDONLY);
>> - if (fd < 0)
>> - return -1;
>> -
>> - ret = read(fd, buf, sizeof(buf) - 1);
>> - close(fd);
>> - if (ret <= 0)
>> - return -1;
>> -
>> - *out = strtoull(buf, NULL, 10);
>> - return 0;
>> -}
>> -
>> -static int mount_cg2_fd(void)
>> -{
>> - int fs_fd, mnt_fd;
>> -
>> - fs_fd = syscall(__NR_fsopen, "cgroup2", 0);
>> - if (fs_fd < 0)
>> - return -1;
>> -
>> - if (syscall(__NR_fsconfig, fs_fd, FSCONFIG_CMD_CREATE,
>> - NULL, NULL, 0) < 0) {
>> - close(fs_fd);
>> - return -1;
>> - }
>> -
>> - mnt_fd = syscall(__NR_fsmount, fs_fd, 0, 0);
>> - close(fs_fd);
>> - return mnt_fd;
>> -}
>> -
>> -static int enter_cgroup(int cgv2_fd, int ctid)
>> -{
>> - char cg_path[64];
>> - char pid_str[64];
>> - int fd;
>> - int ret;
>> -
>> - if (ctid)
>> - snprintf(cg_path, sizeof(cg_path), "%d/cgroup.procs", ctid);
>> - else
>> - snprintf(cg_path, sizeof(cg_path), "cgroup.procs");
>> - fd = openat(cgv2_fd, cg_path, O_WRONLY);
>> - if (fd < 0)
>> - return -1;
>> -
>> - snprintf(pid_str, sizeof(pid_str), "%d", getpid());
>> - ret = write(fd, pid_str, strlen(pid_str));
>> - if (ret < 0 || ret != (int)strlen(pid_str))
>> - ret = -1;
>> -
>> - close(fd);
>> - return ret;
>> -}
>> -
>> /*
>> * Synchronisation across the clone() boundary: child does its setup,
>> * tells parent it is ready, then blocks until parent acknowledges.
>> @@ -199,7 +118,7 @@ static void check_new_ve_owner(struct
>> __test_metadata *_metadata,
>> int cgv2_fd, int ctid)
>> {
>> unsigned long long avail, mnt;
>> - char path[64];
>> + char path[PATH_MAX];
>> snprintf(path, sizeof(path), "%d/ve.netns_avail_nr", ctid);
>> ASSERT_EQ(read_u64_at(cgv2_fd, path, &avail), 0);
>> @@ -219,15 +138,14 @@ FIXTURE(ve_ns_owner)
>> FIXTURE_SETUP(ve_ns_owner)
>> {
>> unsigned long long initial_mnt_nr;
>> - char ctid_str[16];
>> char val[16];
>> - char path[64];
>> + char path[PATH_MAX];
>> self->cgv2_fd = mount_cg2_fd();
>> ASSERT_GE(self->cgv2_fd, 0);
>> ASSERT_EQ(write_file_at(self->cgv2_fd, "cgroup.subtree_control",
>> - "+cpuset +cpu +cpuacct +io +memory +hugetlb +pids +rdma
>> +misc +ve"), 0);
>> + VE_CONTROLLERS), 0);
>> ASSERT_EQ(write_file_at(self->cgv2_fd,
>> "ve.default_sysfs_permissions", "/ rx"), 0);
>> @@ -236,20 +154,8 @@ FIXTURE_SETUP(ve_ns_owner)
>> ASSERT_EQ(write_file_at(self->cgv2_fd,
>> "ve.default_sysfs_permissions", "fs/cgroup rw"), 0);
>> - self->ctid = CTID_MIN;
>> - while (self->ctid < CTID_MAX) {
>> - snprintf(ctid_str, sizeof(ctid_str), "%d", self->ctid);
>> - if (faccessat(self->cgv2_fd, ctid_str, F_OK, 0) != 0 &&
>> - errno == ENOENT)
>> - break;
>> - self->ctid++;
>> - }
>> - ASSERT_LT(self->ctid, CTID_MAX);
>> -
>> - ASSERT_EQ(mkdirat(self->cgv2_fd, ctid_str, 0755), 0);
>> -
>> - snprintf(path, sizeof(path), "%d/cgroup.controllers_hidden",
>> self->ctid);
>> - ASSERT_EQ(write_file_at(self->cgv2_fd, path, "-ve"), 0);
>> + self->ctid = make_ve(self->cgv2_fd, CTID_MIN);
>> + ASSERT_GE(self->ctid, 0);
>> /*
>> * ve.veid and ve.features are deliberately not configured: the
>> @@ -279,11 +185,8 @@ FIXTURE_SETUP(ve_ns_owner)
>> FIXTURE_TEARDOWN(ve_ns_owner)
>> {
>> - char path[64];
>> -
>> enter_cgroup(self->cgv2_fd, 0);
>> - snprintf(path, sizeof(path), "%d", self->ctid);
>> - unlinkat(self->cgv2_fd, path, AT_REMOVEDIR);
>> + destroy_ve(self->cgv2_fd, self->ctid);
>> close(self->cgv2_fd);
>> }
>> diff --git a/tools/testing/selftests/ve/ve_selftest.h b/tools/testing/
>> selftests/ve/ve_selftest.h
>> new file mode 100644
>> index 000000000000..dddb09e948f9
>> --- /dev/null
>> +++ b/tools/testing/selftests/ve/ve_selftest.h
>> @@ -0,0 +1,151 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Shared helpers for the ve selftests: a private cgroup2 mount,
>> small file and
>> + * cgroup helpers, and VE cgroup create and destroy, used across the
>> tests in
>> + * this directory.
>> + */
>> +#ifndef __SELFTESTS_VE_VE_SELFTEST_H
>> +#define __SELFTESTS_VE_VE_SELFTEST_H
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +#include <fcntl.h>
>> +#include <errno.h>
>> +#include <limits.h>
>> +#include <sys/stat.h>
>> +#include <sys/syscall.h>
>> +#include <sys/mount.h>
>> +#include <linux/mount.h>
>> +
>> +#ifndef CLONE_NEWVE
>> +#define CLONE_NEWVE 0x00000040
>> +#endif
>> +
>> +/* VE cgroup id search range shared by the ve tests. */
>> +#define CTID_MIN 108
>> +#define CTID_MAX 200
>> +
>> +/* Controllers a VE cgroup needs enabled in the root subtree_control. */
>> +#define VE_CONTROLLERS \
>> + "+cpuset +cpu +cpuacct +io +memory +hugetlb +pids +rdma +misc +ve"
>> +
>> +static inline int write_file_at(int dirfd, const char *path, const
>> char *val)
>> +{
>> + int fd, ret;
>> + size_t len = strlen(val);
>> +
>> + fd = openat(dirfd, path, O_WRONLY);
>> + if (fd < 0)
>> + return -1;
>> +
>> + ret = write(fd, val, len);
>> + close(fd);
>> + return (ret == (int)len) ? 0 : -1;
>> +}
>> +
>> +static inline int read_u64_at(int dirfd, const char *path,
>> + unsigned long long *out)
>> +{
>> + char buf[32] = {0};
>> + int fd, ret;
>> +
>> + fd = openat(dirfd, path, O_RDONLY);
>> + if (fd < 0)
>> + return -1;
>> +
>> + ret = read(fd, buf, sizeof(buf) - 1);
>> + close(fd);
>> + if (ret <= 0)
>> + return -1;
>> +
>> + *out = strtoull(buf, NULL, 10);
>
> Let's verify that number was actually read: check errno,
> verify end pointer.
read_u64_at was moved as is from ve_ns_owner_test, so this predates the
series. Will fix in v4: clear errno before strtoull and fail if errno is
set or the end pointer did not advance.
>> + return 0;
>> +}
>> +
>> +/* A fresh, private cgroup2 mount, returned as an fd to use with
>> *at() calls. */
>> +static inline int mount_cg2_fd(void)
>> +{
>> + int fs_fd, mnt_fd;
>> +
>> + fs_fd = syscall(__NR_fsopen, "cgroup2", 0);
>
> This function is the only one using syscalls, others use
> libc helpers. Maybe use them here as well?
mount_cg2_fd was carried over from ve_ns_owner_test too. You're right,
will switch it to the fsopen/fsconfig/fsmount wrappers in v4.
>> + if (fs_fd < 0)
>> + return -1;
>> +
>> + if (syscall(__NR_fsconfig, fs_fd, FSCONFIG_CMD_CREATE,
>> + NULL, NULL, 0) < 0) {
>> + close(fs_fd);
>> + return -1;
>> + }
>> +
>> + mnt_fd = syscall(__NR_fsmount, fs_fd, 0, 0);
>> + close(fs_fd);
>> + return mnt_fd;
>> +}
>> +
>> +/* Move the caller into cgroup @ctid (or the root when @ctid is 0). */
>> +static inline int enter_cgroup(int cgv2_fd, int ctid)
>> +{
>> + char cg_path[PATH_MAX];
>> + char pid_str[64];
>> + int fd, ret;
>> +
>> + if (ctid)
>> + snprintf(cg_path, sizeof(cg_path), "%d/cgroup.procs", ctid);
>> + else
>> + snprintf(cg_path, sizeof(cg_path), "cgroup.procs");
>> + fd = openat(cgv2_fd, cg_path, O_WRONLY);
>> + if (fd < 0)
>> + return -1;
>> +
>> + snprintf(pid_str, sizeof(pid_str), "%d", getpid());
>> + ret = write(fd, pid_str, strlen(pid_str));
>> + if (ret < 0 || ret != (int)strlen(pid_str))
>> + ret = -1;
>> +
>> + close(fd);
>> + return ret;
>> +}
>> +
>> +/*
>> + * Create a fresh VE cgroup at the first free id at or after @from
>> and unhide
>> + * its ve.* control files. Return the new id, or -1.
>> + */
>> +static inline int make_ve(int cgv2_fd, int from)
>> +{
>> + char name[16], path[PATH_MAX];
>> + int id;
>> +
>> + for (id = from; id < CTID_MAX; id++) {
>> + snprintf(name, sizeof(name), "%d", id);
>> + if (faccessat(cgv2_fd, name, F_OK, 0) != 0 && errno == ENOENT)
>> + break;
>> + }
>> + if (id >= CTID_MAX)
>> + return -1;
>> + if (mkdirat(cgv2_fd, name, 0755) != 0)
>> + return -1;
>> + snprintf(path, sizeof(path), "%d/cgroup.controllers_hidden", id);
>> + if (write_file_at(cgv2_fd, path, "-ve") < 0)
>
> Missing cleanup.
Will fix: make_ve will remove the cgroup it created if the
controllers_hidden write fails.
>> + return -1;
>> + return id;
>> +}
>> +
>> +/* Remove a VE cgroup, retrying briefly while it is still busy. */
>> +static inline void destroy_ve(int cgv2_fd, int id)
>> +{
>> + char name[16];
>> + int i;
>> +
>> + if (id < 0)
>> + return;
>> + snprintf(name, sizeof(name), "%d", id);
>> + for (i = 0; i < 100; i++) {
>> + if (unlinkat(cgv2_fd, name, AT_REMOVEDIR) == 0 || errno !=
>> EBUSY)
>> + break;
>> + usleep(10000);
>> + }
>
> Let's report here that destroy failed - it usually is a problem.
Agreed, destroy_ve will report, if it still cannot remove the cgroup
after the retries.
These fixes touch a helper moved as is (read_u64_at) and the new
create/destroy helpers, so the move commit stops being a pure move.
I plan to fold them in as they are small, but can split them into a
separate patch if you would prefer. All of it will be in v4.
--
Best regards, Mirian Shilakadze
Linux C Virtualization Developer, Virtuozzo.
More information about the Devel
mailing list