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

[Pavel Emelyanov]

On 06/24/2014 06:02 PM, Serge Hallyn wrote:
> Quoting Pavel Emelyanov (xemul at parallels.com):
>> 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) :)
> 
> I personally think #2 is the way to go.  Tools that use criu may be doing some
> initial setup of cgroups, but I'd wager that not a one of them has a concept of
> tracking changes to cgroups that happen after init.  That's criu's job.  So
> I don't think it's going outside of criu's sphere of responsibility to have it
> recreate all cgroups.

I totally agree with that. I just wanted to note, that restoring only the
part of that tree would be less correct than not restoring the tree at all.

But yes, my plan was to complete full cgroups dump before tagging 1.3, so
we have time :)

> Tycho's patch didn't quite do that, he's just trying to get to a point where we
> can checkpoint and restore a simple container.  I think what he's doing is
> valuable as we can get a list of the issues which will need to be addressed.

Absolutely.

> I propose that Tycho keep a 'for-lxc' criu git branch, continue to send
> patches to this list for feedback (perhaps with a '[FYI]' tag in the Subject?)
> and then when we get to a point where a simple container is restartable, we
> can go back over his branch, break it into topics, and discuss the proper
> way to address each one?

Sure. And if Tycho has time to look at full cgroup tree dump/restore, then
I will wait for his patch set.

Thanks,
Pavel