[Devel] [PATCH VZ10 0/3] Improvements for ve selftests

[Devel] [PATCH VZ10 0/3] Improvements for ve selftests

[Vladimir Riabchun]

This change adds helpers for creating and preserving various
namespaces and running functions inside them.
This appeared useful while implementing ve mount accounting
selftests - it required keeping mount namespace alive and
doing different things at different moments of time.

To make setns work with CLONE_NEWVE without starting VE,
the last patch changes -EINVAL return to warning message.

https://virtuozzo.atlassian.net/browse/VSTOR-144504

Feature: ve selftests
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
Vladimir Riabchun (3):
      selftests/ve: Add namespace management helpers
      selftests/ve: Add more helpers
      ve_namespace: Allow entering non-running VE namespace

 kernel/ve/ve_namespace.c                 |   4 +-
 tools/testing/selftests/ve/ve_selftest.h | 196 ++++++++++++++++++++++++++++++-
 2 files changed, 193 insertions(+), 7 deletions(-)
---
base-commit: 9b2e63dc19e6fb889c48fd1995dfe960af0896b7
change-id: 20260907-ve-selftest-improv-2d043d021cae

Best regards,
--  
Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH VZ10 1/3] selftests/ve: Add namespace management helpers

[Vladimir Riabchun]

When mount namespace is created in a child, it is destroyed
when the child dies. We may need to keep it for a while to check
how VE handles resources.

Instead of keeping a thread which lives in a new ve+mnt namespaces,
let's store open namespace file descriptors to enter them later
without keeping children alive.

https://virtuozzo.atlassian.net/browse/VSTOR-144504

Feature: ve selftests
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 tools/testing/selftests/ve/ve_selftest.h | 118 +++++++++++++++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/tools/testing/selftests/ve/ve_selftest.h b/tools/testing/selftests/ve/ve_selftest.h
index c53bf7900d20..b3c5bf63f34d 100644
--- a/tools/testing/selftests/ve/ve_selftest.h
+++ b/tools/testing/selftests/ve/ve_selftest.h
@@ -31,6 +31,124 @@
 #define VE_CONTROLLERS \
 	"+cpuset +cpu +cpuacct +io +memory +hugetlb +pids +rdma +misc +ve"
 
