[libvirt] [PATCH v2] tests: Add URI precedence checking
Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having to deal with this issue again, herec comes "virsh-uriprecedence". Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- Notes: v2: - Changed the name to virsh-uriprecedence - Using $PWD instead of temporary dir tests/Makefile.am | 2 ++ tests/virsh-uriprecedence | 76 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100755 tests/virsh-uriprecedence diff --git a/tests/Makefile.am b/tests/Makefile.am index 09144d6..2fb8f37 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -98,6 +98,7 @@ EXTRA_DIST = \ storagevolxml2xmlout \ sysinfodata \ test-lib.sh \ + virsh-uriprecedence \ vmx2xmldata \ xencapsdata \ xmconfigdata \ @@ -235,6 +236,7 @@ test_scripts += \ read-bufsiz \ read-non-seekable \ start \ + virsh-uriprecedence \ vcpupin \ virsh-all \ virsh-optparse \ diff --git a/tests/virsh-uriprecedence b/tests/virsh-uriprecedence new file mode 100755 index 0000000..f4d84a4 --- /dev/null +++ b/tests/virsh-uriprecedence @@ -0,0 +1,76 @@ +#!/bin/sh + +: ${srcdir=.} +. $srcdir/test-lib.sh + +# This test checks if virsh obeys the proper precedence of different +# URI settings +test_intro "virsh-uriprecedence" + +virsh_bin="$abs_top_builddir/tools/virsh" +counter=1 +ret=0 + +cleanup_() { rm -rf "$tmphome"; } + +# Create all mock files/directories to avoid permission problems +tmphome="$PWD/tmp_home" +export XDG_CONFIG_HOME="$tmphome/.config" +export XDG_CACHE_HOME="$tmphome/.cache" +export XDG_RUNTIME_HOME="XDG_CACHE_HOME" + +mkdir -p "$XDG_CONFIG_HOME/libvirt" "$XDG_CONFIG_HOME/virsh" +mkdir -p "$XDG_CACHE_HOME/libvirt" "$XDG_CACHE_HOME/virsh" +mkdir -p "$XDG_RUNTIME_HOME/libvirt" "$XDG_RUNTIME_HOME/virsh" + +# Main function checking for the proper uri being returned +test_uri() +{ + result=0 + if [ "$($virsh_bin uri)" != "$good_uri" ]; then + result=1 + ret=1 + fi + test_result "$counter" "$1" "$result" + counter="$((counter+1))" +} + +# Precedence is the following (lowest priority first): +# +# 1) if run as root, 'uri_default' from /etc/libvirtd/libvirt.conf, +# otherwise qemu:///session. There is no way to mock this file for +# virsh/libvirt.so and the user may have set anything in there that +# would spoil the test, so we don't test this +# +# 2) 'uri_default' from $XDG_CONFIG_HOME/libvirt/libvirt.conf +# +# 3) LIBVIRT_DEFAULT_URI +# +# 4) VIRSH_DEFAULT_CONNECT_URI +# +# 5) parameter -c (--connect) + +unset LIBVIRT_DEFAULT_URI +unset VIRSH_DEFAULT_CONNECT_URI +bad_uri="test:///default?bad_uri" +good_uri="test:///default?good_uri" + +printf "uri_default=\"%s\"\n" "$good_uri" >"$XDG_CONFIG_HOME/libvirt/libvirt.conf" +test_uri "User config file" + +printf "uri_default=\"%s\"\n" "$bad_uri" >"$XDG_CONFIG_HOME/libvirt/libvirt.conf" +export LIBVIRT_DEFAULT_URI="$good_uri" +test_uri "LIBVIRT_DEFAULT_URI" + +export LIBVIRT_DEFAULT_URI="$bad_uri" +export VIRSH_DEFAULT_CONNECT_URI="$good_uri" +test_uri "VIRSH_DEFAULT_CONNECT_URI" + +export VIRSH_DEFAULT_CONNECT_URI="$bad_uri" +virsh_bin="$virsh_bin --connect $good_uri" +test_uri "Parameter" + +# test_uri() increases $counter even for the last test, so we must +# decrement it +test_final "$((counter-1))" "$ret" +(exit "$ret"); exit "$ret" -- 1.8.3.2
On 08/22/2013 02:19 PM, Martin Kletzander wrote:
Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having to deal with this issue again, herec comes "virsh-uriprecedence".
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
Notes: v2: - Changed the name to virsh-uriprecedence - Using $PWD instead of temporary dir
tests/Makefile.am | 2 ++ tests/virsh-uriprecedence | 76 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100755 tests/virsh-uriprecedence
ACK Jan
On Thu, Aug 22, 2013 at 02:19:56PM +0200, Martin Kletzander wrote:
Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having to deal with this issue again, herec comes "virsh-uriprecedence".
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
Notes: v2: - Changed the name to virsh-uriprecedence - Using $PWD instead of temporary dir
tests/Makefile.am | 2 ++ tests/virsh-uriprecedence | 76 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100755 tests/virsh-uriprecedence
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 08/22/2013 02:56 PM, Daniel P. Berrange wrote:
On Thu, Aug 22, 2013 at 02:19:56PM +0200, Martin Kletzander wrote:
Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having to deal with this issue again, herec comes "virsh-uriprecedence".
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
Notes: v2: - Changed the name to virsh-uriprecedence - Using $PWD instead of temporary dir
tests/Makefile.am | 2 ++ tests/virsh-uriprecedence | 76 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100755 tests/virsh-uriprecedence
ACK
Thanks guys, pushed now. Martin
On 08/22/2013 06:19 AM, Martin Kletzander wrote:
Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having to deal with this issue again, herec comes "virsh-uriprecedence".
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
@@ -235,6 +236,7 @@ test_scripts += \ read-bufsiz \ read-non-seekable \ start \ + virsh-uriprecedence \ vcpupin \
No longer sorted after the rename :)
+++ b/tests/virsh-uriprecedence @@ -0,0 +1,76 @@ +#!/bin/sh
I'm reviewing this for POSIX sh compatibility...
+# Create all mock files/directories to avoid permission problems +tmphome="$PWD/tmp_home" +export XDG_CONFIG_HOME="$tmphome/.config" +export XDG_CACHE_HOME="$tmphome/.cache" +export XDG_RUNTIME_HOME="XDG_CACHE_HOME" + +mkdir -p "$XDG_CONFIG_HOME/libvirt" "$XDG_CONFIG_HOME/virsh" +mkdir -p "$XDG_CACHE_HOME/libvirt" "$XDG_CACHE_HOME/virsh" +mkdir -p "$XDG_RUNTIME_HOME/libvirt" "$XDG_RUNTIME_HOME/virsh"
Isn't this last line redundant, since XDG_RUNTIME_HOME is equal to XDG_CACHE_HOME? Also, you could use one 'mkdir -p' to make all the directories (but shaving processes isn't essential).
+ +printf "uri_default=\"%s\"\n" "$good_uri" >"$XDG_CONFIG_HOME/libvirt/libvirt.conf"
Style - slightly shorter as printf 'uri_default="%s"\n' ... Long line - is it worth wrapping with \-newline? Everything looks portable - I have no objection to this patch as-is, although if you haven't pushed yet, you can consider tweaking the nits I pointed out. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu 22 Aug 2013 03:15:48 PM CEST, Eric Blake wrote:
On 08/22/2013 06:19 AM, Martin Kletzander wrote:
Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having to deal with this issue again, herec comes "virsh-uriprecedence".
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
@@ -235,6 +236,7 @@ test_scripts += \ read-bufsiz \ read-non-seekable \ start \ + virsh-uriprecedence \ vcpupin \
No longer sorted after the rename :)
Oh, rushing the v2 always gets me :(
+++ b/tests/virsh-uriprecedence @@ -0,0 +1,76 @@ +#!/bin/sh
I'm reviewing this for POSIX sh compatibility...
+# Create all mock files/directories to avoid permission problems +tmphome="$PWD/tmp_home" +export XDG_CONFIG_HOME="$tmphome/.config" +export XDG_CACHE_HOME="$tmphome/.cache" +export XDG_RUNTIME_HOME="XDG_CACHE_HOME" + +mkdir -p "$XDG_CONFIG_HOME/libvirt" "$XDG_CONFIG_HOME/virsh" +mkdir -p "$XDG_CACHE_HOME/libvirt" "$XDG_CACHE_HOME/virsh" +mkdir -p "$XDG_RUNTIME_HOME/libvirt" "$XDG_RUNTIME_HOME/virsh"
Isn't this last line redundant, since XDG_RUNTIME_HOME is equal to XDG_CACHE_HOME? Also, you could use one 'mkdir -p' to make all the directories (but shaving processes isn't essential).
Yes, it can. I thought it'll be better for the future and understandability and won't hurt in case there's one 'mkdir -p'. Unfortunately I the haven't used it.
+ +printf "uri_default=\"%s\"\n" "$good_uri" >"$XDG_CONFIG_HOME/libvirt/libvirt.conf"
Style - slightly shorter as printf 'uri_default="%s"\n' ... Long line - is it worth wrapping with \-newline?
Definitely, I missed this one.
Everything looks portable - I have no objection to this patch as-is, although if you haven't pushed yet, you can consider tweaking the nits I pointed out.
Unfortunately, I pushed it with the thought that 2 ACKs should cover anything. However, I'll be more than happy to fix these in a follow-up patch if you think it's worth it. Martin
On 08/22/2013 07:32 AM, Martin Kletzander wrote:
Everything looks portable - I have no objection to this patch as-is, although if you haven't pushed yet, you can consider tweaking the nits I pointed out.
Unfortunately, I pushed it with the thought that 2 ACKs should cover anything. However, I'll be more than happy to fix these in a follow-up patch if you think it's worth it.
No, nothing I pointed out was fatal, so I won't worry if there is no followup. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Ján Tomko -
Martin Kletzander