[CRIU] [PATCH] cgroup: actually detect cgroupns bind mounts correctly

[Tycho Andersen]

On Wed, Aug 10, 2016 at 04:35:18PM +0300, Pavel Emelyanov wrote:
> On 08/09/2016 01:36 AM, Tycho Andersen wrote:
> > Initially, the cgroupns patchset that went into the Ubuntu kernels had the
> > nsroot= argument in mountopts, which allowed us to distinguish between
> > cgroupfs mounts. The upstream kernel doesn't have this (the reasoning was
> > that it showed up in the root field of mountinfo), so all cgroup mounts
> > look idential if we don't consider the root mount, and we wrongly detect
> > the mounts as bind mounts. Instead, let's remember the root path and
> > compare this when considering when a mount is a bind mount.
> > 
> > This patch is a little ugly because it intorduces cgroup specific behavior
> > into the generic code, which we've thus far been able to avoid. One way to
> > deal with this might be able to add a ->sb_equal hook to the fstype struct
> > that could compare mountinfos' superblocks, and then fall back to
> > mounts_sb_equal if that isn't present. Or we can leave this specific code
> > in the generic bits until more of these appear. Either way is fine with me.
> > 
> > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > ---
> >  criu/mount.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/criu/mount.c b/criu/mount.c
> > index f90ec69..de13915 100644
> > --- a/criu/mount.c
> > +++ b/criu/mount.c
> > @@ -721,6 +721,9 @@ static struct mount_info *find_best_external_match(struct mount_info *list, stru
> >  		if (!mounts_sb_equal(info, it))
> >  			continue;
> >  
> > +		if (info->fstype->code == FSTYPE__CGROUP && strcmp(info->private, it->private))
> > +			continue;
> 
> Shouldn't this be rather in mounts_sb_equal()?

It could be, and I suppose it should be too, I can resend.

> And, BTW, what's there in cgroup's mi->private?

It's the original root path from pm->root, see the hunk below.

Tycho

> > +
> >  		/*
> >  		 * This means we have a situation like:
> >  		 *
> > @@ -1602,7 +1605,7 @@ static int cgroup_parse(struct mount_info *pm)
> >  	/* cgroup namespaced mounts don't look rooted to CRIU, so let's fake it
> >  	 * here.
> >  	 */
> > -	xfree(pm->root);
> > +	pm->private = pm->root;
> >  	pm->root = xstrdup("/");
> >  	if (!pm->root)
> >  		return -1;
> > 
>