New subject: [libvirt] [PATCH] libvirtd: new config-file option: unix_sock_dir [was Re: adding tests....

Thursday, 8 January 2009

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(a)redhat.com&gt;
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(a)redhat.com&gt;
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(a)redhat.com&gt;
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


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

[libvirt] adding tests....