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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list