9:49 a.m.
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(a)redhat.com>
---
v3:
- Don't say "error: Reconnected..." when connected by a user request
(by using the 'connect' command)
v2:
- rebased on top of Cole's changes
- https://www.redhat.com/archives/libvir-list/2016-April/msg01551.html
v1:
- https://www.redhat.com/archives/libvir-list/2016-April/msg01462.html
tests/test-lib.sh | 6 ++++++
tests/virsh-uriprecedence | 54 +++++++++++++++++++++++++++++++++++------------
tools/virsh.c | 50 +++++++++++--------------------------------
tools/virt-admin.c | 6 ++++--
4 files changed, 63 insertions(+), 53 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 1cf3d22ec74c..564e3dc42c72 100755
--- 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 af072510870d..2a807d99af71 100644
--- 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 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