test: fix some failures in test-cgroup

[taskset]

Fix the following issues in test-cgroup:
1, commit 65be7e0 ("pid1: do not reset subtree_control on
already-existing units with delegation") changed the return value
of cg_create () as follows:
"Returns 0 if the group already existed, 1 on success, negative
otherwise."
So we need to modify the test cases related to cg_create ().

2, commit efdb023 ("core: unified cgroup hierarchy support")
changed cg_delete () to cg_rmdir (), so the test cases also need
to be adjusted a bit.

3. There is no cleanup of "test-a". If we execute test-cgroup
   multiple times, we will encounter an error.

Signed-off-by: Wen Yang wenyang@linux.alibaba.com

[evverx]

I think the test was broken mostly because it was never run automatically. I'm wondering if it would make sense to make it "unsafe" (by replacing "manual" with "unsafe" in src/test/meson.build)? (Though as far as I can tell, the unsafe tests aren't run on Ubuntu CI)

Could you temporarily remove "manual" in src/test/meson.build to unleash the CI on the test to make sure it can pass?

[evverx]

FWIW even with this patch applied the test seems to be failing on my machine (where the hybrid hierarchy is being used at the moment as far as I can see) with:

Assertion 'path_equal(path, "/sys/fs/cgroup/systemd/test-b/test-d")' failed at src/test/test-cgroup.c:39, function main(). Aborting."

Breakpoint 1, main (argc=1, argv=0x7fffffffe658) at ../src/test/test-cgroup.c:39
39	        assert_se(path_equal(path, "/sys/fs/cgroup/systemd/test-b/test-d"));
(gdb) p path
$1 = 0x604000000250 "/sys/fs/cgroup/unified/test-b/test-d"

There also seems to be a memory leak there:

=================================================================
==1530==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 7 byte(s) in 1 object(s) allocated from:
    #0 0x7fde9aa8fc58 in __interceptor_malloc (/lib64/libasan.so.5+0x10dc58)
    #1 0x7fde9a9f8383  (/lib64/libasan.so.5+0x76383)
    #2 0x7fde999b3f88 in cg_split_spec ../src/basic/cgroup-util.c:924
    #3 0x40223a in main ../src/test/test-cgroup.c:71
    #4 0x7fde983b7f42 in __libc_start_main (/lib64/libc.so.6+0x23f42)

SUMMARY: AddressSanitizer: 7 byte(s) leaked in 1 allocation(s).

so I'm inclined to say that either the test should be fully resurrected and run automatically or it simply should be dropped so that contributors wouldn't spend their time trying to figure out why it's completely broken.

[cdown]

I need to double check the history later to check why this is only on manual, but yeah, we should fix it for everybody and set it to automatic.

That said, this should be well exercised by integ tests, so the risk is low in the short term.

[keszybz]

I'm wondering if it would make sense to make it "unsafe"

IIRC, it was marked as "manual" because it mucks with the cgroup tree. I agree that we should make it "unsafe", or even just run it everywhere. It shouldn't break anything.

[keszybz]

I pushed some more commits to make the test pass and remove the "manual" mark.

[keszybz]

I think this is OK to merge, but I would appreciate a review from somebody else.

[taskset]

Great!
Thanks so much for all the work.

[cdown]

LGTM.