[libvirt] [PATCH] tools: Fix connect command

[Martin Kletzander]

The man page says: "(Re)-Connect to the hypervisor. When the shell is
first started, this is automatically run with the URI parameter
requested by the "-c" option on the command line."  However, if you run:

  virsh -c 'test://default' 'connect; uri'

the output will not be 'test://default'.  That's because the 'connect'
command does not care about any virsh-only related settings and if it is
run without parameters, it connects with @uri == NULL.  Not only that
doesn't comply to what the man page describes, but it also doesn't make
sense.  It also means you aren't able to reconnect to whatever you are
connected currently.

So let's fix that in both virsh and virt-admin add a test case for it.

Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
---
 tests/test-lib.sh         |  6 ++++++
 tests/virsh-uriprecedence | 54 +++++++++++++++++++++++++++++++++++------------
 tools/virsh.c             |  6 ++++--
 tools/virt-admin.c        |  6 ++++--
 4 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/tests/test-lib.sh b/tests/test-lib.sh
index 8e0ce83e118c..49e8d2209572 100644
--- a/tests/test-lib.sh
+++ b/tests/test-lib.sh
@@ -232,6 +232,12 @@ if test -n "$VIR_TEST_DEBUG" || test -n "$VIR_TEST_VERBOSE" ; then
   verbose=1
 fi

+debug() { :; }
+
+if test "$VIR_TEST_DEBUG" = "2"; then
+  debug() { echo "$@"; }
+fi
+
 # This is a stub function that is run upon trap (upon regular exit and
 # interrupt).  Override it with a per-test function, e.g., to unmount
 # a partition, or to undo any other global state changes.
diff --git a/tests/virsh-uriprecedence b/tests/virsh-uriprecedence
index f9e325658900..4bcce3aeb08f 100755
--- a/tests/virsh-uriprecedence
+++ b/tests/virsh-uriprecedence
@@ -8,7 +8,8 @@
 test_intro "virsh-uriprecedence"

 virsh_bin="$abs_top_builddir/tools/virsh"
-counter=1
+virsh_cmd="$virsh_bin"
+counter=0
 ret=0

 cleanup_() { rm -rf "$tmphome"; }
@@ -23,16 +24,44 @@ 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
+is_uri_good()
+{
+    echo "$1" | grep -q -F "$good_uri"
+}
+
+test_uri_internal()
+{
+    test_name=$1
+    test_cmd="$virsh_cmd \"$2\""
+    result=0
+
+    debug "Running '$test_cmd'"
+    out="$($virsh_cmd "$2")"
+
+    if ! is_uri_good "$out"; then
+        debug "Invalid output: '$out'"
+        result=1
+        ret=1
+    fi
+
+    counter="$((counter+1))"
+    test_result "$counter" "$1" "$result"
+}
+
+test_uri_connect()
+{
+    test_uri_internal "$1" "connect; uri"
+}
+
+test_uri_noconnect()
+{
+    test_uri_internal "$1" "uri"
+}
+
 test_uri()
 {
-  result=0
-  if [ "$($virsh_bin uri)" != "$good_uri" ]; then
-      result=1
-      ret=1
-  fi
-  test_result "$counter" "$1" "$result"
-  counter="$((counter+1))"
+    test_uri_connect "$1"
+    test_uri_noconnect "$1"
 }

 # Precedence is the following (lowest priority first):
@@ -57,6 +86,7 @@ good_uri="test:///default?good_uri"

 printf "uri_default=\"%s\"\n" "$good_uri" >"$XDG_CONFIG_HOME/libvirt/libvirt.conf"
 if uid_is_privileged_; then
+    counter="$((counter+1))"
     test_skip_case "$counter" "User config file" "must not be run as root"
 else
     test_uri "User config file"
@@ -71,10 +101,8 @@ 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"
+virsh_cmd="$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"
+test_final "$counter" "$ret"
 (exit "$ret"); exit "$ret"
diff --git a/tools/virsh.c b/tools/virsh.c
index 366956cd5c75..d17aaf0be8ea 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -306,11 +306,13 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd)
         priv->conn = NULL;
     }

-    VIR_FREE(ctl->connname);
     if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
         return false;

-    ctl->connname = vshStrdup(ctl, name);
+    if (name) {
+        VIR_FREE(ctl->connname);
+        ctl->connname = vshStrdup(ctl, name);
+    }

     priv->useGetInfo = false;
     priv->useSnapshotOld = false;
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 22160ad929d0..4275aa37a1bf 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -291,8 +291,10 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd)
     if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
         return false;

-    VIR_FREE(ctl->connname);
-    ctl->connname = vshStrdup(ctl, name);
+    if (name) {
+        VIR_FREE(ctl->connname);
+        ctl->connname = vshStrdup(ctl, name);
+    }

     vshAdmReconnect(ctl);
     if (!connected)
-- 
2.8.1