Merge lp:~serge-hallyn/upstart/upstart-fix-cgm-env-tests into lp:upstart

Proposed by Serge Hallyn on 2014-07-23
Status: Merged
Merged at revision: 1659
Proposed branch: lp:~serge-hallyn/upstart/upstart-fix-cgm-env-tests
Merge into: lp:upstart
Diff against target: 82 lines (+34/-13)
2 files modified
test/test_util_common.c (+17/-7)
test/tests/test_util_check_env.c (+17/-6)
To merge this branch: bzr merge lp:~serge-hallyn/upstart/upstart-fix-cgm-env-tests
Reviewer Review Type Date Requested Status
James Hunt 2014-07-23 Approve on 2014-07-24
Review via email: mp+227976@code.launchpad.net

Description of the change

This allows 'make check' by an unprivileged user to succeed if they do not own their current cgroups.

The commit message forgets to point out that this also fixes segv bugs by cleaning up the NihErrors after failed cgmanager client calls.

To post a comment you must log in.
James Hunt (jamesodhunt) wrote :

Hi Serge,

Thanks for doing this. There is a small problem in that even on a correctly configured system, test_checks() still displays "Skipping CGManager tests, CGManager not properly configured".

A minimal fix may be something like the following:

=== modified file 'test/tests/test_util_check_env.c'
--- test/tests/test_util_check_env.c 2014-07-23 18:34:26 +0000
+++ test/tests/test_util_check_env.c 2014-07-24 07:39:49 +0000
@@ -190,7 +190,8 @@
        }
 out_skip:
        disconnect_cgmanager();
- nih_warn ("Skipping CGManager tests, CGManager not properly configured");
+ if (ret)
+ nih_warn ("Skipping CGManager tests, CGManager not properly configured");
 #endif /* ENABLE_CGROUPS */

 }

review: Needs Fixing
Serge Hallyn (serge-hallyn) wrote :

Hah! Thanks, James. Updating that now.

1660. By Serge Hallyn on 2014-07-24

Don't warn about skipped tests if the tests were run and passed.

Thanks James.

James Hunt (jamesodhunt) wrote :

Thanks - merged!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'test/test_util_common.c'
2--- test/test_util_common.c 2014-07-10 16:45:19 +0000
3+++ test/test_util_common.c 2014-07-24 16:18:09 +0000
4@@ -1468,21 +1468,31 @@
5 *p = '\0';
6 if (cgmanager_create_sync(NULL, cgroup_manager, line, cg,
7 &e) != 0) {
8- nih_error("%s: failed to create cgroup %s:%s",
9- __func__, line, cg);
10+ NihError *nerr;
11+ nerr = nih_error_get();
12+ nih_error("%s: failed to create cgroup %s:%s: %s",
13+ __func__, line, cg, nerr->message);
14+ nih_free(nerr);
15 goto out;
16 }
17 if (e == 1)
18 nih_warn("%s: boggle: cgroup %s:%s already existed",
19 __func__, line, cg);
20 if (cgmanager_remove_on_empty_sync(NULL, cgroup_manager, line,
21- cg) != 0)
22- nih_warn("%s: failed to mark %s:%s remove-on-empty",
23- __func__, line, cg);
24+ cg) != 0) {
25+ NihError *nerr;
26+ nerr = nih_error_get();
27+ nih_warn("%s: failed to mark %s:%s remove-on-empty: %s",
28+ __func__, line, cg, nerr->message);
29+ nih_free(nerr);
30+ }
31 if (cgmanager_move_pid_sync(NULL, cgroup_manager, line, cg,
32 mypid) != 0) {
33- nih_error("%s: failed to move myself to cgroup %s:%s",
34- __func__, line, cg);
35+ NihError *nerr;
36+ nerr = nih_error_get();
37+ nih_error("%s: failed to move myself to cgroup %s:%s: %s",
38+ __func__, line, cg, nerr->message);
39+ nih_free(nerr);
40 goto out;
41 }
42 }
43
44=== modified file 'test/tests/test_util_check_env.c'
45--- test/tests/test_util_check_env.c 2014-06-05 12:53:33 +0000
46+++ test/tests/test_util_check_env.c 2014-07-24 16:18:09 +0000
47@@ -169,18 +169,29 @@
48 TEST_FEATURE ("checking for cgmanager");
49 ret = connect_to_cgmanager ();
50 switch (ret) {
51- case -2: TEST_FAILED ("Found no cgroup manager"); break;
52- case -1: TEST_FAILED ("Error connecting to cgmanager"); break;
53- case 0: print_my_cgroup (); break;
54- default: TEST_FAILED ("Unknown error from connect_to_cgmanager: %d", ret);
55+ case -2:
56+ nih_warn ("Found no cgroup manager");
57+ goto out_skip;
58+ case -1:
59+ nih_warn ("Error connecting to cgmanager");
60+ goto out_skip;
61+ case 0:
62+ print_my_cgroup ();
63+ break;
64+ default: nih_warn ("Unknown error from connect_to_cgmanager: %d", ret);
65+ goto out_skip;
66 }
67
68 TEST_FEATURE ("cgroup sandbox");
69- TEST_EQ (check_cgroup_sandbox (), 0);
70- disconnect_cgmanager ();
71+ if (check_cgroup_sandbox() != 0)
72+ nih_warn ("Could not create cgroup sandbox");
73 } else {
74 nih_warn ("Skipping CGManager tests, CGManager socket not found");
75 }
76+out_skip:
77+ disconnect_cgmanager();
78+ if (ret)
79+ nih_warn ("Skipping CGManager tests, CGManager not properly configured");
80 #endif /* ENABLE_CGROUPS */
81
82 }

Subscribers

People subscribed via source and target branches