From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
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@xxxxxxxxxx>
+ */
+
+#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, ¶ms) < 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list