bmahler commented on code in PR #529:
URL: https://github.com/apache/mesos/pull/529#discussion_r1534124046
##########
src/tests/containerizer/cgroups2_tests.cpp:
##########
@@ -115,6 +115,14 @@ TEST_F(Cgroups2Test, ROOT_CGROUPS2_AssignProcessToCgroup)
AWAIT_EXPECT_WTERMSIG_EQ(SIGKILL, process::reap(pid));
}
+
+TEST_F(Cgroups2Test, CGROUPS2_Path)
+{
+ EXPECT_EQ("/sys/fs/cgroup/", cgroups2::path(cgroups2::ROOT_CGROUP));
Review Comment:
I guess this is indicating that you have two ways of calling this function?
1. Pass a "cgroup" relative to the hierarchy, you get back the absolute path.
2. Pass a "cgroup" using an absolute path, the absolute path must include
the hierarchy already in this case, e.g. if you pass "/foo", you'll only get
back "/foo", not "/sys/fs/cgroup/foo".
##########
src/linux/cgroups2.hpp:
##########
@@ -75,6 +75,10 @@ Try<Nothing> move_process(const std::string& cgroup, pid_t
pid);
// /sys/fs/cgroup. E.g. For /sys/fs/cgroup/test, this will return "test".
Try<std::string> cgroup(pid_t pid);
+
+// Get the absolute of a cgroup. Assumes to cgroup exists.
Review Comment:
s/to/the/
what does this assumption entail though?
##########
src/linux/cgroups2.cpp:
##########
@@ -309,10 +307,10 @@ Try<Nothing> destroy(const string& cgroup)
Try<Nothing> move_process(const string& cgroup, pid_t pid)
{
- const string absolutePath = path::join(MOUNT_POINT, cgroup);
+ const string& path = cgroups2::path(cgroup);
Review Comment:
we can remove the ref here to be consistent with the other calls
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
