[CRIU] [PATCH 1/2] cgroup: don't look up existing mount point

Andrew Vagin avagin at openvz.org
Wed Jul 16 06:46:44 PDT 2014


A mount point, which is mounted by someone else, may be umounted in
any moment.

For example the test system executes tests concurrently and sometimes
one test looks up a mount point, which has been mounted by another test.

==================================== ERROR ====================================
Test: zdtm/live/static/inotify00, Namespace: 1
Dump log   : /var/lib/jenkins/jobs/CRIU-dump/workspace/test/dump/inotify00/15535/1/dump.log
--------------------------------- grep Error ---------------------------------
(00.021951) Error (cgroup.c:409): cg: failed walking /var/lib/jenkins/jobs/CRIU-dump/workspace/test/dump/signalfd00/15538/1/.criu.cgmounts.UGj28v/ for empty cgroups
(00.021967) Error (cr-dump.c:1601): Dump core (pid: 15535) failed with -1
(00.025509) Error (cr-dump.c:1914): Dumping FAILED.
------------------------------------- END -------------------------------------
================================= ERROR OVER =================================

In the previous patch I suggested to open a mount point, but it brought
other problems. We may open a directory where a cgroup mount has been
umounted and an owner will get EBUSY on attempt to remove this
directory.

Reported-by: Jenkins Criuovich
Acked-by: Tycho Andersen <tycho.andersen at canonical.com>
Signed-off-by: Andrew Vagin <avagin at openvz.org>
---
 cgroup.c | 113 ++++++++++++++-------------------------------------------------
 1 file changed, 25 insertions(+), 88 deletions(-)

diff --git a/cgroup.c b/cgroup.c
index 0a1ef42..71ac271 100644
--- a/cgroup.c
+++ b/cgroup.c
@@ -14,7 +14,7 @@
 #include "proc_parse.h"
 #include "util.h"
 #include "fdset.h"
-#include "string.h"
+#include "util-pie.h"
 #include "protobuf.h"
 #include "protobuf/core.pb-c.h"
 #include "protobuf/cgroup.pb-c.h"
@@ -45,7 +45,6 @@ static u32 cg_set_ids = 1;
 
 static LIST_HEAD(cgroups);
 static unsigned int n_cgroups;
-static struct mount_info *cg_mntinfo;
 
 static CgSetEntry *find_rst_set_by_id(u32 id)
 {
@@ -162,14 +161,9 @@ int parse_cg_info(void)
 	if (parse_cgroups(&cgroups, &n_cgroups) < 0)
 		return -1;
 
-	cg_mntinfo = parse_mountinfo(getpid(), NULL);
-
-	if (!cg_mntinfo)
-		return -1;
 	return 0;
 }
 
-
 /* Check that co-mounted controllers from /proc/cgroups (e.g. cpu and cpuacct)
  * are contained in a comma separated string (e.g. from /proc/self/cgroup or
  * mount options). */
@@ -198,33 +192,6 @@ static bool cgroup_contains(char **controllers, unsigned int n_controllers, char
 	return all_match && n_controllers > 0;
 }
 
-static int get_cgroup_mount_point(char *controller, char *path)
-{
-	struct mount_info *m;
-	char **names;
-	int n_names, i, ret = -1;
-
-	split(controller, ',', &names, &n_names);
-	if (!names)
-		return -1;
-
-	for (m = cg_mntinfo; m != NULL; m = m->next) {
-		if (strcmp(m->fstype->name, "cgroup") == 0 &&
-				cgroup_contains(names, n_names, m->options)) {
-			/* skip the leading '.' in mountpoint */
-			strcpy(path, m->mountpoint + 1);
-			ret = 0;
-			goto out;
-		}
-	}
-
-out:
-	for (i = 0; i < n_names; i++)
-		xfree(names[i]);
-	xfree(names);
-	return ret;
-}
-
 /* This is for use in add_cgroup() as additional arguments for the ftw()
  * callback */
 static struct cg_controller	*current_controller;
