Until now, there has been no coverage of libvirtd's config-processing code. In adding this, (and esp. in comparing actual/expected diagnostics), I noticed and fixed a trivial inconsistency: for a parameter expecting a string value, an invalid values, you'd get a different diagnostic for the four config parameters that are parsed via remoteConfigGetAuth than for all of the other config parameters (which go through checkType). This fixes that, too: Test libvirtd's config-processing code. And remove a minor diagnostic inconsistency. * tests/daemon-conf: New test. * tests/Makefile.am (TESTS_ENVIRONMENT): Prepend qemud/ to PATH, so we can invoke libvirtd without an absolute name. (test_scripts): Add daemon-conf. * qemud/qemud.c (remoteConfigGetAuth): Use checkType, rather than open-coding it with a different diagnostic. Signed-off-by: Jim Meyering <meyering@redhat.com> --- qemud/qemud.c | 4 +-- tests/Makefile.am | 3 ++ tests/daemon-conf | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 3 deletions(-) create mode 100755 tests/daemon-conf diff --git a/qemud/qemud.c b/qemud/qemud.c index 9794c03..f15cadd 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -1783,10 +1783,8 @@ static int remoteConfigGetAuth(virConfPtr conf, const char *key, int *auth, cons if (!p) return 0; - if (p->type != VIR_CONF_STRING) { - qemudLog (QEMUD_ERR, "remoteReadConfigFile: %s: %s: should be a string\n", filename, key); + if (checkType (p, filename, key, VIR_CONF_STRING) < 0) return -1; - } if (!p->str) return 0; diff --git a/tests/Makefile.am b/tests/Makefile.am index bec8b05..a918bcc 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -47,7 +47,9 @@ noinst_PROGRAMS = xmlrpctest xml2sexprtest sexpr2xmltest virshtest conftest \ nodeinfotest test_scripts = \ + daemon-conf \ int-overflow + EXTRA_DIST += $(test_scripts) TESTS = xml2sexprtest sexpr2xmltest virshtest test_conf.sh xmconfigtest \ @@ -63,6 +65,7 @@ endif TESTS_ENVIRONMENT = \ abs_top_builddir=`pwd`/'$(top_builddir)' \ abs_top_srcdir=`pwd`/'$(top_srcdir)' \ + PATH='$(abs_top_builddir)/qemud$(PATH_SEPARATOR)'"$$PATH" \ $(VG) valgrind: diff --git a/tests/daemon-conf b/tests/daemon-conf new file mode 100755 index 0000000..db1f0d3 --- /dev/null +++ b/tests/daemon-conf @@ -0,0 +1,64 @@ +#!/bin/sh +# Get coverage of libvirtd's config-parsing code. + +# Boilerplate code to set up a test directory, cd into it, +# and to ensure we remove it upon completion. +this_test_() { echo "./$0" | sed 's,.*/,,'; } +t_=$(this_test_)-$$ +init_cwd_=$(pwd) +trap 'st=$?; d='"$t_"'; + cd '"$init_cwd_"' && chmod -R u+rwx "$d" && rm -rf "$d" && exit $st' 0 +trap '(exit $?); exit $?' 1 2 13 15 +mkdir "$t_" || fail=1 +cd "$t_" || fail=1 + +# Start with the sample libvirtd.conf file, uncommenting all real directives. +sed -n 's/^#\([^ #]\)/\1/p' $abs_top_srcdir/qemud/libvirtd.conf > tmp.conf + +# Iterate through that list of directives, corrupting one RHS at a +# time and running libvirtd with the resulting config. Each libvirtd +# invocation must fail. +n=$(wc -l < tmp.conf) +i=1 +while :; do + param_name=$(sed -n "$i"'s/ = .*//p' tmp.conf) + rhs=$(sed -n "$i"'s/.* = \(.*\)/\1/p' tmp.conf) + f=in$i.conf + # Change an RHS that starts with '"' or '[' to "3". + # Change an RHS that starts with 0 or 1 to the string '"foo"'. + sed "$i"'s/ = [["].*/ = 3/;'"$i"'s/ = [01].*/ = "foo"/' tmp.conf > $f + libvirtd --config=$f 2> err && fail=1 + case $rhs in + # '"'*) msg='should be a string';; + '"'*) msg='invalid type: got long; expected string';; + [01]*) msg='invalid type: got string; expected long';; + '['*) msg='must be a string or list of strings';; + *) echo "unexpected RHS: $rhs" 1>&2; fail=1;; + esac + + test $i = $n && break + + # Filter out this diagnostic. + sed '/^Cannot set group when not running as root$/d' err > k && mv k err + + printf '%s\n\n' "remoteReadConfigFile: $f: $param_name: $msg" > expected-err + diff -u expected-err err || fail=1 + + i=$(expr $i + 1) +done + +# Run with the unmodified config file. +libvirtd --config=tmp.conf > log 2>&1 & pid=$! +sleep 2 +kill $pid + +# Expect an orderly shut-down and successful exit. +wait $pid || fail=1 + +# "cat log" would print this for non-root: +# Cannot set group when not running as root +# Shutting down on signal 15 +# And normally, we'd require that output, but obviously +# it'd be different for root. + +exit $fail -- 1.5.3.7.1116.gae2a9