[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH v2 2/3] virtestmock: Print invalid file accesses into a file



All the accesses to files outside our build or source directories
are now identified and appended into a file for later processing.
The location of the file that contains all the records can be
controlled via VIR_TEST_FILE_ACCESS env variable and defaults to
abs_builddir "/test_file_access.txt".

The script that will process the access file is to be added in
next commit.

Signed-off-by: Michal Privoznik <mprivozn redhat com>
---
 .gitignore           |   1 +
 HACKING              |  11 ++++++
 cfg.mk               |   6 +--
 docs/hacking.html.in |  15 ++++++++
 tests/testutils.c    |  27 +++++++++----
 tests/virtestmock.c  | 105 +++++++++++++++++++++++++++++++++++++++++++++++++--
 6 files changed, 151 insertions(+), 14 deletions(-)

diff --git a/.gitignore b/.gitignore
index fc21bad..ee89787 100644
--- a/.gitignore
+++ b/.gitignore
@@ -169,6 +169,7 @@
 /tests/objectlocking.cm[ix]
 /tests/reconnect
 /tests/ssh
+/tests/test_file_access.txt
 /tests/test_conf
 /tools/libvirt-guests.sh
 /tools/virt-login-shell
diff --git a/HACKING b/HACKING
index e308568..63ad327 100644
--- a/HACKING
+++ b/HACKING
@@ -152,6 +152,17 @@ There is also a "./run" script at the top level, to make it easier to run
 programs that have not yet been installed, as well as to wrap invocations of
 various tests under gdb or Valgrind.
 
+When running our test suite it may happen that nondeterministic result is
+produced because of the test suite relying on a particular file in the system
+being accessible or having some specific value. This is a problem because if
+ran under different environment the test result may be different and a test
+can fail. To catch this kind of errors, the test suite has a module for that.
+The module prints any path touched that fulfils constraints described above
+into a file. Then "VIR_TEST_FILE_ACCESS" environment variable can alter
+location where the file is stored.
+
+  VIR_TEST_FILE_ACCESS="/tmp/file_access.txt" ./qemuxml2argvtest
+
 
 
 (9) The Valgrind test should produce similar output to "make check". If the output
diff --git a/cfg.mk b/cfg.mk
index c0aba57..1bf63ac 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1181,10 +1181,10 @@ exclude_file_name_regexp--sc_prohibit_access_xok = \
 	^(cfg\.mk|src/util/virutil\.c)$$
 
 exclude_file_name_regexp--sc_prohibit_asprintf = \
-  ^(cfg\.mk|bootstrap.conf$$|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c$$)
+  ^(cfg\.mk|bootstrap.conf$$|src/util/virstring\.[ch]$$|tests/vir(cgroup|test)mock\.c$$)
 
 exclude_file_name_regexp--sc_prohibit_strdup = \
-  ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c$$)
+  ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup|test)mock.c$$)
 
 exclude_file_name_regexp--sc_prohibit_close = \
   (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir.+mock\.c)$$)
@@ -1211,7 +1211,7 @@ exclude_file_name_regexp--sc_prohibit_select = \
 	^cfg\.mk$$
 
 exclude_file_name_regexp--sc_prohibit_raw_allocation = \
-  ^(docs/hacking\.html\.in|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|vircgroupmock)\.c|tools/wireshark/src/packet-libvirt\.c)$$
+  ^(docs/hacking\.html\.in|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|vir(cgroup|test)mock)\.c|tools/wireshark/src/packet-libvirt\.c)$$
 
 exclude_file_name_regexp--sc_prohibit_readlink = \
   ^src/(util/virutil|lxc/lxc_container)\.c$$
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 5cd23a2..63d26bd 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -189,6 +189,21 @@
           under gdb or Valgrind.
         </p>
 
+        <p>When running our test suite it may happen that
+        nondeterministic result is produced because of the test
+        suite relying on a particular file in the system being
+        accessible or having some specific value. This is a
+        problem because if ran under different environment the
+        test result may be different and a test can fail. To
+        catch this kind of errors, the test suite has a module
+        for that. The module prints any path touched that fulfils
+        constraints described above into a file. Then
+        <code>VIR_TEST_FILE_ACCESS</code> environment variable
+        can alter location where the file is stored.</p>
+<pre>
+  VIR_TEST_FILE_ACCESS="/tmp/file_access.txt" ./qemuxml2argvtest
+</pre>
+
       </li>
       <li><p>The Valgrind test should produce similar output to
           <code>make check</code>. If the output has traces within libvirt
