Merge lp:~jamesodhunt/upstart/check-for-overlayfs into lp:upstart

[James Hunt]

* test/tests/test_util_check_env.c: New test to look for
  overlayfs filesystems which could cause tests to fail.
* test/Makefile.am: Added test_util_check_env meta-test.
* test/test_util_common.c: Formatting.

This branch introduces a new meta-test that currently checks for overlayfs. It will produce a warning that tests will probably fail (due to bug LP:#882147) but does not actually abort the test run since:

- the invoker may know better whether a particular FS could cause problems for the tests.
- overlayfs may one day get fixed so hard-failing would be incorrect behaviour.

Note that the new test will be run before any other test. Further, note that since the overlayfs check is a separate test, if an individual test is run in an overlayfs environment, the invoker may be unaware of the strange resultant test failures. However, having a separate test does mean that we avoid linking every test against libnih-dbus et al.

[Steve Langasek (vorlon)]

does not merge cleanly, please sync with trunk and resubmit.

review: Needs Fixing

[Steve Langasek (vorlon)]

> * Returns: TRUE if any mount point is using overlayfs, else FALSE.

Why warn about *any* overlayfs? Shouldn't we only care about whether the path to our temp directory is affected by overlayfs?

[James Hunt (jamesodhunt)]

Ok branch updated.

[Steve Langasek (vorlon)]

Since we're checking the type of a single mountpoint, we should be able to use statfs instead of walking around in /proc/self/mounts. E.g.:

        struct statfs statbuf;

        /* Create a file in the temporary work area */
        TEST_FILENAME (path);
        fclose (fopen (path, "w"));

        assert0 (statfs (path, &statbuf));

        if (statbuf.f_type == OVERLAYFS_SUPER_MAGIC) {
                nih_warn ("Mountpoint for '%s' (needed by the Upstart tests) is an overlayfs "
                                        "filesystem, which does not support inotify.",
                                        path);
                found = TRUE;
        }

        assert0 (unlink (path));

        return found;

It seems that the linux-kernel-headers package does not currently export a define for OVERLAYFS_SUPER_MAGIC, which is an oversight. We can copy this define from the kernel source for now, since it shouldn't change, and ask the kernel team to have it properly exported.

#define OVERLAYFS_SUPER_MAGIC 0x794c764f

What do you think of this approach?

[James Hunt (jamesodhunt)]

Hi Steve,

Yes, that would clearly be more efficient, but I'm not wholly convinced we need to do this since the current approach is simple, it works and we only call this test-only function once anyway. Additionally, the current approach avoids the need to retain the '#ifndef OVERLAYFS_SUPER_MAGIC' forever to handle non-Ubuntu systems.

I've raised bug 1247769 on the kernel headers for the missing define though.

=== modified file 'ChangeLog'
--- ChangeLog	2013-10-04 21:34:25 +0000
+++ ChangeLog	2013-10-17 08:19:01 +0000
@@ -1,4 +1,16 @@
-2013-01-04  Steve Langasek  <[e-mail]
[phone]  James Hunt  <[e-mail]>
+
+	* test/tests/test_util_check_env.c:
+	  - check_for_overlayfs(): Only consider temporary work area.
+
[phone]  James Hunt  <[e-mail]>
+
+	* test/tests/test_util_check_env.c: New test to look for
+	  overlayfs filesystems which could cause tests to fail.
+	* test/Makefile.am: Added test_util_check_env meta-test.
+	* test/test_util_common.c: Formatting.
+
[phone]  Steve Langasek  <[e-mail]
 
 	* extra/upstart-local-bridge.c: use SOCKET_PATH in our event
 	  environment, instead of clobbering PATH.  (LP: #1235480)
 
=== modified file 'test/Makefile.am'
--- test/Makefile.am	2013-06-25 09:19:05 +0000
+++ test/Makefile.am	2013-10-17 08:19:01 +0000
@@ -18,4 +18,17 @@
 	-I$(top_srcdir)/intl
 
 check_LIBRARIES = libtest_util_common.a
-libtest_util_common_a_SOURCES = test_util_common.c test_util_common.h
+libtest_util_common_a_SOURCES = \
+	test_util_common.c \
+	test_util_common.h
+
+TESTS = test_util_check_env
+check_PROGRAMS = $(TESTS)
+
+.PHONY: tests
+tests: $(check_PROGRAMS)
+
+test_util_check_env_SOURCES = tests/test_util_check_env.c
+test_util_check_env_LDADD = \
+	libtest_util_common.a \
+	$(NIH_LIBS)
 
=== modified file 'test/test_util_common.c'
--- test/test_util_common.c	2013-10-02 08:59:20 +0000
+++ test/test_util_common.c	2013-10-17 08:19:01 +0000
@@ -162,7 +162,7 @@
 	 * within a reasonable period of time.
 	 */
 	for (i = 0; i < loops; i++) {
-        sleep (1);
+		sleep (1);
 
 		RUN_COMMAND (NULL, cmd, &output, &lines);
 
 
=== added directory 'test/tests'
=== added file 'test/tests/test_util_check_env.c'
--- test/tests/test_util_check_env.c	1970-01-01 00:00:00 +0000
+++ test/tests/test_util_check_env.c	2013-10-17 08:19:01 +0000
@@ -0,0 +1,128 @@
+/* upstart
+ *
+ * test_util_check_env.c - meta test to ensure environment sane for
+ * running tests.
+ *
+ * Copyright © 2013 Canonical Ltd.
+ * Author: James Hunt <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <limits.h>
+#include <unistd.h>
+#include <mntent.h>
+
+#include <nih/string.h>
+#include <nih/test.h>
+#include <nih/logging.h>
+
+#include "test_util_common.h"
+
+/**
+ * check_for_overlayfs:
+ *
+ * Determine if the mount point used by the tests for creating temporary
+ * files is using overlayfs.
+ *
+ * Returns: TRUE if temporary work area is on overlayfs, else FALSE.
+ **/
+int
+check_for_overlayfs (void)
+{
+	struct stat    statbuf;
+	char           path[PATH_MAX];
+	const char    *fs = "overlayfs";
+	unsigned int   maj;
+	unsigned int   min;
+	const char    *mounts = "/proc/self/mounts";
+	FILE          *mtab;
+	struct mntent *mnt;
+	int            found = FALSE;
+
+	/* Create a file in the temporary work area */
+	TEST_FILENAME (path);
+	fclose (fopen (path, "w"));
+
+	/* Check it exits */
+	assert0 (stat (path, &statbuf));
+
+	/* Extract device details */
+	maj = major (statbuf.st_dev);
+	min = minor (statbuf.st_dev);
+
+	mtab = fopen (mounts, "r");
+	TEST_NE_P (mtab, NULL);
+
+	/* Look through mount table */
+	while ((mnt = getmntent (mtab))) {
+		unsigned int mount_maj;
+		unsigned int mount_min;
+
+		assert0 (stat (mnt->mnt_dir, &statbuf));
+		mount_maj = major (statbuf.st_dev);
+		mount_min = minor (statbuf.st_dev);
+
+		if (! strcmp (mnt->mnt_type, fs) && mount_maj == maj && mount_min == min) {
+			found = TRUE;
+			nih_warn ("Mountpoint '%s' (needed by the Upstart tests) is an '%s' "
+					"filesystem which does not support inotify.",
+					mnt->mnt_dir,
+					fs);
+			goto out;
+		}
+	}
+
+out:
+	fclose (mtab);
+	assert0 (unlink (path));
+
+	return found;
+}
+
+/**
+ * test_checks:
+ *
+ * Perform any checks necessary before real tests are run.
+ **/
+void
+test_checks (void)
+{
+	TEST_GROUP ("test environment");
+
+	/*
+	 * Warn (*) if overlayfs detected.
+	 *
+	 * (*) - Don't fail in the hope that one day someone might fix
+	 * overlayfs.
+	 */
+	TEST_FEATURE ("checking for overlayfs");
+	if (check_for_overlayfs ()) {
+		nih_warn ("Found overlayfs mounts");
+		nih_warn ("This environment will probably cause tests to fail mysteriously!!");
+		nih_warn ("See bug LP:#882147 for further details.");
+	}
+}
+
+int
+main (int   argc,
+      char *argv[])
+{
+	test_checks ();
+
+	return 0;
+}