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

Konstantin Khorenko khorenko at virtuozzo.com
Fri Feb 19 03:37:04 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 9a77a902c3c2bf977b6d9c87d8b08cb719d3e15d
Author: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
Date:   Fri Feb 19 15:37:04 2016 +0400

    ve: move task-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:
    
    ve_can_attach() is messy and unclear. This patch moves all task-related checks
    to a separated ve_task_can_attach() helper.
    
    Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
    Reviewed-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 kernel/ve/ve.c | 55 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index 72dc2d1..67783a5 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -700,6 +700,28 @@ static void free_ve_devmnts(struct ve_struct *ve)
 	}
 }
 
+static bool ve_task_can_attach(struct cgroup *cg, struct cgroup_taskset *tset)
+{
+	struct task_struct *task = cgroup_taskset_first(tset);
+
+	if (cgroup_taskset_size(tset) > 1) {
+		pr_err_ratelimited("ve_cgroup#%s: attach of a thread group is not supported\n",
+				cg->name->name);
+		return false;
+	}
+	if (!thread_group_leader(task)) {
+		pr_err_ratelimited("ve_cgroup#%s: only thread group leader is allowed to attach\n",
+				cg->name->name);
+		return false;
+	}
+	if (!thread_group_empty(task)) {
+		pr_err_ratelimited("ve_cgroup#%s: only single-threaded process is allowed to attach\n",
+				cg->name->name);
+		return false;
+	}
+	return true;
+}
+
 static void ve_destroy(struct cgroup *cg)
 {
 	struct ve_struct *ve = cgroup_ve(cg);
@@ -723,31 +745,18 @@ static int ve_can_attach(struct cgroup *cg, struct cgroup_taskset *tset)
 	if (!ve->veid)
 		return -ENOENT;
 
-	/*
-	 * We allow only one single-threaded process to attach
-	 * into a container, which usually stands for "init"
-	 * process. The rest of processes should be forked
-	 * from the "init".
-	 */
-	if (cgroup_taskset_size(tset) == 1) {
-		struct task_struct *task = cgroup_taskset_first(tset);
-
-		if (!thread_group_leader(task) ||
-		    !thread_group_empty(task))
-			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 (!(task->flags & PF_KTHREAD))
-				return -EPIPE;
-		}
-	} else
+	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;
 }
 


More information about the Devel mailing list