[libvirt] [PATCH 0/2] Exit from virsh when disconnected due to keepalive
When the connection in virsh got disconnected due to keepalive, virsh was trying to reconnect. Adding the REASON_KEEPALIVE to the list of disconnect reasons after virsh should not reconnect (patch 1/1) was not enough because our rpc code was rewriting those and that needs to be fixed too (patch 2/2). Martin Kletzander (2): rpc: Report proper close reason virsh: Really disconnect on keepalive timeout src/rpc/virnetclient.c | 7 +++++-- tools/virsh.c | 13 +++++++++---- 2 files changed, 14 insertions(+), 6 deletions(-) -- 2.1.3
Whenever client socket was marked as closed for some reason, it could've been changed when really closing the connection. With this patch the proper reason is kept since the first time it's marked as closed. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/virnetclient.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 8657b0e..d7455b5 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -634,8 +634,11 @@ virNetClientMarkClose(virNetClientPtr client, VIR_DEBUG("client=%p, reason=%d", client, reason); if (client->sock) virNetSocketRemoveIOCallback(client->sock); - client->wantClose = true; - client->closeReason = reason; + /* Don't override reason that's already set. */ + if (!client->wantClose) { + client->wantClose = true; + client->closeReason = reason; + } } -- 2.1.3
On Sun, Nov 30, 2014 at 20:09:08 +0100, Martin Kletzander wrote:
Whenever client socket was marked as closed for some reason, it could've been changed when really closing the connection. With this patch the proper reason is kept since the first time it's marked as closed.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/virnetclient.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 8657b0e..d7455b5 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -634,8 +634,11 @@ virNetClientMarkClose(virNetClientPtr client, VIR_DEBUG("client=%p, reason=%d", client, reason); if (client->sock) virNetSocketRemoveIOCallback(client->sock); - client->wantClose = true; - client->closeReason = reason; + /* Don't override reason that's already set. */ + if (!client->wantClose) { + client->wantClose = true; + client->closeReason = reason; + } }
ACK Jirka
On Mon, Dec 01, 2014 at 10:46:38AM +0100, Jiri Denemark wrote:
On Sun, Nov 30, 2014 at 20:09:08 +0100, Martin Kletzander wrote:
Whenever client socket was marked as closed for some reason, it could've been changed when really closing the connection. With this patch the proper reason is kept since the first time it's marked as closed.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/virnetclient.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 8657b0e..d7455b5 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -634,8 +634,11 @@ virNetClientMarkClose(virNetClientPtr client, VIR_DEBUG("client=%p, reason=%d", client, reason); if (client->sock) virNetSocketRemoveIOCallback(client->sock); - client->wantClose = true; - client->closeReason = reason; + /* Don't override reason that's already set. */ + if (!client->wantClose) { + client->wantClose = true; + client->closeReason = reason; + } }
ACK
Pushed, thanks. Martin
Commit 676cb4f4e762b8682a06c6dab1f690fbcd939550 added client keepalive support for virsh. But because whenever the connection is closed, virsh tries to reconnect, unless the disconnection happened with the reason VIR_CONNECT_CLOSE_REASON_CLIENT. Because of that, virsh kept trying to reconnect even when the connection was closed due to keepalive. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1073506 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tools/virsh.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index bcfa561..98d8d18 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -327,9 +327,14 @@ static int disconnected; /* we may have been disconnected */ static void vshCatchDisconnect(virConnectPtr conn ATTRIBUTE_UNUSED, int reason, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { - if (reason != VIR_CONNECT_CLOSE_REASON_CLIENT) + vshControl *ctl = opaque; + + vshDebug(ctl, VSH_ERR_INFO, "Received disconnect with reason '%d'", reason); + + if (reason != VIR_CONNECT_CLOSE_REASON_CLIENT && + reason != VIR_CONNECT_CLOSE_REASON_KEEPALIVE) disconnected++; } @@ -407,7 +412,7 @@ vshReconnect(vshControl *ctl) vshError(ctl, "%s", _("failed to connect to the hypervisor")); } else { if (virConnectRegisterCloseCallback(ctl->conn, vshCatchDisconnect, - NULL, NULL) < 0) + ctl, NULL) < 0) vshError(ctl, "%s", _("Unable to register disconnect callback")); if (connected) vshError(ctl, "%s", _("Reconnected to the hypervisor")); @@ -484,7 +489,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) } if (virConnectRegisterCloseCallback(ctl->conn, vshCatchDisconnect, - NULL, NULL) < 0) + ctl, NULL) < 0) vshError(ctl, "%s", _("Unable to register disconnect callback")); return true; -- 2.1.3
On Sun, Nov 30, 2014 at 20:09:09 +0100, Martin Kletzander wrote:
Commit 676cb4f4e762b8682a06c6dab1f690fbcd939550 added client keepalive support for virsh. But because whenever the connection is closed, virsh tries to reconnect, unless the disconnection happened with the reason VIR_CONNECT_CLOSE_REASON_CLIENT. Because of that, virsh kept trying to reconnect even when the connection was closed due to keepalive.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1073506
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tools/virsh.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
It's nice this patch makes non-interactive virsh stop, however, it prevents interactive virsh from automatically reconnecting when a next command is issued, which is a very useful feature. Jirka
On Mon, Dec 01, 2014 at 11:05:04AM +0100, Jiri Denemark wrote:
On Sun, Nov 30, 2014 at 20:09:09 +0100, Martin Kletzander wrote:
Commit 676cb4f4e762b8682a06c6dab1f690fbcd939550 added client keepalive support for virsh. But because whenever the connection is closed, virsh tries to reconnect, unless the disconnection happened with the reason VIR_CONNECT_CLOSE_REASON_CLIENT. Because of that, virsh kept trying to reconnect even when the connection was closed due to keepalive.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1073506
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tools/virsh.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
It's nice this patch makes non-interactive virsh stop, however, it prevents interactive virsh from automatically reconnecting when a next command is issued, which is a very useful feature.
Thanks for finding that out. v2 coming up. Martin
participants (2)
-
Jiri Denemark -
Martin Kletzander