[CRIU] [PATCH 1/5] mnt: add --ext-mount-map auto option

[Tycho Andersen]

On Tue, Apr 14, 2015 at 07:53:16PM +0300, Andrew Vagin wrote:
> On Tue, Apr 14, 2015 at 10:26:17AM -0600, Tycho Andersen wrote:
> > On Tue, Apr 14, 2015 at 07:22:27PM +0300, Andrew Vagin wrote:
> > > On Tue, Apr 14, 2015 at 09:36:54AM -0600, Tycho Andersen wrote:
> > > > On Tue, Apr 14, 2015 at 11:06:49AM +0300, Andrew Vagin wrote:
> > > > > On Mon, Apr 13, 2015 at 08:58:21AM -0600, Tycho Andersen wrote:
> > > > > > On Mon, Apr 13, 2015 at 11:24:06AM +0300, Andrew Vagin wrote:
> > > > > > > On Fri, Apr 10, 2015 at 02:34:37PM +0000, Tycho Andersen wrote:
> > > > > > > > +static int resolve_external_mounts(struct mount_info *info)
> > > > > > > > +{
> > > > > > > > +	struct mount_info *m;
> > > > > > > > +	struct ns_id *ns = NULL, *iter;
> > > > > > > > +
> > > > > > > > +	for (iter = ns_ids; iter->next; iter = iter->next) {
> > > > > > > > +		if (iter->pid == getpid() && iter->nd == &mnt_ns_desc) {
> > > > > > > > +			ns = iter;
> > > > > > > > +			break;
> > > > > > > > +		}
> > > > > > > > +	}
> > > > > > > > +
> > > > > > > > +	if (!ns) {
> > > > > > > > +		pr_err("Failed to find criu pid's mount ns!");
> > > > > > > > +		return -1;
> > > > > > > > +	}
> > > > > > > > +
> > > > > > > > +	for (m = info; m; m = m->next) {
> > > > > > > > +		int ret, size;
> > > > > > > > +		char *p;
> > > > > > > > +		struct ext_mount *em;
> > > > > > > > +		struct mount_info *match;
> > > > > > > > +
> > > > > > > > +		if (m->parent == NULL || m->is_ns_root)
> > > > > > > > +			continue;
> > > > > > > > +
> > > > > > > > +		ret = try_resolve_ext_mount(m);
> > > > > > > > +		if (ret < 0 && ret != -ENOTSUP) {
> > > > > > > > +			return -1;
> > > > > > > > +		} else if (ret == -ENOTSUP && !opts.autodetect_ext_mounts) {
> > > > > > > > +			continue;
> > > > > > > > +		} else if (ret == 0) {
> > > > > > > > +			continue;
> > > > > > > > +		}
> > > > > > > > +
> > > > > > > > +		match = find_best_external_match(ns->mnt.mntinfo_list, m);
> > > > > > > > +		if (!match)
> > > > > > > > +			continue;
> > > > > > > > +
> > > > > > > > +		size = strlen(match->mountpoint + 1) + strlen(m->root) + 1;
> > > > > > > > +		p = xmalloc(sizeof(char) * size);
> > > > > > > > +		if (!p)
> > > > > > > > +			return -1;
> > > > > > > > +
> > > > > > > > +		ret = snprintf(p, size+1, "%s%s", match->mountpoint + 1, m->root);
> > > > > > > 
> > > > > > > Is it ok if match->root isn't /?
> > > > > > 
> > > > > > It should be, presumably that just means there were some extra steps
> > > > > > to set up the external mount point. Am I missing something?
> > > > > 
> > > > > Maybe we need something like this:
> > > > > 
> > > > > snprintf(p, size+1, "%s%s", match->mountpoint + 1, m->root + strlen(match->root));
> > > > 
> > > > I don't think m->root and match->root are relatable at all, though.
> > > > Consider:
> > > > 
> > > > in the container:
> > > > 
> > > > 94 93 0:21 /cgmanager /sys/fs/cgroup/cgmanager rw master:9 - tmpfs tmpfs rw,mode=755
> > > > 
> > > > on the host:
> > > > 
> > > > 27 17 0:21 / /sys/fs/cgroup rw shared:9 - tmpfs tmpfs rw,mode=755
> > > 
> > > Let's imagine that the host contains:
> > > 27 17 0:21 /sys/fs/cgroup/cgmanager /sys/fs/cgroup/cgmanager rw shared:9 - tmpfs tmpfs rw,mode=755
> > > 
> > > How this situation should be handled?
> > > 
> > > snprintf(p, size+1, "%s%s", "/sys/fs/cgroup/cgmanager", "/sys/fs/cgroup/cgmanager");
> > 
> > Doesn't the container then have something like:
> > 
> > 94 93 0:21 / /sys/fs/cgroup/cgmanager rw shared:9 - tmpfs tmpfs rw,mode=755
> > 
> 
> No, it doesn't. The root will be /sys/fs/cgroup/cgmanager.
> 
> You can try to run following commands
> 
> [root at avagin]# mkdir test
> [root at avagin]# mount -t tmpfs test test
> [root at avagin]# mkdir test/a
> [root at avagin]# mkdir test/b
> [root at avagin]# mkdir test/c
> [root at avagin]# mount -t tmpfs test/c test/c
> [root at avagin]# mkdir test/c/dir
> [root at avagin]# mount --bind test/c/dir test/a
> [root at avagin]# mount --bind test/a test/b
> [root at avagin]# umount test/c
> [root at avagin]# cat /proc/self/mountinfo 
> 143 140 8:3 / / rw,relatime - ext4 /dev/sda3 rw,data=ordered
> 149 143 0:4 / /proc rw,nosuid,nodev,noexec,relatime - proc proc rw
> 151 143 0:47 / /root/tmp/test rw,relatime - tmpfs test rw
> 153 151 0:48 /dir /root/tmp/test/a rw,relatime - tmpfs test/c rw
> 154 151 0:48 /dir /root/tmp/test/b rw,relatime - tmpfs test/c rw
> # Create "CT"
> [root at avagin]# unshare -m
> [root at avagin]# umount test/a/
> [root at avagin]# cat /proc/self//mountinfo 
> 155 152 8:3 / / rw,relatime - ext4 /dev/sda3 rw,data=ordered
> 161 155 0:4 / /proc rw,nosuid,nodev,noexec,relatime - proc proc rw
> 164 155 0:47 / /root/tmp/test rw,relatime - tmpfs test rw
> 173 164 0:48 /dir /root/tmp/test/b rw,relatime - tmpfs test/c rw

When I do a:

mount --bind /sys/fs/cgroup/cgmanager/ /sys/fs/cgroup/cgmanager/

on the host, I still end up with:

96 95 0:21 /cgmanager /sys/fs/cgroup/cgmanager rw master:9 - tmpfs tmpfs rw,mode=755

in the container. However, a restore with this bind mount in place
fails:

(00.100974)      1: 168:./sys/fs/cgroup private 0 shared 0 slave 0
(00.103660)      1:   Mounting tmpfs @./sys/fs/cgroup/cgmanager (0)
(00.103678)      1:   Bind /sys/fs/cgroup/cgmanager/cgmanager to ./sys/fs/cgroup/cgmanager

so something is obviously wrong. I'm not sure I understand your example,
though. It doesn't have the full path, and neither does the above?

I'm also not sure how we tell the difference between when this bind mount
exists and when it doesn't, since as far as I can tell the mountinfo for both
of them inside the container is the same.

Tycho

> 
> > so it ends up being
> > 
> > snprintf(p, size+1, "%s%s", "/sys/fs/cgroup/cgmanager", "/");
> > 
> > ?
> > 
> > Tycho
> > 
> > > p = "/sys/fs/cgroup/cgmanager/sys/fs/cgroup/cgmanager"
> > > 
> > > I think it isn't what we want to get. Sorry if I miss something.
> > > 
> > > > 
> > > > Here /sys/fs/cgroup/cgmanager is bind mounted from the host to inside
> > > > the container (to the same path, not that it matters).
> > > > 
> > > > Tycho
> > > > 
> > > > > and check that strlen(match->root) <= strlen(m->root) in
> > > > > find_best_external_match().
> > > > > 
> > > > > Thanks,
> > > > > Andrew