From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vasileios Almpanis Date: Fri, 7 Aug 2026 12:01:18 +0200 Subject: Re: [Devel] [PATCH VZ10 v5 9/9] selftests/ve: Add mount accounting selftest In-Reply-To: <74e7bed574901854c16d282bb96bff7bcf817f4f.1785669419.git.vladimir.riabchun@virtuozzo.com> References: <74e7bed574901854c16d282bb96bff7bcf817f4f.1785669419.git.vladimir.riabchun@virtuozzo.com> Message-ID: <6310131c-6ac1-405b-9d74-667d91d573c5@virtuozzo.com> List-Id: On 8/2/26 1:40 PM, Vladimir Riabchun wrote: > There are 6 test cases, covered in the new test: > 1. Simple mount accouting correctness, just mount/umount. > 2. Verification of correct limit hits and changes, including > negative values. > 3. Patial mounts test, when mount limit is hit in the middle > of creation. nit: Partial > 4. Test that enabled pseudosuper allows overuse. > 5. Test that pseudosuper doesn't affect mount accoutning. nit: accounting > 6. Failcount feature verification. > > https://virtuozzo.atlassian.net/browse/VSTOR-135520 > > Feature: per-ve failcounters > Signed-off-by: Vladimir Riabchun > --- > v4 -> v5: > - Splitted pseudosuper test into two. > - Added new test to check that things go smoothly when > we run out of mounts in the middle of a new mount. > > tools/testing/selftests/ve/.gitignore | 1 + > tools/testing/selftests/ve/Makefile | 1 + > .../selftests/ve/ve_mount_accounting_test.c | 419 ++++++++++++++++++ > 3 files changed, 421 insertions(+) > create mode 100644 tools/testing/selftests/ve/ve_mount_accounting_test.c > > diff --git a/tools/testing/selftests/ve/.gitignore b/tools/testing/selftests/ve/.gitignore > index afa4c568c2c9..3df4d05888dc 100644 > --- a/tools/testing/selftests/ve/.gitignore > +++ b/tools/testing/selftests/ve/.gitignore > @@ -1,2 +1,3 @@ > ve_ns_owner_test > ve_perms_test > +ve_mount_accounting_test > diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile > index ec40cbc7b3a1..c6efe7c4b4fb 100644 > --- a/tools/testing/selftests/ve/Makefile > +++ b/tools/testing/selftests/ve/Makefile > @@ -4,5 +4,6 @@ CFLAGS += -g -Wall -O2 > > TEST_GEN_PROGS += ve_ns_owner_test > TEST_GEN_PROGS += ve_perms_test > +TEST_GEN_PROGS += ve_mount_accounting_test > > include ../lib.mk > diff --git a/tools/testing/selftests/ve/ve_mount_accounting_test.c b/tools/testing/selftests/ve/ve_mount_accounting_test.c > new file mode 100644 > index 000000000000..b295290ec6e8 > --- /dev/null > +++ b/tools/testing/selftests/ve/ve_mount_accounting_test.c > @@ -0,0 +1,419 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * ve_mount_accounting selftests > + * > + * Tests to check the correctness of mount accounting. > + */ > +#define _GNU_SOURCE > +#include > +#include Is linux/mount needed here? You include both linux/mount and sys/mount. You only call mount,umount so dropping it looks generally safe. > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "../kselftest_harness.h" > +#include "ve_selftest.h" > + > +#define TMP_DIR "/ve-mnt-tmp/" > +#define VE_MOUNTS_MAX 128 > + > +static int set_pseudosuper(int cgv2_fd, int ctid, int value) > +{ > + char path[64]; > + > + snprintf(path, sizeof(path), "%d/ve.pseudosuper", ctid); > + return write_u64_at(cgv2_fd, path, value); > +} > + > +static int _create_mount(void *id_ptr) > +{ > + char path[PATH_MAX]; > + int id = *(int *)id_ptr, ret; > + > + snprintf(path, sizeof(path), TMP_DIR "%d", id); > + > + if (mkdir(path, 0755) < 0) { > + fprintf(stderr, "Failed to create directory %s: %s\n", path, strerror(errno)); > + return -1; > + } > + ret = mount("tmpfs", path, "tmpfs", 0, "size=1M"); > + if (!ret) > + return 0; > + fprintf(stderr, "Failed to mount tmpfs to %s: %s\n", path, strerror(errno)); > + > + rmdir(path); > + return ret; > +} > + > +static int create_mount(int cgv2_fd, int ctid, int id) > +{ > + int ret = run_in_ve(cgv2_fd, ctid, CLONE_NEWVE, _create_mount, &id); > + /* > + * If mount fails, cleanup by free_vfsmnt will be called > + * via call_rcu, need to wait for update. > + */ > + sleep(1); > + return ret; > +} > + > +static int _destroy_mount(void *id_ptr) > +{ > + char path[PATH_MAX]; > + struct stat st; > + int id = *(int *)id_ptr; > + > + snprintf(path, sizeof(path), TMP_DIR "%d", id); > + > + if (stat(path, &st)) > + return 1; > + if (umount(path)) { > + fprintf(stderr, "failed to umount directory %s: %s\n", path, strerror(errno)); > + return -1; > + } > + if (rmdir(path)) { > + fprintf(stderr, "failed to remove directory %s: %s\n", path, strerror(errno)); > + return -1; > + } > + return 0; > +} > + > +static int destroy_mount(int cgv2_fd, int ctid, int id) > +{ > + int ret = run_in_ve(cgv2_fd, ctid, CLONE_NEWVE, _destroy_mount, &id); > + /* free_vfsmnt is called via call_rcu, need to wait for update */ > + sleep(1); > + return ret; > +} > + > +#define MAX_MNT_ID 32 > + > +static int get_free_mnt_id(void) > +{ > + int i; > + struct stat st; > + char path[PATH_MAX]; > + > + for (i = 0; i < MAX_MNT_ID; i++) { > + snprintf(path, sizeof(path), TMP_DIR "%d", i); > + if (stat(path, &st)) > + return i; > + } > + return -1; > +} > + > +static int get_mount_cost(int cgv2_fd, int ctid) > +{ > + int avail1, avail2, mnt_id; > + char path[64]; > + > + mnt_id = get_free_mnt_id(); > + > + snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", ctid); > + if (mnt_id < 0 || > + read_s32_at(cgv2_fd, path, &avail1) || > + create_mount(cgv2_fd, ctid, mnt_id) || > + read_s32_at(cgv2_fd, path, &avail2) || > + destroy_mount(cgv2_fd, ctid, mnt_id)) > + return -1; > + > + return avail1 - avail2; > +} > + > +/* Expect mount success and return new avail value */ > +static int mount_and_get_avail(struct __test_metadata *_metadata, > + int cgv2_fd, int ctid, int mnt_id) > +{ > + char path_avail[64]; > + int mnt_avail_nr; > + > + snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", ctid); > + > + ASSERT_EQ(create_mount(cgv2_fd, ctid, mnt_id), 0); > + ASSERT_EQ(read_s32_at(cgv2_fd, path_avail, &mnt_avail_nr), 0); > + return mnt_avail_nr; > +} > + > +/* Expect mount failure and ensure intact avail number */ > +static void assert_mount_fails(struct __test_metadata *_metadata, > + int cgv2_fd, int ctid, int mnt_id, int avail_count) > +{ > + char path_avail[64]; > + int mnt_avail_nr; > + > + snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", ctid); > + > + ASSERT_EQ(read_s32_at(cgv2_fd, path_avail, &mnt_avail_nr), 0); > + ASSERT_EQ(mnt_avail_nr, avail_count); > + ASSERT_LT(create_mount(cgv2_fd, ctid, mnt_id), 0); > + ASSERT_EQ(read_s32_at(cgv2_fd, path_avail, &mnt_avail_nr), 0); > + ASSERT_EQ(mnt_avail_nr, avail_count); > +} > + > +FIXTURE(ve_mnt_acc) > +{ > + int cgv2_fd; > + int ctid; > +}; > + > +FIXTURE_SETUP(ve_mnt_acc) > +{ > + unsigned long long initial_mnt_avail_nr; > + char path[64]; > + > + self->cgv2_fd = mount_cg2_fd(); > + ASSERT_GE(self->cgv2_fd, 0); > + mkdir(TMP_DIR, 0755); > + > + ASSERT_EQ(write_file_at(self->cgv2_fd, "cgroup.subtree_control", > + VE_CONTROLLERS), 0); > + > + self->ctid = make_ve(self->cgv2_fd, CTID_MIN); > + ASSERT_GE(self->ctid, 0); > + > + snprintf(path, sizeof(path), "%d/ve.mnt_max_nr", self->ctid); > + ASSERT_EQ(write_u64_at(self->cgv2_fd, path, VE_MOUNTS_MAX), 0); > + > + /* > + * The new ve cgroup has not been entered by anything yet, so its > + * mnt_avail_nr counter should be VE_MOUNTS_MAX. > + */ > + 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_mnt_acc) > +{ Since we mount tmpfs on host mount namespace (we dont pass CLONE_NEWNS), should we iterate here and umount all the ids that remain mounted after tests bail? There are maybe places where create_mount is tried and if assertion fails the mount remains and leaks to the host possibly also pinning the ve namespace since in ve_try_reserve_mount we get a refcount on it. > + destroy_ve(self->cgv2_fd, self->ctid); > + close(self->cgv2_fd); > + rmdir(TMP_DIR); > +} > + > +/* Simple test to check mount/umount accounting correctness */ > +TEST_F(ve_mnt_acc, mount_umount) > +{ > + int original_mnt_avail, mnt_avail_nr; > + char path[64]; > + > + snprintf(path, sizeof(path), "%d/ve.mnt_avail_nr", self->ctid); > + > + ASSERT_EQ(read_s32_at(self->cgv2_fd, path, &original_mnt_avail), 0); > + > + ASSERT_LT(mount_and_get_avail(_metadata, self->cgv2_fd, self->ctid, 0), > + original_mnt_avail); > + > + ASSERT_EQ(destroy_mount(self->cgv2_fd, self->ctid, 0), 0); > + ASSERT_EQ(read_s32_at(self->cgv2_fd, path, &mnt_avail_nr), 0); > + ASSERT_EQ(mnt_avail_nr, original_mnt_avail); > +} > + > +/* Test mount limit hits */ > +TEST_F(ve_mnt_acc, hit_limits) > +{ > + int original_mnt_avail, mnt_avail_nr, mnt_cost; > + int original_have_mnt; > + char path_avail[64], path_max_nr[64]; > + > + snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", self->ctid); > + snprintf(path_max_nr, sizeof(path_max_nr), "%d/ve.mnt_max_nr", self->ctid); > + > + mnt_cost = get_mount_cost(self->cgv2_fd, self->ctid); > + ASSERT_GE(mnt_cost, 1); > + > + ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &original_mnt_avail), 0); > + original_have_mnt = VE_MOUNTS_MAX - original_mnt_avail; > + > + /* Step 1: reduce number of available mounts to mnt_cost */ > + ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, original_have_mnt + 1 * mnt_cost), 0); > + ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0); > + ASSERT_EQ(mnt_avail_nr, 1 * mnt_cost); > + > + /* Step 2: do one mount, no mounts should be available */ > + ASSERT_EQ(mount_and_get_avail(_metadata, self->cgv2_fd, self->ctid, 0), > + 0); > + > + /* Step 3: check that one more mount falils */ > + assert_mount_fails(_metadata, self->cgv2_fd, self->ctid, 1, 0); > + > + /* Step 4: increase mount limit a little bit, mount should still fail */ > + ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, > + original_have_mnt + 2 * mnt_cost - 1), 0); > + assert_mount_fails(_metadata, self->cgv2_fd, self->ctid, 1, mnt_cost - 1); > + > + /* Step 5: increase by 1 and win now */ > + ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, original_have_mnt + 2 * mnt_cost), 0); > + ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0); > + ASSERT_EQ(mnt_avail_nr, mnt_cost); > + > + ASSERT_EQ(mount_and_get_avail(_metadata, self->cgv2_fd, self->ctid, 1), > + 0); > + > + /* Step 6: reduce mnt_max_nr so we have more mounts than allowed */ > + ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, original_have_mnt + 1 * mnt_cost), 0); > + ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0); > + ASSERT_EQ(mnt_avail_nr, -1 * mnt_cost); > + > + /* Step 7: try to do mount when avail < 0, ensure number is intact */ > + assert_mount_fails(_metadata, self->cgv2_fd, self->ctid, 2, -1 * mnt_cost); > + > + /* Step 8: remove one mount, check avail value update, mount should fail */ > + ASSERT_EQ(destroy_mount(self->cgv2_fd, self->ctid, 0), 0); > + assert_mount_fails(_metadata, self->cgv2_fd, self->ctid, 2, 0); > + > + /* Step 9: remove one more mount and check that new mount succeeds */ > + ASSERT_EQ(destroy_mount(self->cgv2_fd, self->ctid, 1), 0); > + ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0); > + ASSERT_EQ(mnt_avail_nr, 1 * mnt_cost); > + ASSERT_EQ(mount_and_get_avail(_metadata, self->cgv2_fd, self->ctid, 2), > + 0); > + > + ASSERT_EQ(destroy_mount(self->cgv2_fd, self->ctid, 2), 0); > +} > + > +/* > + * Mount propagation makes one mount cost more. > + * This test check that if we run out or mounts in the middle of creating > + * a new one, everything is restored smoothly and nothing leaks. > + */ > +TEST_F(ve_mnt_acc, partial_mounts) > +{ > + char path_avail[64], path_max_nr[64]; > + int mount_cost, i, orig_have, orig_mnt_avail; > + > + snprintf(path_max_nr, sizeof(path_max_nr), "%d/ve.mnt_max_nr", self->ctid); > + snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", self->ctid); > + > + mount_cost = get_mount_cost(self->cgv2_fd, self->ctid); > + ASSERT_GE(mount_cost, 1); > + ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &orig_mnt_avail), 0); > + orig_have = VE_MOUNTS_MAX - orig_mnt_avail; > + > + if (mount_cost == 1) > + SKIP(return, "mount cost is 1, no partial mounts possible"); > + > + for (i = 0; i < mount_cost; i++) { > + ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, orig_have + i), 0); > + assert_mount_fails(_metadata, self->cgv2_fd, self->ctid, 0, i); > + } > +} > + > +/* Test that pseudosuper allows negative avail with correct accounting. */ > +TEST_F(ve_mnt_acc, pseudosuper_allows_overuse) > +{ > + int orig_mnt_avail, orig_have; > + int mount_cost; > + char path_avail[64], path_max_nr[64]; > + > + snprintf(path_max_nr, sizeof(path_max_nr), "%d/ve.mnt_max_nr", self->ctid); > + snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", self->ctid); > + ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &orig_mnt_avail), 0); > + orig_have = VE_MOUNTS_MAX - orig_mnt_avail; > + > + mount_cost = get_mount_cost(self->cgv2_fd, self->ctid); > + ASSERT_GE(mount_cost, 1); > + > + ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, orig_have + mount_cost), 0); > + ASSERT_EQ(set_pseudosuper(self->cgv2_fd, self->ctid, 1), 0); > + > + ASSERT_EQ(mount_and_get_avail(_metadata, self->cgv2_fd, self->ctid, 0), > + 0); > + ASSERT_EQ(mount_and_get_avail(_metadata, self->cgv2_fd, self->ctid, 1), > + -1 * mount_cost); > + > + ASSERT_EQ(set_pseudosuper(self->cgv2_fd, self->ctid, 0), 0); > + > + assert_mount_fails(_metadata, self->cgv2_fd, self->ctid, 2, -1 * mount_cost); > + ASSERT_EQ(destroy_mount(self->cgv2_fd, self->ctid, 0), 0); > + assert_mount_fails(_metadata, self->cgv2_fd, self->ctid, 2, 0); > + ASSERT_EQ(destroy_mount(self->cgv2_fd, self->ctid, 1), 0); > + ASSERT_EQ(get_mount_cost(self->cgv2_fd, self->ctid), mount_cost); > +} > + > +/* Test that pseudosuper doesn't disable accounting. */ > +TEST_F(ve_mnt_acc, pseudosuper_continues_accounting) > +{ > + int orig_mnt_avail, mount_cost, mnt_avail_nr; > + char path_avail[64]; > + > + snprintf(path_avail, sizeof(path_avail), "%d/ve.mnt_avail_nr", self->ctid); > + > + ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &orig_mnt_avail), 0); > + mount_cost = get_mount_cost(self->cgv2_fd, self->ctid); > + ASSERT_GE(mount_cost, 1); > + > + ASSERT_EQ(set_pseudosuper(self->cgv2_fd, self->ctid, 0), 0); > + > + /* mnt 0 - mounted without pseudosuper, umounted with it. */ > + ASSERT_EQ(mount_and_get_avail(_metadata, self->cgv2_fd, self->ctid, 0), > + orig_mnt_avail - mount_cost); > + ASSERT_EQ(set_pseudosuper(self->cgv2_fd, self->ctid, 1), 0); > + > + /* Cost is the same when mount/umount happen under pseudosuper. */ > + ASSERT_EQ(get_mount_cost(self->cgv2_fd, self->ctid), mount_cost); > + > + /* mnt 1 - mounted with pseudosuper, umounted without it. */ > + ASSERT_EQ(mount_and_get_avail(_metadata, self->cgv2_fd, self->ctid, 1), > + orig_mnt_avail - 2 * mount_cost); > + > + ASSERT_EQ(destroy_mount(self->cgv2_fd, self->ctid, 0), 0); > + ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0); > + ASSERT_EQ(mnt_avail_nr, orig_mnt_avail - mount_cost); > + > + ASSERT_EQ(set_pseudosuper(self->cgv2_fd, self->ctid, 0), 0); > + > + ASSERT_EQ(destroy_mount(self->cgv2_fd, self->ctid, 1), 0); > + ASSERT_EQ(read_s32_at(self->cgv2_fd, path_avail, &mnt_avail_nr), 0); > + ASSERT_EQ(mnt_avail_nr, orig_mnt_avail); > +} > + > +/* Test failcount feature */ > +TEST_F(ve_mnt_acc, failcount) > +{ > + char path_fc[64], failcount_str[512], path_max_nr[64]; > + > + snprintf(path_fc, sizeof(path_fc), "%d/ve.failcount", self->ctid); > + snprintf(path_max_nr, sizeof(path_max_nr), "%d/ve.mnt_max_nr", self->ctid); > + > + ASSERT_GE(read_file_at(self->cgv2_fd, path_fc, > + failcount_str, sizeof(failcount_str)), 0); > + ASSERT_TRUE(strstr(failcount_str, "mnt: 0\n") != NULL); > + > + /* Check successful mount doesn't affect failcount */ > + mount_and_get_avail(_metadata, self->cgv2_fd, self->ctid, 0); > + ASSERT_GE(read_file_at(self->cgv2_fd, path_fc, > + failcount_str, sizeof(failcount_str)), 0); > + ASSERT_TRUE(strstr(failcount_str, "mnt: 0\n") != NULL); > + ASSERT_EQ(destroy_mount(self->cgv2_fd, self->ctid, 0), 0); > + > + /* Check failcount update when mount fails */ > + ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, 0), 0); > + ASSERT_LT(create_mount(self->cgv2_fd, self->ctid, 1), 0); > + ASSERT_GE(read_file_at(self->cgv2_fd, path_fc, > + failcount_str, sizeof(failcount_str)), 0); > + ASSERT_TRUE(strstr(failcount_str, "mnt: 1\n") != NULL); > + > + /* Check failcount flush */ > + ASSERT_EQ(write_u64_at(self->cgv2_fd, path_fc, 0), 0); > + ASSERT_GE(read_file_at(self->cgv2_fd, path_fc, > + failcount_str, sizeof(failcount_str)), 0); > + ASSERT_TRUE(strstr(failcount_str, "mnt: 0\n") != NULL); > + > + /* Check failcount update when mount fails again */ > + ASSERT_EQ(write_u64_at(self->cgv2_fd, path_max_nr, 0), 0); > + ASSERT_LT(create_mount(self->cgv2_fd, self->ctid, 1), 0); > + ASSERT_GE(read_file_at(self->cgv2_fd, path_fc, > + failcount_str, sizeof(failcount_str)), 0); > + ASSERT_TRUE(strstr(failcount_str, "mnt: 1\n") != NULL); > +} > + > +TEST_HARNESS_MAIN -- Best regards, Vasileios Almpanis Software Developer, Virtuozzo.