[CRIU] [PATCH] cg: don't try to copy special props to cpuset:/

[Tycho Andersen]

On Fri, Oct 03, 2014 at 12:47:39PM +0400, Pavel Emelyanov wrote:
> On 10/03/2014 03:40 AM, Tycho Andersen wrote:
> > We can't write to cpuset's root cg (indeed, we didn't create it and it has
> > stuff in it), so ignore this special case.
> 
> Wait a second. I though we should avoid writing the "special props"
> to all directories that we did *not* created on restore. Is this
> assumption correct?

Yes, that's what we should do (and you're right that this patch
doesn't guarantee that). However, that does mean that users could set
their cgroups up in a way that the restore can't succeed.

Another issue is that if the user migrates to a node with fewer CPUs,
the restore will fail. e.g. if they have a cpuset.cpus == 0-3, and
they migrate to a machine with only two cores (cpuset.cpus == 0-1), we
will try to write 0-3 in and fail.

I will think about a better way to do this patch. In the meantime, the
first hunk of this diff needs to be applied since it just fixes a
typo. I will resend that.

Tycho

> > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > ---
> >  cgroup.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/cgroup.c b/cgroup.c
> > index 29e373d..36e3487 100644
> > --- a/cgroup.c
> > +++ b/cgroup.c
> > @@ -886,7 +886,7 @@ static int find_value(const char *controller, const char *path, const char *prop
> >  			*pos = 0;
> >  			snprintf(prefix, PATH_MAX, "%s/%s", controller, my_path);
> >  		} else {
> > -			snprintf(fpath, PATH_MAX, "%s", controller);
> > +			snprintf(prefix, PATH_MAX, "%s", controller);
> >  		}
> >  		snprintf(fpath, PATH_MAX, "%s/%s", prefix, prop);
> >  
> > @@ -964,6 +964,17 @@ static int copy_special_cg_props(const char *controller, char *path)
> >  			if (find_value(controller, path, prop, buf, prefix, missing_path) < 0)
> >  				return -1;
> >  
> > +			/*
> > +			 * If the path that was missing is "/", we should skip
> > +			 * copying this property, since cpuset won't allow us
> > +			 * to write to it because there are already processes
> > +			 * there (e.g. init). The root cg had no restrictions
> > +			 * on cpuset resources here, so ignoring this and
> > +			 * writing nothing is safe.
> > +			 */
> > +			if (strcmp(missing_path, "/") == 0)
> > +				continue;
> > +
> >  			if (copy_recursive(prefix, missing_path, prop, buf) < 0) {
> >  				pr_err("copying prop %s failed\n", prop);
> >  				return -1;
> > 
>