Kamalesh Babulal [Fri, 13 Oct 2023 12:02:24 +0000 (17:32 +0530)]
ftests: add test to exercise cgset recursive set option
Add a test to recursive set settings of the controller(s) in a cgroup
and its descendants using cgset '-R' flag.
-----------------------------------------------------------------
Test Results:
Run Date: Oct 28 15:25:57
Passed: 1 test(s)
Skipped: 0 test(s)
Failed: 0 test(s)
-----------------------------------------------------------------
Timing Results:
Test Time (sec)
------------------------------------------
setup 0.00
089-cgset-recursive_flag.py 0.13
teardown 0.00
------------------------------------------
Total Run Time 0.13
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Wed, 11 Oct 2023 11:29:38 +0000 (16:59 +0530)]
tools/cgset: add support for cgroup.subtree_control
The cgroup.subtree_control settings are special, in comparison to other
controller settings. It can both enable and disable the controllers in
the single argument, depending on the argument the cgroup hierarchy walk
is either pre-order or post-order.
tools/cgset: add -R option to recursively set variables
Add -R option to recursively set variable(s) passed to cgroups under
<cgroup_path>. This will help users to set a controller setting for
all the cgroups under a cgroup hierarchy, instead of passing the
cgroups multiple times on the command line.
Without the patch
------------------
./cgset -r cpu.shares=256 foo
./cgget -r cpu.shares foo
foo/ch1 foo/ch2
foo:
cpu.shares: 256
foo/ch1:
cpu.shares: 1024
foo/ch2:
cpu.shares: 1024
With the patch
--------------
./cgset -R -r cpu.shares=512 foo
./cgget -r cpu.shares foo foo/ch1 foo/ch2
foo:
cpu.shares: 512
foo/ch1:
cpu.shares: 512
foo/ch2:
cpu.shares: 512
[check for ECGEOF recommend by Tom Hromatka] Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Move the cgroup controller value setting logic to a helper function,
cgroup_set_cgroup_values() and also introduce program_name variable, to
be used instead of argv[0], when printing the info()/err() messages.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Tue, 17 Oct 2023 10:26:37 +0000 (15:56 +0530)]
ftests: add test to cgcreate of non default controllers
Add test to create cgroups with non default controllers (hugetlb, misc)
using the cgcreate.
-----------------------------------------------------------------
Test Results:
Run Date: Oct 20 05:21:48
Passed: 1 test(s)
Skipped: 0 test(s)
Failed: 0 test(s)
-----------------------------------------------------------------
Timing Results:
Test Time (sec)
---------------------------------------------------------
setup 0.00
091-cgcreate-non-default-controllers-v2.py 0.04
teardown 0.00
---------------------------------------------------------
Total Run Time 0.04
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Mon, 16 Oct 2023 08:43:56 +0000 (14:13 +0530)]
gunit/012: populate root_cgroup.controllers file
To enable a controller in a cgroup, its parent's subtree_control file is
read to check if the controller is enabled, if not enabled, parent's
cgroup.controllers is checked for the supported controllers, before
enabling it in the cgroup.subtree_control.
Populate controllers in the root_cgroup.controllers file, to help with
the check for supported controllers, without this
cgroupv2_subtree_control_recursive() will fail, when it is checked for
controller existence.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Sun, 15 Oct 2023 08:43:56 +0000 (14:13 +0530)]
api.c: enable controller in the root_cgroup.subtree_file
systemd by default only enables cpu, cpuset, io, memory, and pids
controller the root_cgroup.subtree_control. Trying to enable other
controllers (hugetlb and misc) in a nested cgroups create scenario,
will fail because they are not, enabled the controller in the
root_cgroup.subtree_control file.
$ sudo cgcreate -ghugetlb:foo/bar
Error: Failed to delete foo/bar: No such file or directory
cgcreate: can't create cgroup foo/bar: No such file or directory
Fix this by enabling the controller in the root_cgroup.subtree_control
file unconditionally for all controllers, if not already enabled, while
calling cgroupv2_subtree_control_recursive() to enable the controller
in the given cgroup and its descendants. Checking and enabling every
controller unconditionally should work in the future, in case systemd
disables some other controllers too.
Fixes: https://github.com/libcgroup/libcgroup/issues/405 Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Fix unused variable and sign comparison warnings across the test case:
017-API_fuzz_test.cpp: In member function ‘virtual void APIArgsTest_API_cgroup_add_controller_Test::TestBody()’:
017-API_fuzz_test.cpp:190:6: warning: unused variable ‘ret’ [-Wunused-variable]
int ret;
^~~
In file included from 017-API_fuzz_test.cpp:11:
../../googletest/googletest/include/gtest/gtest.h: In instantiation of ‘testing::AssertionResult testing::internal::CmpHelperEQ(const char*, const char*, const T1&, const T2&) [with T1 = long unsigned int; T2 = int]’:
../../googletest/googletest/include/gtest/gtest.h:1421:23: required from ‘static testing::AssertionResult testing::internal::EqHelper<lhs_is_null_literal>::Compare(const char*, const char*, const T1&, const T2&) [with T1 = long unsigned int; T2 = int; bool lhs_is_null_literal = false]’
017-API_fuzz_test.cpp:480:2: required from here
../../googletest/googletest/include/gtest/gtest.h:1392:11: warning: comparison of integer expressions of different signedness: ‘const long unsigned int’ and ‘const int’ [-Wsign-compare]
if (lhs == rhs) {
~~~~^~~~~~
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
015-cgroupv2_controller_enabled.cpp: In member function ‘void CgroupV2ControllerEnabled::InitMountTable()’:
015-cgroupv2_controller_enabled.cpp:69:7: warning: unused variable ‘ret’ [-Wunused-variable]
int ret, i;
^~~
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Fix unused variable warnings and signed comparison warning across the
test case:
013-cgroup_build_tasks_procs_path.cpp: In member function ‘virtual void BuildTasksProcPathTest_BuildTasksProcPathTest_CgV2_Test::TestBody()’:
013-cgroup_build_tasks_procs_path.cpp:112:27: warning: unused variable ‘ctrlr’ [-Wunused-variable]
struct cgroup_controller ctrlr = {0};
^~~~~
013-cgroup_build_tasks_procs_path.cpp: In member function ‘virtual void BuildTasksProcPathTest_BuildTasksProcPathTest_CgV1WithNs_Test::TestBody()’:
013-cgroup_build_tasks_procs_path.cpp:126:27: warning: unused variable ‘ctrlr’ [-Wunused-variable]
struct cgroup_controller ctrlr = {0};
^~~~~
013-cgroup_build_tasks_procs_path.cpp: In member function ‘virtual void BuildTasksProcPathTest_BuildTasksProcPathTest_CgV2WithNs_Test::TestBody()’:
013-cgroup_build_tasks_procs_path.cpp:140:27: warning: unused variable ‘ctrlr’ [-Wunused-variable]
struct cgroup_controller ctrlr = {0};
^~~~~
In file included from 013-cgroup_build_tasks_procs_path.cpp:9:
../../googletest/googletest/include/gtest/gtest.h: In instantiation of ‘testing::AssertionResult testing::internal::CmpHelperLT(const char*, const char*, const T1&, const T2&) [with T1 = int; T2 = long unsigned int]’:
013-cgroup_build_tasks_procs_path.cpp:53:4: required from here
../../googletest/googletest/include/gtest/gtest.h:1526:28: warning: comparison of integer expressions of different signedness: ‘const int’ and ‘const long unsigned int’ [-Wsign-compare]
../../googletest/googletest/include/gtest/gtest.h:1510:7:
if (val1 op val2) {\
~~~~~~~~~~~~
../../googletest/googletest/include/gtest/gtest.h:1526:28:
GTEST_IMPL_CMP_HELPER_(LT, <);
../../googletest/googletest/include/gtest/gtest.h:1510:12: note: in definition of macro ‘GTEST_IMPL_CMP_HELPER_’
if (val1 op val2) {\
^~
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
011-cgroupv2_subtree_control.cpp: In member function ‘virtual void SubtreeControlTest::SetUp()’:
011-cgroupv2_subtree_control.cpp:24:12: warning: unused variable ‘i’ [-Wunused-variable]
int ret, i;
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
In file included from 001-path.cpp:9:
../../googletest/googletest/include/gtest/gtest.h: In instantiation of ‘testing::AssertionResult testing::internal::CmpHelperLT(const char*, const char*, const T1&, const T2&) [with T1 = int; T2 = long unsigned int]’:
001-path.cpp:53:4: required from here
../../googletest/googletest/include/gtest/gtest.h:1526:28: warning: comparison of integer expressions of different signedness: ‘const int’ and ‘const long unsigned int’ [-Wsign-compare]
../../googletest/googletest/include/gtest/gtest.h:1510:7:
if (val1 op val2) {\
~~~~~~~~~~~~
../../googletest/googletest/include/gtest/gtest.h:1526:28: GTEST_IMPL_CMP_HELPER_(LT, <);
../../googletest/googletest/include/gtest/gtest.h:1510:12: note: in
definition of macro ‘GTEST_IMPL_CMP_HELPER_’
if (val1 op val2) {\
^~
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Wed, 23 Aug 2023 11:05:12 +0000 (11:05 +0000)]
ftests: Add a test to stress create systemd scope via cgcreate
Add a test to stress creation of systemd scope via the cgcreate command
line tool, by passing invalid slice and scope names.
-----------------------------------------------------------------
Test Results:
Run Date: Aug 23 11:03:00
Passed: 1 test(s)
Skipped: 0 test(s)
Failed: 0 test(s)
-----------------------------------------------------------------
Timing Results:
Test Time (sec)
-------------------------------------------------
setup 0.00
997-sudo-cgcreate_systemd_scope.py 0.13
teardown 0.00
-------------------------------------------------
Total Run Time 0.13
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Wed, 30 Aug 2023 06:00:31 +0000 (11:30 +0530)]
add FILE_PATH_CHANGES to ignore list of patch review
Add FILE_PATH_CHANGES to the list of warnings, that is very specific to
Linux Kernel, which is triggered when a file is added, moved, or deleted
to the source tree. Let's ignore it and this patch also removes an extra
new line in the .checkpatch.conf.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Sun, 20 Aug 2023 05:42:47 +0000 (11:12 +0530)]
ftests: update test to check for ctrl name in scope name
Update the test case to check for controller names in the scope name.
-----------------------------------------------------------------
Test Results:
Run Date: Aug 18 07:57:32
Passed: 1 test(s)
Skipped: 0 test(s)
Failed: 0 test(s)
-----------------------------------------------------------------
Timing Results:
Test Time (sec)
-------------------------------------------------
setup 0.00
078-sudo-cgcreate_systemd_scope.py 5.28
teardown 0.00
-------------------------------------------------
Total Run Time 5.28
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Kamalesh Babulal [Tue, 22 Aug 2023 06:34:13 +0000 (12:04 +0530)]
src/systemd: check for ctrl name in scope name
systemd will silently prefix a '_' to the scope name and create, and
delegate it under the slice. If it matches with any of the original
cgroup and pseudo BPF-base systemd controllers. i.e.,
this implicit rename will cause confusion to the users, who would not
see any errors during creation but operate on non-delegated cgroup scope
created by libcgroup internally. Disallow such systemd scope names.
Reported-by: Tom Hromatka <tom.hromatka@oracle.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Add checkpatch action from https://github.com/webispy/checkpatch-action
to workflow CI, this project uses the Linux kernel tree's checkpatch.pl
to review the Pull Request, C code changes for the coding style issues.
We try to follow the coding standards that of the Linux Kernel, with some
exceptions recorded in the .checkpatch.conf file, asking checkpatch.pl
to ignore those. Integrating this action will make every change
complaint with Linux Kernel coding standards. As per their GitHub page
[1] the project uses GPL-2.0 license.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
TJH: Since the action requires info embedded in the Github pull request,
only run it on pull requests. Don't run it on pushes as it will
produce a false positive
Tom Hromatka [Tue, 1 Aug 2023 21:17:35 +0000 (15:17 -0600)]
python: Surround systemd code with m4 conditionals
Surround the systemd python logic with m4 ifdef(`WITH_SYSTEMD'...).
This will conditionally add the systemd code iff systemd is enabled
in configure.ac
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Tom Hromatka [Wed, 2 Aug 2023 18:39:06 +0000 (12:39 -0600)]
ftests: Fix erroneous calls to log_error()
Two places in the functional tests were calling a nonexistent
logging function - log_error(). Replace these erroneous calls
with calls to log_critical().
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
configure.ac: add check for systemd library and header files
systemd support is now enabled by default, unless explicitly disabled
during the ./configure by passing --enable-systemd=no. The make may
fail with:
systemd.c:9:10: fatal error: systemd/sd-bus.h: No such file or directory
9 | #include <systemd/sd-bus.h>
| ^~~~~~~~~~~~~~~~~~
compilation terminated.
if the host system is missing the systemd library and development files,
due to missing checks for their existence in ./configure.ac. This patch
add them.
Fixes: https://github.com/libcgroup/libcgroup/issues/387 Reported-by: Tomasz Kłoczko <kloczek@github.com> Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Tom Hromatka [Thu, 20 Jul 2023 14:24:16 +0000 (19:54 +0530)]
cgsnapshot: Fix compiler truncation warning
Fix the following warning when running make distcheck
../../../../src/tools/cgsnapshot.c: In function ‘is_ctlr_on_list’:
../../../../src/tools/cgsnapshot.c:532:57: warning: ‘%s’ directive output may be truncated writing up to 409599 bytes into a region of size 4096 [-Wformat-truncation=]
532 | snprintf(controllers[i], FILENAME_MAX, "%s", tmp_controllers[i]);
| ^~
Suggested-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
api.c: fix unused variable in cgroup_parse_rules_file()
Fix an unused variable warning, reported by the Coverity tool:
CID 320877 (#1 of 1): Unused value (UNUSED_VALUE) assigned_pointer:
Assigning value NULL to pwd here, but that stored value is overwritten
before it can be used.
Move the grp, pwd variables assignment to beginning of the while loop in
the cgroup_parse_rules_file().
Tested with following cgrules
-----------------------------
$ cat /etc/cgrules.conf
student devices /usergroup/students
@admin * admingroup/
peter cpu test1/
% memory test2/
* * default/
$ sudo CGROUP_LOGLEVEL=DEBUG ./src/daemon/cgrulesengd -d -n
Parsing configuration file /etc/cgrules.conf.
Added rule student (UID: 1001, GID: -1) -> /usergroup/students for
controllers: devices
Added rule @admin (UID: -1, GID: 1004) -> admingroup/ for controllers: *
Added rule peter (UID: 1003, GID: -1) -> test1/ for controllers: cpu
Added rule % (UID: 1003, GID: -1) -> test2/ for controllers: memory
Added rule * (UID: -2, GID: -2) -> default/ for controllers: *
Parsing of configuration file complete.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
tools/cgsnapshot: fix string null terminate in parse_controllers()
Fix string not null terminated warning, reported by the Coverity tool:
CID 258299 (#2 of 2): String not null terminated (STRING_NULL)18.
string_null: Passing unterminated string *controllers to
display_controller_data, which expects a null-terminated string.
use snprintf() instead of strncpy() and manually terminate the string to
keep Coverity happy.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
src/api.c: fix TOCTOU in cg_get_procname_from_proc_cmdline()
Fix a TOCTOU issue, reported by Coverity tool:
CID 258283 (#1 of 1): Time of check time of use (TOCTOU)1.
fs_check_call: Calling function readlink to perform check on path
Coverity gets confused when a char array is re-used for constructing
different paths and complains about TOCTOU for unrelated paths, fix it
by using different char arrays for different path, as a side effect it
also improves the readability of the code.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
CID 258268 (#1 of 1): Time of check time of use (TOCTOU)28.
fs_check_call: Calling function access to perform check on path.
Coverity gets confused when a char array is re-used for constructing
different paths and complains about TOCTOU for unrelated paths, fix it
by using different char arrays for different path, as a side effect it
also improves the readability of the code.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
tools/cgxget: fix TOCTOU in fill_empty_controller()
Fix a TOCTOU issue, reported by Coverity tool:
CID 258277 (#1 of 1): Time of check time of use (TOCTOU)13.
fs_check_call: Calling function access to perform check on path
Coverity gets confused when a char array is re-used for constructing
different paths and complains about TOCTOU for unrelated paths, fix it
by using different char arrays for different path, as a side effect it
also improves the readability of the code.
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com> Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>