Replace daemon-conf test script with a proper test case

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

.gitignore

@@ -150,6 +150,7 @@
 /tests/vmx2xmltest
 /tests/xencapstest
 /tests/xmconfigtest
+/tests/libvirtdconftest
 /tools/*.[18]
 /tools/libvirt-guests.init
 /tools/virsh

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)

tests/daemon-conf

@@ -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

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@redhat.com>
+ */
+
+#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)