I applied this patch and tried it out. It appears to work as advertised,
which is useful. Would be even better if there was some higher level
handling to automatically retry the commands that fail due to sigpipe.
Beyond that, I noticed a few typos, but don't have enough expertise
about signals to know the definitive answer about resetting the signal
handler (multiple times does work properly for me, so at least on Linux
it seems to be unnecessary).
On 03/05/2010 05:11 AM, Daniel Veillard wrote:
This is a usability issue for virsh in case of disconnections,
for example if the remote libvirtd is restarted:
https://bugzilla.redhat.com/show_bug.cgi?id=526656
the patch catch those and tries to automatically reconnect instead
of virsh exitting. The typical interaction with this patch is that
s/exitting/exiting/
the command fails, but virsh automatically reconnects instead of
exitting, but it won't try to restart the failed command (since this
s/exitting/exiting/
could have significant side effects). Example of such interraction:
s/interraction/interaction/
(your key repeat setting is too quick ;-)
-------------------------------------------------------
virsh # list --all
Id Name State
----------------------------------
- RHEL-5.4-64 shut off
- WinXP shut off
virsh # list --all
error: Failed to list active domains
error: cannot send data: Broken pipe
error: Reconnected to the hypervisor
virsh # list --all
Id Name State
----------------------------------
- RHEL-5.4-64 shut off
- WinXP shut off
virsh #
-------------------------------------------------------
The only thing I'm unsure is if the signal handler should be reset
once it was received once. I don't in this patch and that seems to
work fine, but I somehow remember the fact that in some circumstances
a signal handler needs to be rearmed when received once. As is this
seems to work fine with SIGPIPE and linux.
Make virsh reconnect when loosing connection
s/loosing/losing/
Right now when the connected libvirtd restarts virsh gets a SIGPIPE
and dies, this change the behaviour to try to reconnect if the
signal was received or command error indicated a connection or RPC
failure. Note that the failing command is not restarted.
* tools/virsh.c: catch SIGPIPE signals as well as connection related
failures, add some automatic reconnection code and appropriate error
messages.
diff --git a/tools/virsh.c b/tools/virsh.c
index 65487ed..2ec9cfc 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -30,6 +30,7 @@
#include<errno.h>
#include<sys/stat.h>
#include<inttypes.h>
+#include<signal.h>
#include<libxml/parser.h>
#include<libxml/tree.h>
@@ -397,6 +398,58 @@ out:
last_error = NULL;
}
+/*
+ * Detection of disconnections and automatic reconnection support
+ */
+static int disconnected = 0; /* we may have been disconnected */
+
+/*
+ * vshCatchDisconnect:
+ *
+ * We get here when a SIGPIPE is being raised, we can't do much in the
+ * handler, just save the fact it was raised
+ */
+static void vshCatchDisconnect(int sig, siginfo_t * siginfo,
+ void* context ATTRIBUTE_UNUSED) {
+ if ((sig == SIGPIPE) || (siginfo->si_signo == SIGPIPE))
+ disconnected++;
+}
+
+/*
+ * vshSetupSignals:
+ *
+ * Catch SIGPIPE signals which may arise when desconnection from libvirtd occurs */
s/desconnection/disconnection/
+static int
+vshSetupSignals(void) {
+ struct sigaction sig_action;
+
+ sig_action.sa_sigaction = vshCatchDisconnect;
+ sig_action.sa_flags = SA_SIGINFO;
+ sigemptyset(&sig_action.sa_mask);
+
+ sigaction(SIGPIPE,&sig_action, NULL);
+}
+
+/*
+ * vshReconnect:
+ *
+ * Reconnect after an
+ *
+ */
+static int
+vshReconnect(vshControl *ctl) {
+ if (ctl->conn != NULL)
+ virConnectClose(ctl->conn);
+
+ ctl->conn = virConnectOpenAuth(ctl->name,
+ virConnectAuthPtrDefault,
+ ctl->readonly ? VIR_CONNECT_RO : 0);
+ if (!ctl->conn)
+ vshError(ctl, "%s", _("Failed to reconnect to the hypervisor"));
+ else
+ vshError(ctl, "%s", _("Reconnected to the hypervisor"));
+ disconnected = 0;
+}
/* ---------------
* Commands
@@ -8332,6 +8385,9 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd)
while (cmd) {
struct timeval before, after;
+ if ((ctl->conn == NULL) || (disconnected != 0))
+ vshReconnect(ctl);
+
if (ctl->timing)
GETTIMEOFDAY(&before);
@@ -8343,6 +8399,17 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd)
if (ret == FALSE)
virshReportError(ctl);
+ /* try to catch automatically disconnections */
s/catch automatically/automatically catch/
+ if ((ret == FALSE)&&
+ ((disconnected != 0) ||
+ ((last_error != NULL)&&
+ (((last_error->code == VIR_ERR_SYSTEM_ERROR)&&
+ (last_error->domain == 13)) ||
13 == VIR_FROM_REMOTE, right? Why use the literal?
+ (last_error->code == VIR_ERR_RPC) ||
+ (last_error->code == VIR_ERR_NO_CONNECT) ||
+ (last_error->code == VIR_ERR_INVALID_CONN)))))
+ vshReconnect(ctl);
+
if (STREQ(cmd->def->name, "quit")) /* hack ... */
return ret;
@@ -8673,9 +8740,11 @@ vshError(vshControl *ctl, const char *format, ...)
{
va_list ap;
- va_start(ap, format);
- vshOutputLogFile(ctl, VSH_ERR_ERROR, format, ap);
- va_end(ap);
+ if (ctl != NULL) {
+ va_start(ap, format);
+ vshOutputLogFile(ctl, VSH_ERR_ERROR, format, ap);
+ va_end(ap);
+ }
fputs(_("error: "), stderr);
@@ -8751,6 +8820,9 @@ vshInit(vshControl *ctl)
/* set up the library error handler */
virSetErrorFunc(NULL, virshErrorHandler);
+ /* set up the signals handlers to catch disconnections */
+ vshSetupSignals();
+
ctl->conn = virConnectOpenAuth(ctl->name,
virConnectAuthPtrDefault,
ctl->readonly ? VIR_CONNECT_RO : 0);
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list