@@ -264,7 +231,6 @@ static int add_cgroup(const char *fpath, const struct stat *sb, int typeflag)
 
 	if (typeflag == FTW_D) {
 		int mtype;
-		struct mount_info *mi;
 
 		strncpy(pbuf, fpath, PATH_MAX);
 
@@ -275,29 +241,12 @@ static int add_cgroup(const char *fpath, const struct stat *sb, int typeflag)
 			ret = -1;
 			goto out;
 		}
-		ncd->path = NULL;
-
-		for (mi = cg_mntinfo; mi != NULL; mi = mi->next) {
-			if (is_path_prefix(fpath, mi->mountpoint + 1)) {
-				ncd->path = xstrdup(fpath + strlen(mi->mountpoint));
-				if (!ncd->path) {
-					ret = -1;
-					goto out;
-				}
-				break;
-			}
-		}
 
+		/* chop off the first strlen(".criu.cgmounts.XXXXXX") */
+		ncd->path = xstrdup(fpath + 21);
 		if (!ncd->path) {
-			/* We couldn't find fpath in mountinfo, which means we
-			 * mounted it ourselves, so we just chop off the first
-			 * strlen(".criu.cgmounts.XXXXXX").
-			 */
-			ncd->path = xstrdup(fpath + 21);
-			if (!ncd->path) {
-				ret = -1;
-				goto out;
-			}
+			ret = -1;
+			goto out;
 		}
 
 		mtype = find_dir(ncd->path, &current_controller->heads, &match);
@@ -335,11 +284,11 @@ static int collect_cgroups(struct list_head *ctls)
 {
 	struct cg_ctl *cc;
 	int ret = 0;
+	int fd = -1;
 
 	list_for_each_entry(cc, ctls, l) {
-		char path[PATH_MAX];
+		char path[PATH_MAX], opts[1024];
 		char *name, prefix[] = ".criu.cgmounts.XXXXXX";
-		bool temp_mount = false;
 		struct cg_controller *cg;
 
 		if (strstartswith(cc->name, "name="))
@@ -347,36 +296,27 @@ static int collect_cgroups(struct list_head *ctls)
 		else
 			name = cc->name;
 
-		if (get_cgroup_mount_point(name, path) < 0) {
-			/* Someone is trying to dump a process that is in
-			 * a controller that isn't mounted, so we mount it for
-			 * them.
-			 */
-			char opts[1024];
-			temp_mount = true;
-
-			pr_info("Couldn't find mount point for %s mounting..\n", name);
-
-			if (mkdtemp(prefix) == NULL) {
-				pr_perror("can't make dir for cg mounts\n");
-				return -1;
-			}
-
-			if (name == cc->name)
-				sprintf(opts, "%s", name);
-			else
-				sprintf(opts, "none,%s", cc->name);
+		if (mkdtemp(prefix) == NULL) {
+			pr_perror("can't make dir for cg mounts\n");
+			return -1;
+		}
 
-			if (mount("none", prefix, "cgroup", 0, opts) < 0) {
-				pr_perror("couldn't mount %s\n", opts);
-				rmdir(prefix);
-				return -1;
-			}
+		if (name == cc->name)
+			sprintf(opts, "%s", name);
+		else
+			sprintf(opts, "none,%s", cc->name);
 
-			strcpy(path, prefix);
+		if (mount("none", prefix, "cgroup", 0, opts) < 0) {
+			pr_perror("couldn't mount %s\n", opts);
+			rmdir(prefix);
+			return -1;
 		}
 
-		strlcat(path, cc->path, PATH_MAX);
+		fd = open_detach_mount(prefix);
+		if (fd < 0)
+			return -1;
+
+		snprintf(path, PATH_MAX, "/proc/self/fd/%d/%s", fd, cc->path);
 
 		current_controller = NULL;
 
@@ -411,10 +351,7 @@ static int collect_cgroups(struct list_head *ctls)
 		}
 
 out:
-		if (temp_mount) {
-			umount(prefix);
-			rmdir(prefix);
-		}
+		close_safe(&fd);
 
 		if (ret < 0)
 			return ret;
-- 
1.9.3



More information about the CRIU mailing list