In adding a couple of tests, I noticed that libvirtd --config=no-such
didn't diagnose my mistake.
I fixed that with the first patch below:
diagnose "libvirtd --config=no-such-file"
* qemud/qemud.c (remoteReadConfigFile): Don't return 0 (success)
when the config file is unreadable or nonexistent
Return -1, not 0, upon virConfReadFile failure.
(main): If remote_config_file is not specified via --config(-f),
use the default config file only if it exists. Otherwise,
use /dev/null.
However, that made it so libvirtd gave two diagnostics:
Failed to open file 'no-such': No such file or directory
libvir: Config error : failed to open no-such for reading
The latter part of that patch fixes it like this:
* src/conf.c (virConfReadFile): Don't diagnose virFileReadAll
failure, since it already does that.
Finally, I went to add the two tests,
one to ensure libvirtd --config=no-such-file now fails, as I expected
another to start libvirtd and then run a small pool-related test via
a separate virsh invocation.
But that made me see a bug in tests/Makefile.am:
A missing backslash made it so the virsh-all test wasn't being run.
Easy to fix. But then, I saw when virsh-all runs it generated too
much output, so I did this:
tests: quiet virsh-all
* tests/virsh-all: For now, ignore diagnostics and exit status,
when running all virsh commands.
Finally, here are the two tests:
add tests
* tests/libvirtd-pool: New file.
* tests/libvirtd-fail: New file.
* tests/Makefile.am (test_scripts): Add omitted backslash,
so that the virsh-all test is run.
Add libvirtd-fail and libvirtd-pool.
>From aaea6d51f5200e58086d9cabf8b6e7fac7c460f8 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@xxxxxxxxxx>
Date: Thu, 8 Jan 2009 20:17:35 +0100
Subject: [PATCH 1/3] diagnose "libvirtd --config=no-such-file"
* qemud/qemud.c (remoteReadConfigFile): Don't return 0 (success)
when the config file is unreadable or nonexistent
Return -1, not 0, upon virConfReadFile failure.
(main): If remote_config_file is not specified via --config(-f),
use the default config file only if it exists. Otherwise,
use /dev/null.
* src/conf.c (virConfReadFile): Don't diagnose virFileReadAll
failure, since it already does that.
---
qemud/qemud.c | 20 ++++++++++++--------
src/conf.c | 3 +--
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/qemud/qemud.c b/qemud/qemud.c
index c3d3c02..9835e0a 100644
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -1,7 +1,7 @@
/*
* qemud.c: daemon start of day, guest process & i/o management
*
- * Copyright (C) 2006, 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -2116,13 +2116,8 @@ remoteReadConfigFile (struct qemud_server *server, const char *filename)
auth_unix_ro = REMOTE_AUTH_NONE;
#endif
- /* Just check the file is readable before opening it, otherwise
- * libvirt emits an error.
- */
- if (access (filename, R_OK) == -1) return 0;
-
conf = virConfReadFile (filename);
- if (!conf) return 0;
+ if (!conf) return -1;
/*
* First get all the logging settings and activate them
@@ -2301,7 +2296,7 @@ int main(int argc, char **argv) {
struct sigaction sig_action;
int sigpipe[2];
const char *pid_file = NULL;
- const char *remote_config_file = SYSCONF_DIR "/libvirt/libvirtd.conf";
+ const char *remote_config_file = NULL;
int ret = 1;
struct option opts[] = {
@@ -2372,6 +2367,15 @@ int main(int argc, char **argv) {
}
}
+ if (remote_config_file == NULL) {
+ static const char *default_config_file
+ = SYSCONF_DIR "/libvirt/libvirtd.conf";
+ remote_config_file =
+ (access(default_config_file, X_OK) == 0
+ ? default_config_file
+ : "/dev/null");
+ }
+
if (godaemon) {
if (qemudGoDaemon() < 0) {
VIR_ERROR(_("Failed to fork as daemon: %s"), strerror(errno));
diff --git a/src/conf.c b/src/conf.c
index 9f0fc34..339a150 100644
--- a/src/conf.c
+++ b/src/conf.c
@@ -1,7 +1,7 @@
/**
* conf.c: parser for a subset of the Python encoded Xen configuration files
*
- * Copyright (C) 2006, 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -712,7 +712,6 @@ virConfReadFile(const char *filename)
}
if ((len = virFileReadAll(filename, MAX_CONFIG_FILE_SIZE, &content)) < 0) {
- virConfError(NULL, VIR_ERR_OPEN_FAILED, filename);
return NULL;
}
--
1.6.1.141.gfe98e
>From e70a4556a8422779b7997ab03997af4b9a03087c Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@xxxxxxxxxx>
Date: Thu, 8 Jan 2009 19:58:19 +0100
Subject: [PATCH 2/3] tests: quiet virsh-all
* tests/virsh-all: For now, ignore diagnostics and exit status,
when running all virsh commands.
---
tests/virsh-all | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/virsh-all b/tests/virsh-all
index f1c84a3..03ea466 100755
--- a/tests/virsh-all
+++ b/tests/virsh-all
@@ -1,7 +1,7 @@
#!/bin/sh
# blindly run each and every command listed by "virsh help"
-# Copyright (C) 2008 Free Software Foundation, Inc.
+# Copyright (C) 2008, 2009 Free Software Foundation, Inc.
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -35,7 +35,8 @@ test -n "$cmds" || framework_failure
for i in $cmds; do
echo testing $i... 1>&2
- virsh -c $test_url $i < /dev/null
+ # For now, just run the command and ignore output and exit status.
+ virsh -c $test_url $i < /dev/null > /dev/null 2>&1
done
(exit $fail); exit $fail
--
1.6.1.141.gfe98e
>From 307c465a008cbb4a497a6553046a036e95843519 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@xxxxxxxxxx>
Date: Thu, 8 Jan 2009 20:18:17 +0100
Subject: [PATCH 3/3] add two tests
* tests/libvirtd-pool: New file.
* tests/libvirtd-fail: New file.
* tests/Makefile.am (test_scripts): Add omitted backslash,
so that the virsh-all test is run.
Add libvirtd-fail and libvirtd-pool.
---
tests/Makefile.am | 24 ++++++++++++----------
tests/libvirtd-fail | 20 +++++++++++++++++++
tests/libvirtd-pool | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 86 insertions(+), 11 deletions(-)
create mode 100755 tests/libvirtd-fail
create mode 100755 tests/libvirtd-pool
diff --git a/tests/Makefile.am b/tests/Makefile.am
index d8b5e44..a3e4383 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -56,17 +56,19 @@ endif
test_scripts = domainschematest
if WITH_LIBVIRTD
-test_scripts += \
- test_conf.sh \
- cpuset \
- daemon-conf \
- int-overflow \
- read-bufsiz \
- read-non-seekable \
- start \
- undefine \
- vcpupin
- virsh-all
+test_scripts += \
+ cpuset \
+ daemon-conf \
+ int-overflow \
+ libvirtd-fail \
+ libvirtd-pool \
+ read-bufsiz \
+ read-non-seekable \
+ start \
+ test_conf.sh \
+ undefine \
+ vcpupin \
+ virsh-all \
virsh-synopsis
endif
diff --git a/tests/libvirtd-fail b/tests/libvirtd-fail
new file mode 100755
index 0000000..724d7c6
--- /dev/null
+++ b/tests/libvirtd-fail
@@ -0,0 +1,20 @@
+#!/bin/sh
+# Ensure that libvirt fails when given nonexistent --config=FILE
+
+if test "$VERBOSE" = yes; then
+ set -x
+ libvirtd --version
+fi
+
+test -z "$srcdir" && srcdir=$(pwd)
+test -z "$abs_top_srcdir" && abs_top_srcdir=$(pwd)/..
+. "$srcdir/test-lib.sh"
+
+fail=0
+
+libvirtd --config=no-such-file > log 2>&1 && fail=1
+cat <<\EOF > exp
+Failed to open file 'no-such-file': No such file or directory
+EOF
+
+compare exp log
diff --git a/tests/libvirtd-pool b/tests/libvirtd-pool
new file mode 100755
index 0000000..72afa12
--- /dev/null
+++ b/tests/libvirtd-pool
@@ -0,0 +1,53 @@
+#!/bin/sh
+# Get coverage of libvirtd's config-parsing code.
+
+if test "$VERBOSE" = yes; then
+ set -x
+ libvirtd --version
+ virsh --version
+fi
+
+test -z "$srcdir" && srcdir=$(pwd)
+test -z "$abs_top_srcdir" && abs_top_srcdir=$(pwd)/..
+. "$srcdir/test-lib.sh"
+
+fail=0
+
+libvirtd > log 2>&1 & pid=$!
+sleep 1
+
+virsh --connect qemu:///session \
+ pool-define-as P dir src-host /src/path /src/dev S /target-path > out 2>&1 \
+ || fail=1
+virsh --connect qemu:///session pool-dumpxml P >> out 2>&1 || fail=1
+
+# remove random uuid
+sed 's/<uuid>.*/-/' out > k && mv k out || fail=1
+
+kill $pid
+
+cat <<EOF > exp
+Pool P defined
+
+<pool type='dir'>
+ <name>P</name>
+ -
+ <capacity>0</capacity>
+ <allocation>0</allocation>
+ <available>0</available>
+ <source>
+ </source>
+ <target>
+ <path>/target-path</path>
+ <permissions>
+ <mode>0700</mode>
+ <owner>500</owner>
+ <group>500</group>
+ </permissions>
+ </target>
+</pool>
+
+EOF
+
+compare exp out
+compare /dev/null log
--
1.6.1.141.gfe98e
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list