bmahler commented on code in PR #523:
URL: https://github.com/apache/mesos/pull/523#discussion_r1531024162
##########
src/linux/cgroups2.cpp:
##########
@@ -274,6 +274,46 @@ Try<Nothing> destroy(const string& cgroup)
return Nothing();
}
+
+Try<Nothing> move_process(const string& cgroup, pid_t pid)
+{
+ const string absolutePath = path::join(MOUNT_POINT, cgroup);
+
+ if (!os::exists(absolutePath)) {
+ return Error("There does not exist a cgroup at '" + absolutePath + "'");
+ }
+
+ return cgroups2::write(cgroup, control::PROCESSES, std::to_string(pid));
+}
+
+
+Try<string> cgroup(pid_t pid)
+{
+ // A process's cgroup membership is listed in '/proc/{pid}/cgroup'.
+ //
+ // The format, e.g for the cgroup `/sys/fs/cgroup/foo/bar`, is:
+ // """"
+ // 0::/foo/bar
+ // or
+ // 0::/foo/bar (deleted)
+ // """
+ const string& cgroupFile = path::join("/proc", std::to_string(pid),
"cgroup");
+ if (!os::exists(cgroupFile)) {
+ return Error("File '" + cgroupFile + "' does not exist");
+ }
+
+ Try<string> content = os::read(cgroupFile);
+ if (content.isError()) {
+ return Error("Failed to read '" + cgroupFile + "'");
+ }
+
+ string parsed = strings::remove(
+ strings::trim(*content), "0::/", strings::Mode::PREFIX);
+ parsed = strings::remove(parsed, " (deleted)", strings::Mode::SUFFIX);
Review Comment:
hm.. seems like we should return an error if the pid belongs to a v1 cgroup
and therefore we find the wrong format in the file? Rather than return a string
like:
"5:cpuacct,cpu,cpuset:/daemons"
which is the current behavior in this case
##########
src/linux/cgroups2.cpp:
##########
@@ -274,6 +274,46 @@ Try<Nothing> destroy(const string& cgroup)
return Nothing();
}
+
+Try<Nothing> move_process(const string& cgroup, pid_t pid)
+{
+ const string absolutePath = path::join(MOUNT_POINT, cgroup);
+
+ if (!os::exists(absolutePath)) {
+ return Error("There does not exist a cgroup at '" + absolutePath + "'");
+ }
+
+ return cgroups2::write(cgroup, control::PROCESSES, std::to_string(pid));
+}
+
+
+Try<string> cgroup(pid_t pid)
+{
+ // A process's cgroup membership is listed in '/proc/{pid}/cgroup'.
+ //
+ // The format, e.g for the cgroup `/sys/fs/cgroup/foo/bar`, is:
+ // """"
+ // 0::/foo/bar
+ // or
+ // 0::/foo/bar (deleted)
+ // """
+ const string& cgroupFile = path::join("/proc", std::to_string(pid),
"cgroup");
+ if (!os::exists(cgroupFile)) {
+ return Error("File '" + cgroupFile + "' does not exist");
+ }
+
+ Try<string> content = os::read(cgroupFile);
+ if (content.isError()) {
+ return Error("Failed to read '" + cgroupFile + "'");
Review Comment:
include the read error in the message here?
##########
src/tests/containerizer/cgroups2_tests.cpp:
##########
@@ -59,6 +63,35 @@ TEST_F(Cgroups2Test, ROOT_CGROUPS2_AvailableSubsystems)
}
}
+
+TEST_F(Cgroups2Test, ROOT_CGROUPS2_AssignProcessToCgroup)
+{
+ string cgroup = "test";
+ ASSERT_SOME(cgroups2::create(cgroup));
Review Comment:
We need to add cleanup logic into the Cgroups2Test fixture, otherwise if
this test leaves early nothing cleans up the test cgroup, can you follow up on
this in a separate PR?
##########
src/linux/cgroups2.cpp:
##########
@@ -274,6 +274,46 @@ Try<Nothing> destroy(const string& cgroup)
return Nothing();
}
+
+Try<Nothing> move_process(const string& cgroup, pid_t pid)
+{
+ const string absolutePath = path::join(MOUNT_POINT, cgroup);
+
+ if (!os::exists(absolutePath)) {
+ return Error("There does not exist a cgroup at '" + absolutePath + "'");
+ }
+
+ return cgroups2::write(cgroup, control::PROCESSES, std::to_string(pid));
Review Comment:
we use stringify() for stringification, ditto below
--
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]
