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>

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.

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"

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

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)