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

[Serge Hallyn]

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.

[James Hunt (jamesodhunt)]

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)]

Hah! Thanks, James. Updating that now.

[lp:~serge-hallyn/upstart/upstart-fix-cgm-env-tests]

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)]

Thanks - merged!

review: Approve

=== modified file 'test/test_util_common.c'
--- test/test_util_common.c	2014-07-10 16:45:19 +0000
+++ test/test_util_common.c	2014-07-24 16:18:09 +0000
@@ -1468,21 +1468,31 @@
 		*p = '\0';
 		if (cgmanager_create_sync(NULL, cgroup_manager, line, cg,
 					&e) != 0) {
-			nih_error("%s: failed to create cgroup %s:%s",
-				__func__, line, cg);
+			NihError *nerr;
+			nerr = nih_error_get();
+			nih_error("%s: failed to create cgroup %s:%s: %s",
+				__func__, line, cg, nerr->message);
+			nih_free(nerr);
 			goto out;
 		}
 		if (e == 1)
 			nih_warn("%s: boggle: cgroup %s:%s already existed",
 					__func__, line, cg);
 		if (cgmanager_remove_on_empty_sync(NULL, cgroup_manager, line,
-					cg) != 0)
-			nih_warn("%s: failed to mark %s:%s remove-on-empty",
-				__func__, line, cg);
+					cg) != 0) {
+			NihError *nerr;
+			nerr = nih_error_get();
+			nih_warn("%s: failed to mark %s:%s remove-on-empty: %s",
+				__func__, line, cg, nerr->message);
+			nih_free(nerr);
+		}
 		if (cgmanager_move_pid_sync(NULL, cgroup_manager, line, cg,
 					mypid) != 0) {
-			nih_error("%s: failed to move myself to cgroup %s:%s",
-				__func__, line, cg);
+			NihError *nerr;
+			nerr = nih_error_get();
+			nih_error("%s: failed to move myself to cgroup %s:%s: %s",
+				__func__, line, cg, nerr->message);
+			nih_free(nerr);
 			goto out;
 		}
 	}

=== modified file 'test/tests/test_util_check_env.c'
--- test/tests/test_util_check_env.c	2014-06-05 12:53:33 +0000
+++ test/tests/test_util_check_env.c	2014-07-24 16:18:09 +0000
@@ -169,18 +169,29 @@
 		TEST_FEATURE ("checking for cgmanager");
 		ret = connect_to_cgmanager ();
 		switch (ret) {
-		case -2: TEST_FAILED ("Found no cgroup manager"); break;
-		case -1: TEST_FAILED ("Error connecting to cgmanager"); break;
-		case 0: print_my_cgroup (); break;
-		default: TEST_FAILED ("Unknown error from connect_to_cgmanager: %d", ret);
+		case -2:
+			nih_warn ("Found no cgroup manager");
+			goto out_skip;
+		case -1:
+			nih_warn ("Error connecting to cgmanager");
+			goto out_skip;
+		case 0:
+			print_my_cgroup ();
+			break;
+		default: nih_warn ("Unknown error from connect_to_cgmanager: %d", ret);
+			goto out_skip;
 		}
 
 		TEST_FEATURE ("cgroup sandbox");
-		TEST_EQ (check_cgroup_sandbox (), 0);
-		disconnect_cgmanager ();
+		if (check_cgroup_sandbox() != 0)
+			nih_warn ("Could not create cgroup sandbox");
 	} else {
 		nih_warn ("Skipping CGManager tests, CGManager socket not found");
 	}
+out_skip:
+	disconnect_cgmanager();
+	if (ret)
+		nih_warn ("Skipping CGManager tests, CGManager not properly configured");
 #endif /* ENABLE_CGROUPS */
 
 }