[libvirt] [PATCH 4/4] Replace daemon-conf test script with a proper test case

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; The daemon-conf test script continues to be very fragile to changes in libvirt. It currently fails 1 time in 3/4 due to race conditions in startup/shutdown of the test script. Replace it with a proper test case tailored to the code being tested * tests/Makefile.am: Remove daemon-conf, add libvirtdconftest * tests/daemon-conf: Delete obsolete test * tests/libvirtdconftest.c: Test config file handling --- tests/Makefile.am | 16 +++- tests/daemon-conf | 115 ----------------------- tests/libvirtdconftest.c | 230 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 243 insertions(+), 118 deletions(-) delete mode 100755 tests/daemon-conf create mode 100644 tests/libvirtdconftest.c diff --git a/tests/Makefile.am b/tests/Makefile.am index dd8bf4f..57358e9 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -170,7 +170,6 @@ if WITH_LIBVIRTD test_scripts += \ test_conf.sh \ cpuset \ - daemon-conf \ define-dev-segfault \ int-overflow \ libvirtd-fail \ @@ -185,12 +184,13 @@ test_scripts += \ virsh-schedinfo \ virsh-synopsis -test_programs += eventtest +test_programs += \ + eventtest \ + libvirtdconftest else EXTRA_DIST += \ test_conf.sh \ cpuset \ - daemon-conf \ define-dev-segfault \ int-overflow \ libvirtd-fail \ @@ -445,6 +445,16 @@ commandhelper_SOURCES = \ commandhelper_CFLAGS = -Dabs_builddir="\"`pwd`\"" $(AM_CFLAGS) commandhelper_LDADD = $(LDADDS) +if WITH_LIBVIRTD +libvirtdconftest_SOURCES = \ + libvirtdconftest.c testutils.h testutils.c \ + ../daemon/libvirtd-config.c +libvirtdconftest_CFLAGS = $(AM_CFLAGS) +libvirtdconftest_LDADD = $(LDADDS) +else +EXTRA_DIST += libvirtdconftest.c +endif + virnetmessagetest_SOURCES = \ virnetmessagetest.c testutils.h testutils.c virnetmessagetest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS) diff --git a/tests/daemon-conf b/tests/daemon-conf deleted file mode 100755 index f2b513d..0000000 --- a/tests/daemon-conf +++ /dev/null @@ -1,115 +0,0 @@ -#!/bin/sh -# Get coverage of libvirtd's config-parsing code. - -test -z "$srcdir" && srcdir=$(pwd) -LC_ALL=C -. "$srcdir/test-lib.sh" - -if test "$verbose" = yes; then - $abs_top_builddir/daemon/libvirtd --version -fi - -test_intro "$this_test" - -test -z "$CONFIG_HEADER" && CONFIG_HEADER="$abs_top_builddir/config.h" - -grep '^#define WITH_QEMU 1' "$CONFIG_HEADER" > /dev/null || - skip_test_ "configured without QEMU support" - -conf="$abs_top_srcdir/daemon/libvirtd.conf" - -# Ensure that each commented out PARAMETER = VALUE line has the expected form. -grep -v '\"PARAMETER = VALUE\"' "$conf" | grep '[a-z_] *= *[^ ]' | grep -vE '^#[a-z_]+ = ' \ - && { echo "$0: found unexpected lines (above) in $conf" 1>&2; exit 1; } - -# Start with the sample libvirtd.conf file, uncommenting all real directives. -sed -n 's/^#\([^ #]\)/\1/p' "$conf" > tmp.conf - -sed -e "s/^\(unix_sock_group =\).*/\1 \"$USER\"/g" tmp.conf > k -mv k 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=0 -fail=0 -while :; do - i=$(expr $i + 1) - - param_name=$(sed -n "$i"'s/ = .*//p' tmp.conf) - rhs=$(sed -n "$i"'s/.* = \(.*\)/\1/p' tmp.conf) - f=in$i.conf - case $rhs in - # Change an RHS that starts with '"' or '[' to "3". - [[\"]*) sed "$i"'s/ = [["].*/ = 3/' tmp.conf > $f;; - # Change an RHS that starts with a digit to the string '"foo"'. - [0-9]*) sed "$i"'s/ = [0-9].*/ = "foo"/' tmp.conf > $f;; - esac - - # Run libvirtd, expecting it to fail. - $abs_top_builddir/daemon/libvirtd --pid-file=pid-file --config=$f 2> err && fail=1 - - case $rhs in - # '"'*) msg='should be a string';; - '"'*) msg='invalid type: got long; expected string';; - [0-9]*) 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 - - # Check that the diagnostic we want appears - grep "$msg" err 1>/dev/null 2>&1 - RET=$? - test_result $i "corrupted config $param_name" $RET - test "$RET" = "0" || fail=1 -done - -# Run with the unmodified config file. -sleep_secs=2 - -# Be careful to specify a non-default socket directory: -sed 's,^unix_sock_dir.*,unix_sock_dir="'"$(pwd)"'",' tmp.conf > k || fail=1 -mv k tmp.conf || fail=1 - -# Also, specify a test-specific log directory: -sed 's,^log_outputs.*,log_outputs="3:file:'"$(pwd)/log"'",' tmp.conf > k \ - || fail=1 -mv k tmp.conf || fail=1 - -# Unix socket max path size is 108 on linux. If the generated sock path -# exceeds this, the test will fail, so skip it if CWD is too long -SOCKPATH=`pwd`/libvirt-sock -if test 108 -lt `echo $SOCKPATH | wc -c`; then - skip_test_ "CWD too long" -fi - -# Replace the invalid host_uuid with one that is valid and -# relax audit_level from 2 to 1, otherwise libvirtd will report an error -# when auditing is disabled on the host or during compilation -sed 's/^\(host_uuid =.*\)0"$/\11"/; s/^\(audit_level =.*\)2$/\1 1/' tmp.conf > k -mv k tmp.conf - -$abs_top_builddir/daemon/libvirtd --pid-file=pid-file --config=tmp.conf \ - > log 2>&1 & pid=$! -sleep $sleep_secs -kill $pid - -RET=0 -# Expect an orderly shut-down and successful exit. -wait $pid || RET=1 - -test_result $i "valid config file (sleeping $sleep_secs seconds)" $RET -test $RET = 0 || fail=1 - -test_final $i $fail - -# "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 diff --git a/tests/libvirtdconftest.c b/tests/libvirtdconftest.c new file mode 100644 index 0000000..f97bf80 --- /dev/null +++ b/tests/libvirtdconftest.c @@ -0,0 +1,230 @@ +/* + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange(a)redhat.com&gt; + */ + +#include <config.h> + +#include <stdlib.h> + +#include "testutils.h" +#include "daemon/libvirtd-config.h" +#include "util.h" +#include "c-ctype.h" +#include "virterror_internal.h" +#include "logging.h" +#include "conf.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +struct testCorruptData { + size_t *params; + const char *filedata; + const char *filename; + size_t paramnum; +}; + +static char * +munge_param(const char *datain, + size_t *params, + size_t paramnum, + int *type) +{ + char *dataout; + const char *sol; + const char *eol; + const char *eq; + const char *tmp; + size_t dataoutlen; + const char *replace = NULL; + + sol = datain + params[paramnum]; + eq = strchr(sol, '='); + eol = strchr(sol, '\n'); + + for (tmp = eq + 1; tmp < eol && !replace; tmp++) { + if (c_isspace(*tmp)) + continue; + if (c_isdigit(*tmp)) { + *type = VIR_CONF_LONG; + replace = "\"foo\""; + } else if (*tmp == '[') { + *type = VIR_CONF_LIST; + replace = "666"; + } else { + *type = VIR_CONF_STRING; + replace = "666"; + } + } + + dataoutlen = (eq - datain) + 1 + + strlen(replace) + + strlen(eol) + 1; + + if (VIR_ALLOC_N(dataout, dataoutlen) < 0) { + virReportOOMError(); + return NULL; + } + memcpy(dataout, datain, (eq - datain) + 1); + memcpy(dataout + (eq - datain) + 1, + replace, strlen(replace)); + memcpy(dataout + (eq - datain) + 1 + strlen(replace), + eol, strlen(eol) + 1); + + return dataout; +} + +static int +testCorrupt(const void *opaque) +{ + const struct testCorruptData *data = opaque; + struct daemonConfig *conf = daemonConfigNew(false); + int ret = 0; + int type = VIR_CONF_NONE; + char *newdata = munge_param(data->filedata, + data->params, + data->paramnum, + &type); + virErrorPtr err = NULL; + + if (!newdata) + return -1; + + //VIR_DEBUG("New config [%s]", newdata); + + if (daemonConfigLoadData(conf, data->filename, newdata) != -1) { + VIR_DEBUG("Did not see a failure"); + ret = -1; + goto cleanup; + } + + err = virGetLastError(); + if (!err || !err->message) { + VIR_DEBUG("No error or message %p", err); + ret = -1; + goto cleanup; + } + + switch (type) { + case VIR_CONF_LONG: + if (!strstr(err->message, "invalid type: got string; expected long")) { + VIR_DEBUG("Wrong error for long: '%s'", + err->message); + ret = -1; + } + break; + case VIR_CONF_STRING: + if (!strstr(err->message, "invalid type: got long; expected string")) { + VIR_DEBUG("Wrong error for string: '%s'", + err->message); + ret = -1; + } + break; + case VIR_CONF_LIST: + if (!strstr(err->message, "must be a string or list of strings")) { + VIR_DEBUG("Wrong error for list: '%s'", + err->message); + ret = -1; + } + break; + } + +cleanup: + VIR_FREE(newdata); + daemonConfigFree(conf); + return ret; +} + +static int +uncomment_all_params(char *data, + size_t **ret) +{ + size_t count = 0; + char *tmp; + size_t *params = 0; + + tmp = data; + while (tmp && *tmp) { + tmp = strchr(tmp, '\n'); + if (!tmp) + break; + + tmp++; + + /* Uncomment any lines starting #some_var */ + if (*tmp == '#' && + c_isalpha(*(tmp + 1))) { + if (VIR_EXPAND_N(params, count, 1) < 0) { + VIR_FREE(params); + return -1; + } + *tmp = ' '; + params[count-1] = (tmp + 1) - data; + } + } + if (VIR_EXPAND_N(params, count, 1) < 0) { + VIR_FREE(params); + return -1; + } + params[count-1] = 0; + *ret = params; + return count; +} + +static int +mymain(void) +{ + int ret = 0; + char *filedata = NULL; + char *filename = NULL; + size_t i; + size_t *params = NULL; + + if (virAsprintf(&filename, "%s/../daemon/libvirtd.conf", + abs_srcdir) < 0) { + perror("Format filename"); + return EXIT_FAILURE; + } + + if (virFileReadAll(filename, 1024*1024, &filedata) < 0) { + virErrorPtr err = virGetLastError(); + fprintf(stderr, "Cannot load %s for testing: %s", filename, err->message); + ret = -1; + goto cleanup; + } + + if (uncomment_all_params(filedata, &params) < 0){ + perror("Find params"); + ret = -1; + goto cleanup; + } + VIR_DEBUG("Initial config [%s]", filedata); + for (i = 0 ; params[i] != 0 ; i++) { + const struct testCorruptData data = { params, filedata, filename, i }; + if (virtTestRun("Test corruption", 1, testCorrupt, &data) < 0) + ret = -1; + } + +cleanup: + VIR_FREE(filename); + VIR_FREE(filedata); + VIR_FREE(params); + return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN(mymain) -- 1.7.7.6