+struct namespace_ctx {
+	int ns_save_mask;	/* Mask of stored CLONE_NEW* namespaces. */
+	/* Namespace file descriptors. Name should match /proc/PID/ns/NAME */
+	int ve_fd;
+	int mnt_fd;
+};
+
+static inline void close_namespace_fds(struct namespace_ctx *ctx)
+{
+	if (ctx->ns_save_mask & CLONE_NEWVE)
+		close(ctx->ve_fd);
+	if (ctx->ns_save_mask & CLONE_NEWNS)
+		close(ctx->mnt_fd);
+
+	ctx->ns_save_mask = 0;
+}
+
+static inline int save_namespace_fds(struct namespace_ctx *ctx, int flags)
+{
+#define OPEN_NS(name, flag)                                                \
+do {                                                                       \
+	if (flags & flag) {                                                \
+		ctx->name##_fd = open("/proc/self/ns/" #name, O_RDONLY);   \
+		if (ctx->name##_fd < 0) {                                  \
+			fprintf(stderr, "%s: could not open fd %s: %s\n",  \
+				__func__, #name, strerror(errno));         \
+			goto err;                                          \
+		}                                                          \
+		ctx->ns_save_mask |= flag;                                 \
+	}                                                                  \
+} while (0)
+
+	ctx->ns_save_mask = 0;
+
+	OPEN_NS(ve, CLONE_NEWVE);
+	OPEN_NS(mnt, CLONE_NEWNS);
+
+	return 0;
+
+err:
+	close_namespace_fds(ctx);
+	return -1;
+}
+
+static inline int enter_namespaces(struct namespace_ctx *ctx, int flags)
+{
+#define JOIN_NS(name, flag)                                                 \
+do {                                                                        \
+	if (flags & flag) {                                                 \
+		if (!(ctx->ns_save_mask & flag)) {                          \
+			fprintf(stderr, "%s: could not switch ns %s:"       \
+				" no fd in context.\n", __func__, #name);   \
+			return -1;                                          \
+		}                                                           \
+		if (setns(ctx->name##_fd, flag)) {                          \
+			fprintf(stderr, "%s: could not switch ns %s: %s\n", \
+				__func__, #name, strerror(errno));          \
+			return -1;                                          \
+		}                                                           \
+	}                                                                   \
+} while (0)
+
+	JOIN_NS(mnt, CLONE_NEWNS);
+	JOIN_NS(ve, CLONE_NEWVE);
+	return 0;
+}
+
+static inline int enter_cgroup(int cgv2_fd, int ctid);
+
+/*
+ * Create new namespaces and store them in ctx.
+ * CLONE_NEWVE is treated specially - before unshare corresponding
+ * CT cgroup is entered, after it root cgroup is entered(not original one).
+ */
+static inline int prepare_namespaces(struct namespace_ctx *ctx, int flags,
+	int cgv2_fd, int ctid)
+{
+	struct namespace_ctx tmp_ctx;
+
+	if (save_namespace_fds(&tmp_ctx, flags) < 0)
+		return -1;
+
+	if (flags & CLONE_NEWVE) {
+		if (enter_cgroup(cgv2_fd, ctid) < 0) {
+			fprintf(stderr, "%s: failed to enter CT cgroup\n", __func__);
+			return -1;
+		}
+	}
+
+	if (unshare(flags) < 0) {
+		fprintf(stderr, "%s: failed to unshare, flags %x: %s\n",
+			__func__, flags, strerror(errno));
+		return -1;
+	}
+
+	if (save_namespace_fds(ctx, flags)) {
+		fprintf(stderr, "%s: failed to save new namespaces, flags %x\n",
+			__func__, flags);
+		return -1;
+	}
+
+	if (flags & CLONE_NEWVE) {
+		if (enter_cgroup(cgv2_fd, 0) < 0) {
+			fprintf(stderr, "%s: failed to restore root cgroup\n", __func__);
+			return -1;
+		}
+	}
+
+	if (enter_namespaces(&tmp_ctx, flags) < 0) {
+		fprintf(stderr, "%s: failed to restore old namespaces, flags %x\n",
+			__func__, flags);
+		return -1;
+	}
+
+	close_namespace_fds(&tmp_ctx);
+	return 0;
+}
+
 static inline int write_file_at(int dirfd, const char *path, const char *val)
 {
 	int fd, ret;

-- 
2.47.1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH VZ10 1/3] selftests/ve: Add namespace management helpers

[Vladimir Riabchun]

Sashiko review + replies.

On 9/7/26 21:14, Vladimir Riabchun wrote:
> When mount namespace is created in a child, it is destroyed
> when the child dies. We may need to keep it for a while to check
> how VE handles resources.
> 
> Instead of keeping a thread which lives in a new ve+mnt namespaces,
> let's store open namespace file descriptors to enter them later
> without keeping children alive.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-144504
> 
> Feature: ve selftests
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>   tools/testing/selftests/ve/ve_selftest.h | 118 +++++++++++++++++++++++++++++++
>   1 file changed, 118 insertions(+)
> 
> diff --git a/tools/testing/selftests/ve/ve_selftest.h b/tools/testing/selftests/ve/ve_selftest.h
> index c53bf7900d20..b3c5bf63f34d 100644
> --- a/tools/testing/selftests/ve/ve_selftest.h
> +++ b/tools/testing/selftests/ve/ve_selftest.h
> @@ -31,6 +31,124 @@
>   #define VE_CONTROLLERS \
>   	"+cpuset +cpu +cpuacct +io +memory +hugetlb +pids +rdma +misc +ve"
>   
> +struct namespace_ctx {
> +	int ns_save_mask;	/* Mask of stored CLONE_NEW* namespaces. */
> +	/* Namespace file descriptors. Name should match /proc/PID/ns/NAME */
> +	int ve_fd;
> +	int mnt_fd;
> +};
> +
> +static inline void close_namespace_fds(struct namespace_ctx *ctx)
> +{
> +	if (ctx->ns_save_mask & CLONE_NEWVE)
> +		close(ctx->ve_fd);
> +	if (ctx->ns_save_mask & CLONE_NEWNS)
> +		close(ctx->mnt_fd);
> +
> +	ctx->ns_save_mask = 0;
> +}
> +
> +static inline int save_namespace_fds(struct namespace_ctx *ctx, int flags)
> +{
> +#define OPEN_NS(name, flag)                                                \
> +do {                                                                       \
> +	if (flags & flag) {                                                \
> +		ctx->name##_fd = open("/proc/self/ns/" #name, O_RDONLY);   \
> +		if (ctx->name##_fd < 0) {                                  \
> +			fprintf(stderr, "%s: could not open fd %s: %s\n",  \
> +				__func__, #name, strerror(errno));         \
> +			goto err;                                          \
> +		}                                                          \
> +		ctx->ns_save_mask |= flag;                                 \
> +	}                                                                  \
> +} while (0)
> +
> +	ctx->ns_save_mask = 0;

Sashiko says:
  If save_namespace_fds() is called twice on the same namespace_ctx
  without calling close_namespace_fds() in between, does this line reset
  the mask and leak the previously opened file descriptors?

User is responsible for freeing resources correctly. The idea of this design
was to avoid additional initialization of ctx before calling it. With current
approach we can pass ctx with any stack garbage and it will work.

> +
> +	OPEN_NS(ve, CLONE_NEWVE);
> +	OPEN_NS(mnt, CLONE_NEWNS);
> +
> +	return 0;
> +
> +err:
> +	close_namespace_fds(ctx);
> +	return -1;
> +}
> +
> +static inline int enter_namespaces(struct namespace_ctx *ctx, int flags)
> +{
> +#define JOIN_NS(name, flag)                                                 \
> +do {                                                                        \
> +	if (flags & flag) {                                                 \
> +		if (!(ctx->ns_save_mask & flag)) {                          \
> +			fprintf(stderr, "%s: could not switch ns %s:"       \
> +				" no fd in context.\n", __func__, #name);   \
> +			return -1;                                          \
> +		}                                                           \
> +		if (setns(ctx->name##_fd, flag)) {                          \
> +			fprintf(stderr, "%s: could not switch ns %s: %s\n", \
> +				__func__, #name, strerror(errno));          \
> +			return -1;                                          \
> +		}                                                           \
> +	}                                                                   \
> +} while (0)
> +
> +	JOIN_NS(mnt, CLONE_NEWNS);
> +	JOIN_NS(ve, CLONE_NEWVE);
> +	return 0;
> +}

Sashiko says:
  In enter_namespaces(), the namespaces are restored in a fixed order.
  Could this cause issues if there is a dependency between the VE and
  MNT namespaces that requires a specific restoration order based on
  the flags provided?

At least we should keep the same order as in save_namespace_fds. Will fix
in v2.

> +
> +static inline int enter_cgroup(int cgv2_fd, int ctid);
> +
> +/*
> + * Create new namespaces and store them in ctx.
> + * CLONE_NEWVE is treated specially - before unshare corresponding
> + * CT cgroup is entered, after it root cgroup is entered(not original one).
> + */
> +static inline int prepare_namespaces(struct namespace_ctx *ctx, int flags,
> +	int cgv2_fd, int ctid)
> +{
> +	struct namespace_ctx tmp_ctx;
> +
> +	if (save_namespace_fds(&tmp_ctx, flags) < 0)
> +		return -1;
> +
> +	if (flags & CLONE_NEWVE) {
> +		if (enter_cgroup(cgv2_fd, ctid) < 0) {
> +			fprintf(stderr, "%s: failed to enter CT cgroup\n", __func__);
> +			return -1;
> +		}
> +	}
> +
> +	if (unshare(flags) < 0) {
> +		fprintf(stderr, "%s: failed to unshare, flags %x: %s\n",
> +			__func__, flags, strerror(errno));
> +		return -1;
> +	}
> +
> +	if (save_namespace_fds(ctx, flags)) {
> +		fprintf(stderr, "%s: failed to save new namespaces, flags %x\n",
> +			__func__, flags);
> +		return -1;
> +	}

Sashiko says:
  In the error paths above, such as when enter_cgroup(), unshare(), or
  save_namespace_fds(ctx, flags) fails, are the file descriptors held
  in tmp_ctx leaked?

True, but honestly I don't think it matters a lot - failing here is a
critical error and the test should be aborted. But I'll fix in v2.

> +
> +	if (flags & CLONE_NEWVE) {
> +		if (enter_cgroup(cgv2_fd, 0) < 0) {
> +			fprintf(stderr, "%s: failed to restore root cgroup\n", __func__);
> +			return -1;
> +		}
> +	}
> +
> +	if (enter_namespaces(&tmp_ctx, flags) < 0) {
> +		fprintf(stderr, "%s: failed to restore old namespaces, flags %x\n",
> +			__func__, flags);
> +		return -1;
> +	}
> +
> +	close_namespace_fds(&tmp_ctx);
> +	return 0;
> +}

Sashiko says:
  If enter_namespaces(&tmp_ctx, flags) fails, does the process remain
  in an inconsistent namespace state for subsequent tests?
  Additionally, does this failure path leak the file descriptors
  stored in tmp_ctx?

Fair, will fix in v2.

> +
>   static inline int write_file_at(int dirfd, const char *path, const char *val)
>   {
>   	int fd, ret;
> 

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH VZ10 2/3] selftests/ve: Add more helpers

[Vladimir Riabchun]

Some more read/write helpers may be useful.

Also, add a helper to execute functions in child process with
switched namespaces and cgroup.

https://virtuozzo.atlassian.net/browse/VSTOR-144504

Feature: ve selftests
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 tools/testing/selftests/ve/ve_selftest.h | 78 +++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/ve/ve_selftest.h b/tools/testing/selftests/ve/ve_selftest.h
index b3c5bf63f34d..d35fec30c7b4 100644
--- a/tools/testing/selftests/ve/ve_selftest.h
+++ b/tools/testing/selftests/ve/ve_selftest.h
@@ -163,6 +163,14 @@ static inline int write_file_at(int dirfd, const char *path, const char *val)
 	return (ret == (int)len) ? 0 : -1;
 }
 
+static inline int write_u64_at(int dirfd, const char *path, unsigned long long val)
+{
+	char s[20];
+
+	snprintf(s, sizeof(s), "%llu", val);
+	return write_file_at(dirfd, path, s);
+}
+
 static inline int read_file_at(int dirfd, const char *path, char *buf,
 			       size_t buflen)
 {
@@ -193,19 +201,31 @@ static inline int read_u64_at(int dirfd, const char *path,
 			      unsigned long long *out)
 {
 	char buf[32] = {0}, *end;
-	int fd, ret;
+	int ret;
 
-	fd = openat(dirfd, path, O_RDONLY);
-	if (fd < 0)
+	ret = read_file_at(dirfd, path, buf, sizeof(buf));
+	if (ret <= 0)
 		return -1;
 
-	ret = read(fd, buf, sizeof(buf) - 1);
-	close(fd);
+	errno = 0;
+	*out = strtoull(buf, &end, 10);
+	if (errno || end == buf)
+		return -1;
+	return 0;
+}
+
+static inline int read_s32_at(int dirfd, const char *path,
+			      int *out)
+{
+	char buf[32] = {0}, *end;
+	int ret;
+
+	ret = read_file_at(dirfd, path, buf, sizeof(buf));
 	if (ret <= 0)
 		return -1;
 
 	errno = 0;
-	*out = strtoull(buf, &end, 10);
+	*out = strtol(buf, &end, 10);
 	if (errno || end == buf)
 		return -1;
 	return 0;
@@ -254,6 +274,52 @@ static inline int enter_cgroup(int cgv2_fd, int ctid)
 	return ret;
 }
 
+/*
+ * Run function in VE cgroup and new namespaces.
+ *
+ * Namespaces are provided via unshare_flags.
+ * CLONE_NEWVE flag is set by this function.
+ * Return values:
+ *  -  0 if function returns zero
+ *  - -1 if function returns negative value
+ *  -  1 if setup fails or function returns positive value
+ */
+static inline int run_in_ve(int cgv2_fd, int ctid, struct namespace_ctx *ctx,
+		int flags, int (*fn)(void *), void *arg)
+{
+	int status;
+	pid_t pid;
+
+	pid = fork();
+	if (pid < 0) {
+		fprintf(stderr, "%s: fork failed\n", __func__);
+		return 1;
+	}
+	if (pid == 0) {
+		int ret;
+
+		if (enter_cgroup(cgv2_fd, ctid) < 0) {
+			fprintf(stderr, "%s: enter_cgroup failed\n", __func__);
+			_exit(255);
+		}
+		if (enter_namespaces(ctx, flags) < 0)
+			_exit(255);
+
+		ret = fn(arg);
+		if (ret < 0)
+			ret = 1;
+		else if (ret > 0)
+			ret = 255;
+		_exit(ret);
+	}
+	if (waitpid(pid, &status, 0) < 0 || !WIFEXITED(status) || WEXITSTATUS(status) == 255)
+		return 1;
+	if (WEXITSTATUS(status))
+		return -1;
+	return 0;
+
+}
+
 /*
  * 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.

-- 
2.47.1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH VZ10 2/3] selftests/ve: Add more helpers

[Vladimir Riabchun]

Sashiko review + replies.

On 9/7/26 21:14, Vladimir Riabchun wrote:
> Some more read/write helpers may be useful.
> 
> Also, add a helper to execute functions in child process with
> switched namespaces and cgroup.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-144504
> 
> Feature: ve selftests
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>   tools/testing/selftests/ve/ve_selftest.h | 78 +++++++++++++++++++++++++++++---
>   1 file changed, 72 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/ve/ve_selftest.h b/tools/testing/selftests/ve/ve_selftest.h
> index b3c5bf63f34d..d35fec30c7b4 100644
> --- a/tools/testing/selftests/ve/ve_selftest.h
> +++ b/tools/testing/selftests/ve/ve_selftest.h
> @@ -163,6 +163,14 @@ static inline int write_file_at(int dirfd, const char *path, const char *val)
>   	return (ret == (int)len) ? 0 : -1;
>   }
>   
> +static inline int write_u64_at(int dirfd, const char *path, unsigned long long val)
> +{
> +	char s[20];
> +
> +	snprintf(s, sizeof(s), "%llu", val);
> +	return write_file_at(dirfd, path, s);
> +}

Sashiko says:
  Could this buffer be too small? A 64-bit unsigned integer can have up to
  20 digits. With the null terminator, the buffer s[] might need 21 bytes
  to avoid truncation.

Fair. Will be fixed in v2.

> +
>   static inline int read_file_at(int dirfd, const char *path, char *buf,
>   			       size_t buflen)
>   {
> @@ -193,19 +201,31 @@ static inline int read_u64_at(int dirfd, const char *path,
>   			      unsigned long long *out)
>   {
>   	char buf[32] = {0}, *end;
> -	int fd, ret;
> +	int ret;
>   
> -	fd = openat(dirfd, path, O_RDONLY);
> -	if (fd < 0)
> +	ret = read_file_at(dirfd, path, buf, sizeof(buf));
> +	if (ret <= 0)
>   		return -1;
>   
> -	ret = read(fd, buf, sizeof(buf) - 1);
> -	close(fd);
> +	errno = 0;
> +	*out = strtoull(buf, &end, 10);

Sashiko says:
  If read_file_at() fills the 32-byte buffer completely, does that remove
  the null terminator? If so, could strtoull() read out of bounds?

No, read_file_at is guaranteed to put \0 at the end of buffer.

> +	if (errno || end == buf)
> +		return -1;
> +	return 0;
> +}
> +
> +static inline int read_s32_at(int dirfd, const char *path,
> +			      int *out)
> +{
> +	char buf[32] = {0}, *end;
> +	int ret;
> +
> +	ret = read_file_at(dirfd, path, buf, sizeof(buf));
>   	if (ret <= 0)
>   		return -1;
>   
>   	errno = 0;
> -	*out = strtoull(buf, &end, 10);
> +	*out = strtol(buf, &end, 10);

Sashiko says:
  Does reading 32 bytes into a 32-byte buffer cause a similar out-of-bounds
  read issue here in read_s32_at() via strtol()?
  On 64-bit systems, strtol() returns a long. Can this lead to integer
  truncation when the result is stored in the 32-bit int pointer out?

1. No, same reason as above.
2. Maybe. It's worth adding range sanity check. Will do in v2.

>   	if (errno || end == buf)
>   		return -1;
>   	return 0;
> @@ -254,6 +274,52 @@ static inline int enter_cgroup(int cgv2_fd, int ctid)
>   	return ret;
>   }
>   
> +/*
> + * Run function in VE cgroup and new namespaces.
> + *
> + * Namespaces are provided via unshare_flags.
> + * CLONE_NEWVE flag is set by this function.
> + * Return values:
> + *  -  0 if function returns zero
> + *  - -1 if function returns negative value
> + *  -  1 if setup fails or function returns positive value
> + */
> +static inline int run_in_ve(int cgv2_fd, int ctid, struct namespace_ctx *ctx,
> +		int flags, int (*fn)(void *), void *arg)
> +{
> +	int status;
> +	pid_t pid;
> +
> +	pid = fork();
> +	if (pid < 0) {
> +		fprintf(stderr, "%s: fork failed\n", __func__);
> +		return 1;
> +	}
> +	if (pid == 0) {
> +		int ret;
> +
> +		if (enter_cgroup(cgv2_fd, ctid) < 0) {
> +			fprintf(stderr, "%s: enter_cgroup failed\n", __func__);
> +			_exit(255);
> +		}
> +		if (enter_namespaces(ctx, flags) < 0)
> +			_exit(255);
> +
> +		ret = fn(arg);
> +		if (ret < 0)
> +			ret = 1;
> +		else if (ret > 0)
> +			ret = 255;
> +		_exit(ret);
> +	}
> +	if (waitpid(pid, &status, 0) < 0 || !WIFEXITED(status) || WEXITSTATUS(status) == 255)
> +		return 1;
> +	if (WEXITSTATUS(status))
> +		return -1;
> +	return 0;
> +
> +}
> +

Sashiko says:
  Does the return logic in run_in_ve() mask the root cause of failures?
  By returning 1 for waitpid failures, signal terminations, and
  WEXITSTATUS 255, it seems setup failures and process crashes are
  collapsed into a single value.

True. While values still will collapse due to return value truncation,
it is worth separating fork/waitpid errors and fn setup errors.
Will do this in v2.

>   /*
>    * 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.
> 

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH VZ10 3/3] ve_namespace: Allow entering non-running VE namespace

[Vladimir Riabchun]

This doesn't bring a lot of value and makes tests more complex.
To enter VE namespace we need to actually start VE.

In vzctl we have locking to prevent parallel creations.
If someone decides to skip locks and start one CT
in parallel - big mistake, but not ours.

https://virtuozzo.atlassian.net/browse/VSTOR-144504

Feature: ve selftests
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 kernel/ve/ve_namespace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/ve/ve_namespace.c b/kernel/ve/ve_namespace.c
index 3dbe0dde2645..2ddec9ef312f 100644
--- a/kernel/ve/ve_namespace.c
+++ b/kernel/ve/ve_namespace.c
@@ -227,7 +227,9 @@ static int ve_ns_install(struct nsset *nsset, struct ns_common *new)
 		return -EPERM;
 
 	if (!VE_IS_RUNNING(ve_ns->ve))
-		return -EINVAL;
+		pr_warn_once("Switching to a non-running ve %s.\n"
+			"This should happen in kselftests only.\n",
+			ve_name(ve_ns->ve));
 
 	/*
 	 * Don't need to put_ve_ns(nsset->ve_ns) here,

-- 
2.47.1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH VZ10 3/3] ve_namespace: Allow entering non-running VE namespace

[Pavel Tikhomirov]

> This doesn't bring a lot of value and makes tests more complex.
> To enter VE namespace we need to actually start VE.
> 
> In vzctl we have locking to prevent parallel creations.
> If someone decides to skip locks and start one CT
> in parallel - big mistake, but not ours.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-144504
> 
> Feature: ve selftests
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
>
> diff --git a/kernel/ve/ve_namespace.c b/kernel/ve/ve_namespace.c
> index 3dbe0dde2645d..2ddec9ef312f9 100644
> --- a/kernel/ve/ve_namespace.c
> +++ b/kernel/ve/ve_namespace.c
> @@ -227,7 +227,9 @@ static int ve_ns_install(struct nsset *nsset, struct ns_common *new)
>  		return -EPERM;
>  
>  	if (!VE_IS_RUNNING(ve_ns->ve))
> -		return -EINVAL;
> +		pr_warn_once("Switching to a non-running ve %s.\n"
> +			"This should happen in kselftests only.\n",

nit: I would rather avoid mentioning kselftests in a general warning the
customers might see.

-- 
Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH VZ10 0/3] Improvements for ve selftests

[Pavel Tikhomirov]

> This change adds helpers for creating and preserving various
> namespaces and running functions inside them.
> This appeared useful while implementing ve mount accounting
> selftests - it required keeping mount namespace alive and
> doing different things at different moments of time.
> 
> To make setns work with CLONE_NEWVE without starting VE,
> the last patch changes -EINVAL return to warning message.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-144504
> 
> Feature: ve selftests
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

Code itself looks good. Removing the EINVAL does not seem to be braking
any syncronization, and simplifies the separate use of VE namespace for
tests.

I think we can go gradully and update it with actual ve START when we
will need it.

Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

-- 
Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel