[CRIU] [PATCH] cgroup: don't look up existing mount point
Pavel Emelyanov
xemul at parallels.com
Wed Jul 16 06:11:43 PDT 2014
On 07/15/2014 04:06 PM, Andrew Vagin wrote:
> 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 =================================
>
> Reported-by: Jenkins Criuovich
> Cc: Tycho Andersen <tycho.andersen at canonical.com>
> Signed-off-by: Andrew Vagin <avagin at openvz.org>
Plz, rebase the patch, it doesn't apply heavily.
> ---
> cgroup.c | 123 ++++++++++++++-------------------------------------------------
> 1 file changed, 26 insertions(+), 97 deletions(-)
>
> diff --git a/cgroup.c b/cgroup.c
> index 5114935..9ee54d3 100644
> --- a/cgroup.c
> +++ b/cgroup.c
> @@ -14,6 +14,7 @@
> #include "proc_parse.h"
> #include "util.h"
> #include "fdset.h"
> +#include "util-pie.h"
> #include "protobuf.h"
> #include "protobuf/core.pb-c.h"
> #include "protobuf/cgroup.pb-c.h"
> @@ -44,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)
> {
> @@ -161,53 +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;
> }
>
> -static int get_cgroup_mount_point(const char *controller, char *path)
> -{
> - struct mount_info *m;
> - char name[1024];
> -
> - for (m = cg_mntinfo; m != NULL; m = m->next) {
> - if (strcmp(m->fstype->name, "cgroup") == 0) {
> - char *start, *end;
> -
> - start = strstr(m->options, "name=");
> - if (start) {
> - /* strlen("name=") == 5 */
> - start = start + 5;
> -
> - end = strstr(start, ",");
> - if (end) {
> - strncpy(name, start, end - start);
> - name[end - start] = '\0';
> - } else
> - strcpy(name, start);
> - } else {
> - start = strrchr(m->mountpoint, '/');
> - if (!start) {
> - pr_err("bad path %s\n", m->mountpoint);
> - return -1;
> - }
> - strcpy(name, start+1);
> - }
> -
> - if (strcmp(name, controller) == 0) {
> - /* skip the leading '.' in mountpoint */
> - strcpy(path, m->mountpoint + 1);
> - return 0;
> - }
> - }
> - }
> -
> - return -1;
> -}
> -
> /* Check that co-mounted controllers from /proc/cgroups (e.g. cpu and cpuacct)
> * are contained in a name from /proc/self/cgroup (e.g. cpu,cpuacct). */
> bool cgroup_contains(char **controllers, unsigned int n_controllers, char *name)
> @@ -274,7 +230,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);
>
> @@ -287,27 +242,11 @@ static int add_cgroup(const char *fpath, const struct stat *sb, int typeflag)
> }
> 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, ¤t_controller->heads, &match);
> @@ -346,11 +285,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 *name, mount_point[PATH_MAX], prefix[] = ".criu.cgmounts.XXXXXX";
> - bool temp_mount = false;
> + char path[PATH_MAX], opts[1024];
> + char *name, prefix[] = ".criu.cgmounts.XXXXXX";
> struct cg_controller *cg;
>
> if (strstartswith(cc->name, "name="))
> @@ -358,34 +297,27 @@ static int collect_cgroups(struct list_head *ctls)
> else
> name = cc->name;
>
> - if (get_cgroup_mount_point(name, mount_point) < 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;
> -
> - 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(mount_point, prefix);
> + if (mount("none", prefix, "cgroup", 0, opts) < 0) {
> + pr_perror("couldn't mount %s\n", opts);
> + rmdir(prefix);
> + return -1;
> }
>
> - snprintf(path, PATH_MAX, "%s/%s", mount_point, cc->path);
> + 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;
>
> @@ -420,10 +352,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;
>
More information about the CRIU
mailing list