tools: Fix connect command

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@redhat.com>

diff --git a/tests/test-lib.sh b/tests/test-lib.sh
index 8e0ce83e118c4fba059b526ebfdc1e1a9831495e..49e8d220957235cb788c96435e2d1316a4aec663 100644 (file)
--- 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 1cf3d22ec74c176a42e2e78b2f2774fe47630871..564e3dc42c72b99c3bf6708f064c75754e5c3eb4 100755 (executable)
--- a/tests/virsh-uriprecedence
+++ b/tests/virsh-uriprecedence
@@ -7,7 +7,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"; }
@@ -22,16 +23,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):
@@ -56,6 +85,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"
@@ -70,10 +100,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 af072510870de3cbfd5cf26fe1dc3a5f81d5b8d5..2a807d99af71d60fa9c523f321582b7acdc82148 100644 (file)
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -211,10 +211,11 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly)
  *
  */
 static void
-virshReconnect(vshControl *ctl)
+virshReconnect(vshControl *ctl, const char *name, bool readonly, bool force)
 {
     bool connected = false;
     virshControlPtr priv = ctl->privData;
+    bool ro = name ? readonly : priv->readonly;
 
     if (priv->conn) {
         int ret;
@@ -229,7 +230,7 @@ virshReconnect(vshControl *ctl)
                                   "disconnect from the hypervisor"));
     }
 
-    priv->conn = virshConnect(ctl, ctl->connname, priv->readonly);
+    priv->conn = virshConnect(ctl, name ? name : ctl->connname, ro);
 
     if (!priv->conn) {
         if (disconnected)
@@ -237,10 +238,15 @@ virshReconnect(vshControl *ctl)
         else
             vshError(ctl, "%s", _("failed to connect to the hypervisor"));
     } else {
+        if (name) {
+            VIR_FREE(ctl->connname);
+            ctl->connname = vshStrdup(ctl, name);
+            priv->readonly = readonly;
+        }
         if (virConnectRegisterCloseCallback(priv->conn, virshCatchDisconnect,
                                             ctl, NULL) < 0)
             vshError(ctl, "%s", _("Unable to register disconnect callback"));
-        if (connected)
+        if (connected && !force)
             vshError(ctl, "%s", _("Reconnected to the hypervisor"));
     }
     disconnected = 0;
@@ -291,43 +297,11 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd)
 {
     bool ro = vshCommandOptBool(cmd, "readonly");
     const char *name = NULL;
-    virshControlPtr priv = ctl->privData;
-    virConnectPtr conn;
 
     if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
         return false;
 
-    conn = virshConnect(ctl, name, ro);
-
-    if (!conn) {
-        vshError(ctl, "%s", _("Failed to connect to the hypervisor"));
-        return false;
-    }
-
-    if (priv->conn) {
-        int ret;
-
-        virConnectUnregisterCloseCallback(priv->conn, virshCatchDisconnect);
-        ret = virConnectClose(priv->conn);
-        if (ret < 0)
-            vshError(ctl, "%s", _("Failed to disconnect from the hypervisor"));
-        else if (ret > 0)
-            vshError(ctl, "%s", _("One or more references were leaked after "
-                                  "disconnect from the hypervisor"));
-    }
-    priv->conn = conn;
-
-    VIR_FREE(ctl->connname);
-    ctl->connname = vshStrdup(ctl, name);
-
-    priv->useGetInfo = false;
-    priv->useSnapshotOld = false;
-    priv->blockJobNoBytes = false;
-    priv->readonly = ro;
-
-    if (virConnectRegisterCloseCallback(priv->conn, virshCatchDisconnect,
-                                        ctl, NULL) < 0)
-        vshError(ctl, "%s", _("Unable to register disconnect callback"));
+    virshReconnect(ctl, name, ro, true);
 
     return true;
 }
@@ -360,7 +334,7 @@ virshConnectionHandler(vshControl *ctl)
     virshControlPtr priv = ctl->privData;
 
     if (!priv->conn || disconnected)
-        virshReconnect(ctl);
+        virshReconnect(ctl, NULL, false, false);
 
     if (virshConnectionUsability(ctl, priv->conn))
         return priv->conn;
@@ -431,7 +405,7 @@ virshInit(vshControl *ctl)
         return false;
 
     if (ctl->connname) {
-        virshReconnect(ctl);
+        virshReconnect(ctl, NULL, false, false);
         /* Connecting to a named connection must succeed, but we delay
          * connecting to the default connection until we need it
          * (since the first command might be 'connect' which allows a

diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 22160ad929d0dcc0c86bdeda3c3fbde8e6784dce..4275aa37a1bfb477655a6e0bbaf5a043947517c9 100644 (file)
--- 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)