So far we are setting only fake secret and storage drivers. Therefore if the code wants to call a public NWFilter API (like qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() are doing) the virGetConnectNWFilter() function will try to actually spawn session daemon because there's no connection object set to handle NWFilter driver. Even though I haven't experienced the same problem with the rest of the drivers (interface, network and node dev), the reasoning above can be applied to them as well. At the same time, now that connection object is registered for the drivers, the public APIs will throw virReportUnsupportedError(). And since we don't provide any error func the error is printed to stderr. Fix this by setting dummy error func. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- tests/qemuxml2argvtest.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 84117a3e63..8901c7bde4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -471,6 +471,10 @@ testCompareXMLToArgv(const void *data) conn->secretDriver = &fakeSecretDriver; conn->storageDriver = &fakeStorageDriver; + virSetConnectInterface(conn); + virSetConnectNetwork(conn); + virSetConnectNWFilter(conn); + virSetConnectNodeDev(conn); virSetConnectSecret(conn); virSetConnectStorage(conn); -- 2.16.4

The check-file-access.pl script is used to match access list generated by virtestmock against whitelisted rules stored in file_access_whitelist.txt. So far the rules are in form: $path: $progname: $testname This is not sufficient because the rule does not take into account 'action' that caused $path to appear in the list of accessed files. After this commit the rule can be in new form: $path: $action: $progname: $testname where $action is one from ("open", "fopen", "access", "stat", "lstat", "connect"). This way the white list can be fine tuned to allow say access() but not connect(). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- tests/check-file-access.pl | 32 +++++++++++++++++++++++++++----- tests/file_access_whitelist.txt | 15 ++++++++++----- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/tests/check-file-access.pl b/tests/check-file-access.pl index 977a2bc533..ea0b7a18a2 100755 --- a/tests/check-file-access.pl +++ b/tests/check-file-access.pl @@ -27,18 +27,21 @@ use warnings; my $access_file = "test_file_access.txt"; my $whitelist_file = "file_access_whitelist.txt"; +my @known_actions = ("open", "fopen", "access", "stat", "lstat", "connect"); + my @files; my @whitelist; open FILE, "<", $access_file or die "Unable to open $access_file: $!"; while (<FILE>) { chomp; - if (/^(\S*):\s*(\S*)(\s*:\s*(.*))?$/) { + if (/^(\S*):\s*(\S*):\s*(\S*)(\s*:\s*(.*))?$/) { my %rec; ${rec}{path} = $1; - ${rec}{progname} = $2; - if (defined $4) { - ${rec}{testname} = $4; + ${rec}{action} = $2; + ${rec}{progname} = $3; + if (defined $5) { + ${rec}{testname} = $5; } push (@files, \%rec); } else { @@ -52,7 +55,21 @@ while (<FILE>) { chomp; if (/^\s*#.*$/) { # comment + } elsif (/^(\S*):\s*(\S*)(:\s*(\S*)(\s*:\s*(.*))?)?$/ and + grep /^$2$/, @known_actions) { + # $path: $action: $progname: $testname + my %rec; + ${rec}{path} = $1; + ${rec}{action} = $3; + if (defined $4) { + ${rec}{progname} = $4; + } + if (defined $6) { + ${rec}{testname} = $6; + } + push (@whitelist, \%rec); } elsif (/^(\S*)(:\s*(\S*)(\s*:\s*(.*))?)?$/) { + # $path: $progname: $testname my %rec; ${rec}{path} = $1; if (defined $3) { @@ -79,6 +96,11 @@ for my $file (@files) { next; } + if (defined %${rule}{action} and + not %${file}{action} =~ m/^$rule->{action}$/) { + next; + } + if (defined %${rule}{progname} and not %${file}{progname} =~ m/^$rule->{progname}$/) { next; @@ -95,7 +117,7 @@ for my $file (@files) { if (not $match) { $error = 1; - print "$file->{path}: $file->{progname}"; + print "$file->{path}: $file->{action}: $file->{progname}"; print ": $file->{testname}" if defined %${file}{testname}; print "\n"; } diff --git a/tests/file_access_whitelist.txt b/tests/file_access_whitelist.txt index 850b28506e..3fb318cbab 100644 --- a/tests/file_access_whitelist.txt +++ b/tests/file_access_whitelist.txt @@ -1,14 +1,17 @@ # This is a whitelist that allows accesses to files not in our # build directory nor source directory. The records are in the -# following format: +# following formats: # # $path: $progname: $testname +# $path: $action: $progname: $testname # -# All these three are evaluated as perl RE. So to allow /dev/sda -# and /dev/sdb, you can just '/dev/sd[a-b]', or to allow +# All these variables are evaluated as perl RE. So to allow +# /dev/sda and /dev/sdb, you can just '/dev/sd[a-b]', or to allow # /proc/$pid/status you can '/proc/\d+/status' and so on. -# Moreover, $progname and $testname can be empty, in which which -# case $path is allowed for all tests. +# Moreover, $action, $progname and $testname can be empty, in which +# which case $path is allowed for all tests. However, $action (if +# specified) must be one of "open", "fopen", "access", "stat", +# "lstat", "connect". /bin/cat: sysinfotest /bin/dirname: sysinfotest: x86 sysinfo @@ -19,5 +22,7 @@ /etc/hosts /proc/\d+/status +/etc/passwd: fopen + # This is just a dummy example, DO NOT USE IT LIKE THAT! .*: nonexistent-test-touching-everything -- 2.16.4

As advertised in the previous commit, we need the list of accessed files to also contain action that caused the $path to appear on the list. Not only this enables us to fine tune our white list rules it also helps us to see why $path is reported. For instance: /run/user/1000/libvirt/libvirt-sock: connect: qemuxml2argvtest: QEMU XML-2-ARGV net-vhostuser-multiq Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- tests/virtestmock.c | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/tests/virtestmock.c b/tests/virtestmock.c index 654af24a10..25aadf8aea 100644 --- a/tests/virtestmock.c +++ b/tests/virtestmock.c @@ -88,7 +88,8 @@ static void init_syms(void) } static void -printFile(const char *file) +printFile(const char *file, + const char *func) { FILE *fp; const char *testname = getenv("VIR_TEST_MOCK_TESTNAME"); @@ -116,9 +117,9 @@ printFile(const char *file) } /* Now append the following line into the output file: - * $file: $progname $testname */ + * $file: $progname: $func: $testname */ - fprintf(fp, "%s: %s", file, progname); + fprintf(fp, "%s: %s: %s", file, func, progname); if (testname) fprintf(fp, ": %s", testname); @@ -128,8 +129,12 @@ printFile(const char *file) fclose(fp); } +#define CHECK_PATH(path) \ + checkPath(path, __FUNCTION__) + static void -checkPath(const char *path) +checkPath(const char *path, + const char *func) { char *fullPath = NULL; char *relPath = NULL; @@ -160,7 +165,7 @@ checkPath(const char *path) if (!STRPREFIX(path, abs_topsrcdir) && !STRPREFIX(path, abs_topbuilddir)) { - printFile(path); + printFile(path, func); } VIR_FREE(crippledPath); @@ -180,7 +185,7 @@ int open(const char *path, int flags, ...) init_syms(); - checkPath(path); + CHECK_PATH(path); if (flags & O_CREAT) { va_list ap; @@ -199,7 +204,7 @@ FILE *fopen(const char *path, const char *mode) { init_syms(); - checkPath(path); + CHECK_PATH(path); return real_fopen(path, mode); } @@ -209,7 +214,7 @@ int access(const char *path, int mode) { init_syms(); - checkPath(path); + CHECK_PATH(path); return real_access(path, mode); } @@ -239,7 +244,7 @@ int stat(const char *path, struct stat *sb) { init_syms(); - checkPath(path); + checkPath(path, "stat"); return real_stat(path, sb); } @@ -250,7 +255,7 @@ int stat64(const char *path, struct stat64 *sb) { init_syms(); - checkPath(path); + checkPath(path, "stat"); return real_stat64(path, sb); } @@ -262,7 +267,7 @@ __xstat(int ver, const char *path, struct stat *sb) { init_syms(); - checkPath(path); + checkPath(path, "stat"); return real___xstat(ver, path, sb); } @@ -274,7 +279,7 @@ __xstat64(int ver, const char *path, struct stat64 *sb) { init_syms(); - checkPath(path); + checkPath(path, "stat"); return real___xstat64(ver, path, sb); } @@ -286,7 +291,7 @@ lstat(const char *path, struct stat *sb) { init_syms(); - checkPath(path); + checkPath(path, "lstat"); return real_lstat(path, sb); } @@ -298,7 +303,7 @@ lstat64(const char *path, struct stat64 *sb) { init_syms(); - checkPath(path); + checkPath(path, "lstat"); return real_lstat64(path, sb); } @@ -310,7 +315,7 @@ __lxstat(int ver, const char *path, struct stat *sb) { init_syms(); - checkPath(path); + checkPath(path, "lstat"); return real___lxstat(ver, path, sb); } @@ -322,7 +327,7 @@ __lxstat64(int ver, const char *path, struct stat64 *sb) { init_syms(); - checkPath(path); + checkPath(path, "lstat"); return real___lxstat64(ver, path, sb); } @@ -337,7 +342,7 @@ int connect(int sockfd, const struct sockaddr *addr, socklen_t addrlen) if (addrlen == sizeof(struct sockaddr_un)) { struct sockaddr_un *tmp = (struct sockaddr_un *) addr; if (tmp->sun_family == AF_UNIX) - checkPath(tmp->sun_path); + CHECK_PATH(tmp->sun_path); } #endif -- 2.16.4