From 4d137b81a0b6e4c5dc5bb093d2cbbc6ea04cb486 Mon Sep 17 00:00:00 2001
From: Valeriy Vdovin <valeriy.vdovin@virtuozzo.com>
Date: Tue, 9 Feb 2021 16:55:48 +0300
Subject: [PATCH] cgroup/restore: split prepare_task_cgroup code into two
 separate functions

This does cgroup namespace creation separately from joining task
cgroups. This makes the code more logical, because creating cgroup
namespace also involves joining cgroups but these cgroups can be
different to task's cgroups as they are cgroup namespace roots
(cgns_prefix), and mixing all of them together may lead to
misunderstanding.

Another positive thing is that we consolidate !item->parent checks in
one place in restore_task_with_children.

Signed-off-by: Valeriy Vdovin <valeriy.vdovin@virtuozzo.com>
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 criu/cgroup.c         | 54 ++++++++++++++++++++++++++++++++-----------
 criu/cr-restore.c     |  9 +++++++-
 criu/include/cgroup.h |  3 ++-
 3 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/criu/cgroup.c b/criu/cgroup.c
index 8243ac6d3c..bcb7b405a3 100644
--- a/criu/cgroup.c
+++ b/criu/cgroup.c
@@ -1202,17 +1202,12 @@ static int prepare_cgns(CgSetEntry *se)
 	return 0;
 }
 
-static int move_in_cgroup(CgSetEntry *se, bool setup_cgns)
+static int move_in_cgroup(CgSetEntry *se)
 {
 	int i;
 
 	pr_info("Move into %d\n", se->id);
 
-	if (setup_cgns && prepare_cgns(se) < 0) {
-		pr_err("failed preparing cgns\n");
-		return -1;
-	}
-
 	for (i = 0; i < se->n_ctls; i++) {
 		char aux[PATH_MAX];
 		int fd = -1, err, j, aux_off;
@@ -1252,7 +1247,44 @@ static int move_in_cgroup(CgSetEntry *se, bool setup_cgns)
 	return 0;
 }
 
-int prepare_task_cgroup(struct pstree_item *me)
+int prepare_cgroup_namespace(struct pstree_item *root_task)
+{
+	CgSetEntry *se;
+
+	if (opts.manage_cgroups == CG_MODE_IGNORE)
+		return 0;
+
+	if (root_task->parent) {
+		pr_err("Expecting root_task to restore cgroup namespace\n");
+		return -1;
+	}
+
+	/*
+	 * If on dump all dumped tasks are in same cgset with criu we don't
+	 * dump cgsets and thus cgroup namespaces and rely that on restore
+	 * criu caller would prepare proper cgset/cgns for us. Also in case
+	 * of --unprivileged we don't even have the root cgset here.
+	 */
+	if (!rsti(root_task)->cg_set || rsti(root_task)->cg_set == root_cg_set) {
+		pr_info("Cgroup namespace inherited from parent\n");
+		return 0;
+	}
+
+	se = find_rst_set_by_id(rsti(root_task)->cg_set);
+	if (!se) {
+		pr_err("No set %d found\n", rsti(root_task)->cg_set);
+		return -1;
+	}
+
+	if (prepare_cgns(se) < 0) {
+		pr_err("failed preparing cgns\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+int restore_task_cgroup(struct pstree_item *me)
 {
 	struct pstree_item *parent = me->parent;
 	CgSetEntry *se;
@@ -1284,13 +1316,7 @@ int prepare_task_cgroup(struct pstree_item *me)
 		return -1;
 	}
 
-	/* Since don't support nesting of cgroup namespaces, let's only set up
-	 * the cgns (if it exists) in the init task. In the future, we should
-	 * just check that the cgns prefix string matches for all the entries
-	 * in the cgset, and only unshare if that's true.
-	 */
-
-	return move_in_cgroup(se, !me->parent);
+	return move_in_cgroup(se);
 }
 
 void fini_cgroup(void)
diff --git a/criu/cr-restore.c b/criu/cr-restore.c
index f02e95f6d2..2b99a775d8 100644
--- a/criu/cr-restore.c
+++ b/criu/cr-restore.c
@@ -1827,6 +1827,13 @@ static int restore_task_with_children(void *_arg)
 		/* Wait prepare_userns */
 		if (restore_finish_ns_stage(CR_STATE_ROOT_TASK, CR_STATE_PREPARE_NAMESPACES) < 0)
 			goto err;
+
+		/*
+		 * Since we don't support nesting of cgroup namespaces, let's
+		 * only set up the cgns (if it exists) in the init task.
+		 */
+		if (prepare_cgroup_namespace(current) < 0)
+			goto err;
 	}
 
 	if (needs_prep_creds(current) && (prepare_userns_creds()))
@@ -1838,7 +1845,7 @@ static int restore_task_with_children(void *_arg)
 	 * we will only move the root one there, others will
 	 * just have it inherited.
 	 */
-	if (prepare_task_cgroup(current) < 0)
+	if (restore_task_cgroup(current) < 0)
 		goto err;
 
 	/* Restore root task */
diff --git a/criu/include/cgroup.h b/criu/include/cgroup.h
index 93f61539cf..dc264032e8 100644
--- a/criu/include/cgroup.h
+++ b/criu/include/cgroup.h
@@ -9,7 +9,8 @@ struct parasite_dump_cgroup_args;
 extern u32 root_cg_set;
 int dump_thread_cgroup(const struct pstree_item *, u32 *, struct parasite_dump_cgroup_args *args, int id);
 int dump_cgroups(void);
-int prepare_task_cgroup(struct pstree_item *);
+int restore_task_cgroup(struct pstree_item *);
+int prepare_cgroup_namespace(struct pstree_item *);
 int prepare_cgroup(void);
 /* Restore things like cpu_limit in known cgroups. */
 int prepare_cgroup_properties(void);