diff --git a/tests/testutils.c b/tests/testutils.c
index 595b64d..725398c 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -156,6 +156,13 @@ virtTestRun(const char *title,
 {
     int ret = 0;
 
+    /* Some test are fragile about environ settings.  If that's
+     * the case, don't poison it. All this means is that we will
+     * not see which test in particular is touching which file,
+     * but we are still able to tell which binary is doing so. */
+    if (getenv("VIR_TEST_MOCK_PROGNAME"))
+        setenv("VIR_TEST_MOCK_TESTNAME", title, 1);
+
     if (testCounter == 0 && !virTestGetVerbose())
         fprintf(stderr, "      ");
 
@@ -280,6 +287,7 @@ virtTestRun(const char *title,
     }
 #endif /* TEST_OOM */
 
+    unsetenv("VIR_TEST_MOCK_TESTNAME");
     return ret;
 }
 
@@ -851,17 +859,20 @@ int virtTestMain(int argc,
     if (lib)
         VIRT_TEST_PRELOAD(lib);
 
-    virFileActivateDirOverride(argv[0]);
-
-    if (virTestSetEnvPath() < 0)
-        return EXIT_AM_HARDFAIL;
-
-    if (!virFileExists(abs_srcdir))
-        return EXIT_AM_HARDFAIL;
-
     progname = last_component(argv[0]);
     if (STRPREFIX(progname, "lt-"))
         progname += 3;
+
+    setenv("VIR_TEST_MOCK_PROGNAME", progname, 1);
+
+    virFileActivateDirOverride(argv[0]);
+
+    if (virTestSetEnvPath() < 0)
+        return EXIT_AM_HARDFAIL;
+
+    if (!virFileExists(abs_srcdir))
+        return EXIT_AM_HARDFAIL;
+
     if (argc > 1) {
         fprintf(stderr, "Usage: %s\n", argv[0]);
         fputs("effective environment variables:\n"
diff --git a/tests/virtestmock.c b/tests/virtestmock.c
index f138e98..c47eb0c 100644
--- a/tests/virtestmock.c
+++ b/tests/virtestmock.c
@@ -25,6 +25,8 @@
 #include <dlfcn.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <execinfo.h>
+#include <sys/file.h>
 
 #include "internal.h"
 #include "configmake.h"
@@ -37,6 +39,8 @@ static int (*real__xstat)(int ver, const char *path, struct stat *sb);
 static int (*reallstat)(const char *path, struct stat *sb);
 static int (*real__lxstat)(int ver, const char *path, struct stat *sb);
 
+static const char *progname;
+
 static void init_syms(void)
 {
     if (realopen)
@@ -64,17 +68,112 @@ static void init_syms(void)
     LOAD_SYM(access);
     LOAD_SYM_ALT(stat, __xstat);
     LOAD_SYM_ALT(lstat, __lxstat);
+
+}
+
+static void
+printFile(const char *output,
+          const char *file)
+{
+    FILE *fp;
+    const char *testname = getenv("VIR_TEST_MOCK_TESTNAME");
+
+    if (!progname) {
+        progname = getenv("VIR_TEST_MOCK_PROGNAME");
+
+        if (!progname)
+            return;
+    }
+
+    if (!(fp = realfopen(output, "a"))) {
+        fprintf(stderr, "Unable to open %s: %s\n", output, strerror(errno));
+        abort();
+    }
+
+    if (flock(fileno(fp), LOCK_EX) < 0) {
+        fprintf(stderr, "Unable to lock %s: %s\n", output, strerror(errno));
+        fclose(fp);
+        abort();
+    }
+
+    /* Now append the following line into the output file:
+     * $file: $progname $testname */
+
+    fprintf(fp, "%s: %s", file, progname);
+    if (testname)
+        fprintf(fp, ": %s", testname);
+
+    fputc('\n', fp);
+
+    flock(fileno(fp), LOCK_UN);
+    fclose(fp);
+}
+
+
+#define VIR_FILE_ACCESS_DEFAULT abs_builddir "/test_file_access.txt"
+
+static void
+cutOffLastComponent(char *path)
+{
+    char *base = path, *p = base;
+
+    while (*p) {
+        if (*p == '/')
+            base = p;
+        p++;
+    }
+
+    if (base != p)
+        *base = '\0';
 }
 
 static void
 checkPath(const char *path)
 {
+    char *fullPath = NULL;
+    char *relPath = NULL;
+    char *crippledPath = NULL;
+
+    if (path[0] != '/' &&
+        asprintf(&relPath, "./%s", path) < 0)
+        goto error;
+
+    /* Le sigh. Both canonicalize_file_name() and realpath()
+     * expect @path to exist otherwise they return an error. So
+     * if we are called over an non-existent file, this could
+     * return an error. In that case do our best and hope we will
+     * catch possible error. */
+    if ((fullPath = canonicalize_file_name(relPath ? relPath : path))) {
+        path = fullPath;
+    } else {
+        /* Yeah, our worst nightmares just became true. Path does
+         * not exist. Cut off the last component and retry. */
+        if (!(crippledPath = strdup(relPath ? relPath : path)))
+            goto error;
+
+        cutOffLastComponent(crippledPath);
+
+        if ((fullPath = canonicalize_file_name(crippledPath)))
+            path = fullPath;
+    }
+
+
     if (!STRPREFIX(path, abs_topsrcdir) &&
         !STRPREFIX(path, abs_topbuilddir)) {
-        /* Okay, this is a dummy check and spurious print.
-         * But this is going to be replaced soon. */
-        fprintf(stderr, "*** %s ***\n", path);
+        const char *output = getenv("VIR_TEST_FILE_ACCESS");
+        if (!output)
+            output = VIR_FILE_ACCESS_DEFAULT;
+        printFile(output, path);
     }
+
+    free(crippledPath);
+    free(relPath);
+    free(fullPath);
+
+    return;
+ error:
+    fprintf(stderr, "Out of memory\n");
+    abort();
 }
 
 
-- 
2.8.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]