[CRIU] [PATCH] Skip restoring certain cgroup root directories when using full mode

[Pavel Emelyanov]

On 08/22/2015 07:30 AM, Hui Kang wrote:
> On Sat, Aug 22, 2015 at 12:03 AM, Hui Kang <hkang.sunysb at gmail.com> wrote:
>> On Fri, Aug 21, 2015 at 2:12 PM, Pavel Emelyanov <xemul at parallels.com> wrote:
>>> On 08/19/2015 12:40 AM, Hui Kang wrote:
>>>>
>>>>
>>>> On Tue, Aug 18, 2015 at 11:13 AM, Pavel Emelyanov <xemul at parallels.com <mailto:xemul at parallels.com>> wrote:
>>>>
>>>>     On 08/18/2015 12:55 AM, Hui Kang wrote:
>>>>     > On Mon, Aug 17, 2015 at 10:48 AM, Pavel Emelyanov <xemul at parallels.com <mailto:xemul at parallels.com>> wrote:
>>>>     >> On 08/17/2015 08:07 AM, Hui Kang wrote:
>>>>     >>> On Sun, Aug 16, 2015 at 9:20 PM, Pavel Emelyanov <xemul at parallels.com <mailto:xemul at parallels.com>> wrote:
>>>>     >>>> On 08/13/2015 07:41 PM, Hui Kang wrote:
>>>>     >>>>> Skip these top cgroup directories: cpu, cpuset, cpuacct, memory
>>>>     >>>>>
>>>>     >>>>> Signed-off-by: Hui Kang <hkang.sunysb at gmail.com <mailto:hkang.sunysb at gmail.com>>
>>>>     >>>>> ---
>>>>     >>>>>  cgroup.c | 4 ++++
>>>>     >>>>>  1 file changed, 4 insertions(+)
>>>>     >>>>>
>>>>     >>>>> diff --git a/cgroup.c b/cgroup.c
>>>>     >>>>> index a4e0146..94749b1 100644
>>>>     >>>>> --- a/cgroup.c
>>>>     >>>>> +++ b/cgroup.c
>>>>     >>>>> @@ -992,6 +992,10 @@ static int prepare_cgroup_dir_properties(char *path, int off, CgroupDirEntry **e
>>>>     >>>>>               CgroupDirEntry *e = ents[i];
>>>>     >>>>>               size_t off2 = off;
>>>>     >>>>>
>>>>     >>>>> +             if (strcmp(e->dir_name, "") == 0 &&
>>>>     >>>>> +                 ((strstr(path, "cpu")) || (strstr(path, "memory"))))
>>>>     >>>>> +                     continue;
>>>>     >>>>> +
>>>>     >>>>
>>>>     >>>> With this "continue" won't we skip all the properties restore? The e-s we
>>>>     >>>> have here represent all the tree, starting from root and descending. If you
>>>>     >>>> skip the root one, then the whole subtree is skipped too.
>>>>     >>>
>>>>     >>> No. Although the e-s represent the tree, the for loops will iterate
>>>>     >>> all the directories.
>>>>     >>
>>>>     >> The loop iterates top-level directories, then dives into children. Can you show
>>>>     >> the contents of your cgroup.img file?
>>>>     >
>>>>     > {
>>>>     >     "magic": "CGROUP",
>>>>     >     "entries": [
>>>>
>>>>     Ah, I see. The thing is that in your case you don't have nested directories. In
>>>>     particular -- cpuset is only the "foo/foo1" dir, so your check doesn't match
>>>>     one and it's restored. The cpu/cpuacct are both "/" only, so you check matches
>>>>     them, skips, but we notice nothing as there are no sub-directories.
>>>>
>>>>
>>>> HI, Pavel,
>>>> can you give me an example which has nested directories? I added the process to foo/foo1/tasks and then dump the process. So no value in foo is dumped.
>>>>
>>>> For a nested cgroup directory, do you mean I should also add a parent tasks to foo and then dump both parent and child?
>>>
>>> Create at least two directories (foo/A and foo/A/B) and two tasks living in each. This will give you
>>> two entries in image file.
>>>
>>> The bug I'm thinking of would happen if you have groups / and /A and two tasks living in each (again).
>>> With your patch the /A's props will not be restored.
>>
>> I was trying to make the bug happen as you described. I mkdir such
>> cgroup strucutre
>>
>> /sys/fs/cgroup/cpuset/foo
>> /sys/fs/cgroup/cpuset/foo/A
>> /sys/fs/cgroup/cpuset/foo/A/B
>>
>> Step 1: Put task 1001 to /sys/fs/cgroup/cpuset/foo/A and 1002 to
>> /sys/fs/cgroup/cpuset/foo/A/B
>>
>> Step 2: Then dump 1001 and 1002 separately.
> 
> The cgroup image of 1001 which lives in foo/A/tasks is
> 
> {
...
> }
> 
> The patched criu can restore the process without any bug. Any thought?

Sure, you patch criu to skip root cgroup, but in your test you don't have
any, so your patch simply doesn't affect the restore behavior.

-- Pavel