[CRIU] [PATCH 1/2] Re-create cgroups if necessary

[Pavel Emelyanov]

On 06/24/2014 05:59 PM, Tycho Andersen wrote:
> Hi Pavel,
> 
> On Tue, Jun 24, 2014 at 11:03:14AM +0400, Pavel Emelyanov wrote:
>> On 06/23/2014 07:54 PM, Tycho Andersen wrote:
>>> If a task had cgroups, these should be restored. (Also any properties of
>>> cgroups, but this commit doesn't do that yet.)
>>>
>>> Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
>>> ---
>>>  cgroup.c       |  6 +++++-
>>>  include/util.h |  5 +++++
>>>  util.c         | 26 ++++++++++++++++++++++++++
>>>  3 files changed, 36 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/cgroup.c b/cgroup.c
>>> index 2d9ebad..8b2deb2 100644
>>> --- a/cgroup.c
>>> +++ b/cgroup.c
>>> @@ -258,9 +258,13 @@ static int move_in_cgroup(CgSetEntry *se)
>>>  		int fd, err;
>>>  		ControllerEntry *ce = se->ctls[i];
>>>  
>>> +		sprintf(aux, "%s/%s", ce->name, ce->path);
>>> +		if (mkdirpat(cg, aux) < 0)
>>
>> Tycho, there's one thing about this mkdir-s that we've discussed with Saied (in Cc).
>> The problem is that in cgroup FS there may be directories corresponding to cgroups
>> in which there are no tasks, but that would appear some time soon.
>>
>> When we dump such a container we don't dump the information about such empty cgroups.
>> And on restore, if we create the directories by criu, we will _not_ create empty
>> cgroups and further 'echo $pid > cgroup/tasks' would fail.
>>
>> Thus we should chose one of two ways:
>>
>> 1. We do not handle cgroup hierarchy at all in criu, asking caller to do this.
>>    What we do it just populate cgroups with tasks. This is how criu works now.
>>
>> 2. We state, that we take full care of cgroup hierarchy on dump and restore. This
>>    is were we should go, but in that case we should also dump and restore those
>>    empty cgroups I've mentioned.
>>
>> And, once we start to go the route #2 we should also take care of the cgroups
>> configuration.
>>
>> The fix you're proposing is how we (probably) should start doing the way #2, so
>> let's decide how can we handle empty cgroups (and configuration) :)
> 
> Understood. For dumping empty cgroups, is it just that the current
> code only looks at the task and finds its cgroups, and we need to add
> an additional step that walks /sys/fs/cgroup (or wherever)? A quick
> glance through the code didn't reveal any "skip empty cgroups" type
> steps.

Yes. Emply cgroups cannot be discovered through /proc/pid/cgroup file,
we should walk the alive cgroup mount. But the problem is -- we cannot
just take the system /sys/fs/cgroup/ directories, since there will be
cgroups from other containers as well. We should find the root subdir
of the container we dump and walk _this_ subtree.

Thanks,
Pavel