c/r: create cgroups to restore a container #253
Conversation
|
@avagin Thanks. This does resolve the pause after restore issue we were discussing on the mailing list. One thing I just noticed is that "docker top" doesn't quite seem to work in the same situation. Prior to this patch it failed the same way, complaining about the missing cgroup. With this patch, it just reports an empty process list. |
|
@boucher I have fixed a bug, I filled CgRoot incorrectly. Could you try again? |
|
I'll try now. Proto.string is the only change, right? |
|
I'm now seeing: |
|
I tried again and got a different error this time: |
|
(I'm running as root and have 20gb available on this vm) This is failing for me both in the regular checkpoint/restore same container case, and the restore into a new container case. |
avagin
force-pushed
the
cr-cgroups
branch
2 times, most recently
from
26a1ef6 to
1a137c6
Compare
|
@boucher, could you try out this version? I want to think that it's a final one:) |
|
This is working again, but the issue with docker top remains. And, looking a bit closer, I think there's still something not quite right. If I do docker run and then docker pause, and then look in /sys/fs/cgroup/freezer/docker//cgroup/procs, I see the pid of the process running in the container. However, if I checkpoint and then restore and then pause, that file is empty (whether I checkpoint/restore the same container, or restore into a new container). |
|
Damn! I fixed one more bug. |
| return cgroups, nil | ||
| } | ||
|
|
||
| func getControlerPath(subsystem string, cgroups map[string]string) (string, error) { |
There was a problem hiding this comment.
You might want to rename this getControllerPath with two Ls.
Currently we parse /proc/self/cgroup for each controller. It's ineffective. Signed-off-by: Andrey Vagin <avagin@openvz.org>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
Here are two reasons: * If we use systemd, we need to ask it to create cgroups * If a container is restored with another ID, we need to change paths to cgroups. Signed-off-by: Andrey Vagin <avagin@openvz.org>
|
This looks good. Everything seems to be working. |
|
LGTM |
|
LGTM |
c/r: create cgroups to restore a container
Clarify backwards compatibility for major version 0
This reverts commit 0f25f18, opencontainers#253. Now that we're on to 1.0, we don't need to talk about 0.x. And the lack of 0.x backwards compatability is covered by SemVer 2.0 section 4 [1]: Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable. so removing the echo from our spec doesn't actually change anything. The conflict is due to 4e63ee0 (config: qualify the name of the version field, 2016-01-13, opencontainers#309), and only impacted the context and line-wrapping around the sentence I'm removing. Conflicts: config.md [1]: http://semver.org/spec/v2.0.0.html Signed-off-by: W. Trevor King <wking@tremily.us>
Here are two reasons:
change paths to cgroups.
Cc: @boucher
Signed-off-by: Andrey Vagin avagin@openvz.org