On 09/17/2015 08:23 AM, Jiri Denemark wrote:
After my "client rpc: Report proper error for keepalive disconnections" patch, virsh would no long print a warning when it closes a connection to a daemon after a keepalive timeout. Although the warning
virsh # 2015-09-15 10:59:26.729+0000: 642080: info : libvirt version: 1.2.19 2015-09-15 10:59:26.729+0000: 642080: warning : virKeepAliveTimerInternal:143 : No response from client 0x7efdc0a46730 after 1 keepalive messages in 2 seconds
was pretty ugly, it was still useful. This patch brings the useful part back while making it much nicer:
virsh # error: Disconnected from qemu:///system due to keepalive timeout
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/virsh.c | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-)
Patch series seems reasonable to me, except for one minor thing.... Found by my coverity checker... Naturally
diff --git a/tools/virsh.c b/tools/virsh.c index bb12dec..23436ea 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -95,12 +95,41 @@ static int disconnected; /* we may have been disconnected */ * handler, just save the fact it was raised. */ static void -virshCatchDisconnect(virConnectPtr conn ATTRIBUTE_UNUSED, +virshCatchDisconnect(virConnectPtr conn, int reason, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { - if (reason != VIR_CONNECT_CLOSE_REASON_CLIENT) + if (reason != VIR_CONNECT_CLOSE_REASON_CLIENT) {
[1]
+ vshControl *ctl = opaque; + const char *str = "unknown reason"; + virErrorPtr error; + char *uri; + + error = virSaveLastError(); + uri = virConnectGetURI(conn); + + switch ((virConnectCloseReason) reason) { + case VIR_CONNECT_CLOSE_REASON_ERROR: + str = N_("Disconnected from %s due to I/O error"); + break; + case VIR_CONNECT_CLOSE_REASON_EOF: + str = N_("Disconnected from %s due to end of file"); + break; + case VIR_CONNECT_CLOSE_REASON_KEEPALIVE: + str = N_("Disconnected from %s due to keepalive timeout"); + break; + case VIR_CONNECT_CLOSE_REASON_CLIENT:
[1] ^^^ Coverity complains about DEADCODE Other than setting str = NULL and testing for it later, I'm not exactly sure of the 'best' course of action. e.g., if (str) { vshError(); disconnected++; } I think if this is handled, then the series is ACK-able. John
+ case VIR_CONNECT_CLOSE_REASON_LAST: + break; + } + vshError(ctl, _(str), NULLSTR(uri)); + + if (error) { + virSetError(error); + virFreeError(error); + } disconnected++; + } }
/* Main Function which should be used for connecting.