[CRIU] [PATCH] Attempt to restore cgroups

Tycho Andersen tycho.andersen at canonical.com
Tue Jul 8 10:37:54 PDT 2014


Hi Pavel,

On Tue, Jul 08, 2014 at 06:30:35PM +0400, Pavel Emelyanov wrote:
> On 07/08/2014 01:33 AM, Tycho Andersen wrote:
> > On Mon, Jul 07, 2014 at 04:28:00PM -0500, Tycho Andersen wrote:
> >> ...
> > 
> > Ok, I think this fixes everything that has been mentioned so far in
> > the thread. Thoughts and comments welcome!
> 
> Yup, thanks Tycho! I have only one concern about the patch:
> 
> > +		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], prefix[] = ".criu.cgmounts.XXXXXX";
> > +
> > +			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 (mount("none", prefix, "cgroup", 0, opts) < 0) {
> 
> This temporary mount doesn't get unmounted.
> 
> > +				pr_perror("couldn't mount %s\n", opts);
> > +				return -1;
> > +			}
> > +
> > +			strcpy(mount_point, prefix);
> > +		}
> 
> Other than this, everything looks great! So, if you can temporary remove 
> the per-cgroup configuration dump and restore (you told it was put there 
> just for demonstation) I will happily merge the patch :)

Ok, I think I've fixed both of those, as well as one other small bug
the tests revealed.

Thanks,

Tycho


More information about the CRIU mailing list