[libvirt] adding tests....

[Daniel P. Berrange]

On Thu, Jan 08, 2009 at 08:55:31PM +0100, Jim Meyering wrote:
> 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.

Ok, makes sense.

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


ACK

> @@ -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");
> +    }

Indentation looks off-by-2 there.

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

Using qemu:///session here is fragile because it'll see
all existing user defined vms/network/storage/etc. Use
the test:///default  driver instead (or test:///path/to/custom/config.xml)

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|