[libvirt] [PATCH v2 1/2] Debug: Add DEBUG messages for when a client has opened/closed connection.

Prerna Saxena posted 2 patches 8 years, 10 months ago
[libvirt] [PATCH v2 1/2] Debug: Add DEBUG messages for when a client has opened/closed connection.
Posted by Prerna Saxena 8 years, 10 months ago
While tracing connections from a remote client, it helps to keep track
of the connection lifecycle. Messages such as the following :

error : virNetSocketReadWire:1574 : End of file while reading data: Input/output error

are rather unhelpful. They do not indicate if the client had earlier asked for
connection closure via libvirt API.
This patch introduces messages to annotate when a client connected/disconnected.

Signed-off-by: Prerna Saxena <saxenap.ltc@gmail.com>
---
 src/rpc/virnetserverclient.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 85857bc..24c9c33 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -964,8 +964,11 @@ void virNetServerClientClose(virNetServerClientPtr client)
     virNetServerClientCloseFunc cf;
     virKeepAlivePtr ka;
 
+    VIR_DEBUG("Free'ing up resources for client=%p sock=%d", client,
+               virNetServerClientGetFD(client));
+
     virObjectLock(client);
-    VIR_DEBUG("client=%p", client);
+
     if (!client->sock) {
         virObjectUnlock(client);
         return;
@@ -1039,10 +1042,14 @@ void virNetServerClientDelayedClose(virNetServerClientPtr client)
     virObjectLock(client);
     client->delayedClose = true;
     virObjectUnlock(client);
+    VIR_DEBUG("Client=%p sock=%d requested closure of connection.",
+              client, virNetServerClientGetFD(client));
 }
 
 void virNetServerClientImmediateClose(virNetServerClientPtr client)
 {
+    VIR_DEBUG("Client %p sock %d closed the connection", client,
+                virNetServerClientGetFD(client));
     virObjectLock(client);
     client->wantClose = true;
     virObjectUnlock(client);
@@ -1151,6 +1158,7 @@ static void virNetServerClientDispatchRead(virNetServerClientPtr client)
     if (client->rx->nfds == 0) {
         if (virNetServerClientRead(client) < 0) {
             client->wantClose = true;
+            VIR_WARN("Client=%p sock=%p closed connection", client, client->sock);
             return; /* Error */
         }
     }
-- 
1.8.1.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] Debug: Add DEBUG messages for when a client has opened/closed connection.
Posted by Peter Krempa 8 years, 9 months ago
On Mon, Apr 03, 2017 at 01:35:53 -0700, Prerna Saxena wrote:
> While tracing connections from a remote client, it helps to keep track
> of the connection lifecycle. Messages such as the following :
> 
> error : virNetSocketReadWire:1574 : End of file while reading data: Input/output error
> 
> are rather unhelpful. They do not indicate if the client had earlier asked for
> connection closure via libvirt API.
> This patch introduces messages to annotate when a client connected/disconnected.
> 
> Signed-off-by: Prerna Saxena <saxenap.ltc@gmail.com>
> ---
>  src/rpc/virnetserverclient.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
> index 85857bc..24c9c33 100644
> --- a/src/rpc/virnetserverclient.c
> +++ b/src/rpc/virnetserverclient.c
> @@ -964,8 +964,11 @@ void virNetServerClientClose(virNetServerClientPtr client)
>      virNetServerClientCloseFunc cf;
>      virKeepAlivePtr ka;
>  
> +    VIR_DEBUG("Free'ing up resources for client=%p sock=%d", client,
> +               virNetServerClientGetFD(client));

Locking the client object just for the sole purpose of getting the file
descriptor for the debug message and then ulocking it is in my opinion
not justifiable.

> +
>      virObjectLock(client);
> -    VIR_DEBUG("client=%p", client);
> +
>      if (!client->sock) {
>          virObjectUnlock(client);
>          return;
> @@ -1039,10 +1042,14 @@ void virNetServerClientDelayedClose(virNetServerClientPtr client)
>      virObjectLock(client);
>      client->delayedClose = true;
>      virObjectUnlock(client);
> +    VIR_DEBUG("Client=%p sock=%d requested closure of connection.",
> +              client, virNetServerClientGetFD(client));

Same as above. You unlock the object and then relock it just to get the
fd number to the error message. Does not seem worth to me.

>  }
>  
>  void virNetServerClientImmediateClose(virNetServerClientPtr client)
>  {
> +    VIR_DEBUG("Client %p sock %d closed the connection", client,
> +                virNetServerClientGetFD(client));

Same issue.

>      virObjectLock(client);
>      client->wantClose = true;
>      virObjectUnlock(client);
> @@ -1151,6 +1158,7 @@ static void virNetServerClientDispatchRead(virNetServerClientPtr client)
>      if (client->rx->nfds == 0) {
>          if (virNetServerClientRead(client) < 0) {
>              client->wantClose = true;
> +            VIR_WARN("Client=%p sock=%p closed connection", client, client->sock);

There are multiple reasons where virNetServerClientRead, the warning
could be misleading here.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list