[Devel] [PATCH RHEL7 COMMIT] ve: move ve-related checks to separated function from ve_can_attach()

Konstantin Khorenko khorenko at virtuozzo.com
Fri Feb 19 03:37:05 PST 2016


The commit is pushed to "branch-rh7-3.10.0-327.4.5.vz7.11.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-327.4.5.vz7.11.4
------>
commit 4738d245d27354b82ffbb820973ad436fca6a464
Author: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
Date:   Fri Feb 19 15:37:05 2016 +0400

    ve: move ve-related checks to separated function from ve_can_attach()
    
    Patch series description:
    
    Inspired by https://jira.sw.ru/browse/PSBM-44143
    
    Function ve_can_attach is a mess: it's unclear, review is complicated, some of
    the hunks are simply copied without explanation, some - old code without
    comments and clear understanding of the introductions reasons.
    Wha't even worse is that no error messages are printed in case of failure.
    This series tries to rework this code to make it clearer by splitting
    independent parts, adding comments and error messages.
    
    v2:
    1) Check to empty process group returned back.
    2) Error messages patch merged with corresponding code introducing ones.
    
    =======================
    This patch description:
    
    Patch moves all the ve-related checks to another new helper function
    ve_is_attachable().
    
    Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
    Reviewed-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 kernel/ve/ve.c | 53 ++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index 67783a5..84fd806 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -722,6 +722,43 @@ static bool ve_task_can_attach(struct cgroup *cg, struct cgroup_taskset *tset)
 	return true;
 }
 
+static int ve_is_attachable(struct cgroup *cg, struct cgroup_taskset *tset)
+{
+	struct task_struct *task = cgroup_taskset_first(tset);
+	struct ve_struct *ve = cgroup_ve(cg);
+
+	if (ve->is_running)
+		return 0;
+
+	if (!ve->veid) {
+		pr_err_ratelimited("ve_cgroup#%s: container's veid is not set\n",
+				cg->name->name);
+		return -EINVAL;
+	}
+
+	if (task->flags & PF_KTHREAD) {
+		/* Paranoia check: allow to attach kthread only, if cgroup is
+		 * not empty.
+		 * This check is required for kthreadd, which is created on CT
+		 * start.
+		 */
+		if (nr_threads_ve(ve))
+			return 0;
+		pr_err_ratelimited("ve_cgroup#%s: can't attach kthread - empty group\n",
+				cg->name->name);
+	} else {
+		/* In case of generic task only one is allowed to enter to
+		 * non-running container: init.
+		 */
+		if (nr_threads_ve(ve) == 0)
+			return 0;
+		pr_err_ratelimited("ve_cgroup#%s: can't attach more than 1 task to "
+				"non-running container\n",
+				cg->name->name);
+	}
+	return -EINVAL;
+}
+
 static void ve_destroy(struct cgroup *cg)
 {
 	struct ve_struct *ve = cgroup_ve(cg);
@@ -740,24 +777,10 @@ static void ve_destroy(struct cgroup *cg)
 
 static int ve_can_attach(struct cgroup *cg, struct cgroup_taskset *tset)
 {
-	struct ve_struct *ve = cgroup_ve(cg);
-
-	if (!ve->veid)
-		return -ENOENT;
-
 	if (!ve_task_can_attach(cg, tset))
 		return -EINVAL;
 
-	/*
-	 * XXX Still permit attaching kernel threads
-	 * for this container. Wonder if we really need it,
-	 * looks like some legacy code chunk.
-	 */
-	if (!ve->is_running && (ve->ve_ns || nr_threads_ve(ve))) {
-		if (!(cgroup_taskset_first(tset)->flags & PF_KTHREAD))
-			return -EPIPE;
-	}
-	return 0;
+	return ve_is_attachable(cg, tset);
 }
 
 static void ve_update_cpuid_faulting(void *dummy)


More information about the Devel mailing list