Fix: cgroup2: Check cgroup existence on Load#384
Fix: cgroup2: Check cgroup existence on Load#384Berbe wants to merge 2 commits intocontainerd:mainfrom
Conversation
Berbe
force-pushed
the
fix/cgroup2/load
branch
3 times, most recently
from
59fb1f1 to
f122353
Compare
Signed-off-by: Berbe <4251220+Berbe@users.noreply.github.com>
Signed-off-by: Berbe <4251220+Berbe@users.noreply.github.com>
Berbe
force-pushed
the
fix/cgroup2/load
branch
from
f122353 to
dbf9ebd
Compare
| return nil, err | ||
| } | ||
| path := filepath.Join(c.mountpoint, group) | ||
| _, err := os.Stat(path) |
There was a problem hiding this comment.
I am not sure we should check this. It's just loading into object.
All the operations doesn't hold fd for this path. It could be valid case that Load it first and create it later.
ping @AkihiroSuda @mikebrow @dmcgowan to review this.
There was a problem hiding this comment.
I would, maybe naively, expect a function named Load to verify what we were trying to load is valid, and that's how I'd envision the API would be used (try to load an existing cgroup, and if it's not there I'd want to get some classic stdlib error to handle the not existing case). We could discuss whether Stat is a good enough metric for proving it's valid as well (e.g. maybe we want to actually see whether the path is a path in a cg2 mountpoint), but that's another topic. The API for creating and getting a handle to a cg2 in this library is NewManager(), and the only other way to create a new cg through the API is manager.NewChild(), but the expectation is you'd be creating a child cg from the one you already have a handle to.
3 participants
Loaddoes not check for cgroup existence: this proposal fixes this