[libvirt] [PATCH v2 0/2] Augment debug messages.

This is a v2 of the previous set of debug enhancements posted at : https://www.redhat.com/archives/libvir-list/2017-March/msg00959.html Changelog: --------- 1) Patch 1/3 : virNetSocketGetFD() is reverted to its old state, and only new DEBUG messages are introduced. 2) Patch 2/3 : Dropped, per list suggestions. 3) Patch 3/3 : Same as v1 Prerna Saxena (2): Debug: Add DEBUG messages for when a client has opened/closed connection. Debug: Report VM errors more concisely. src/qemu/qemu_process.c | 4 ++-- src/rpc/virnetserverclient.c | 10 +++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) -- 1.8.1.2

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

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.

Current logs: error : qemuProcessFindDomainDiskByAlias:411 : internal error: no disk found with alias ide0-0-0 There is no way to find which VM was seeing this error. Makes debugging very hard, and the message itself is no good. Signed-off-by: Prerna Saxena <saxenap.ltc@gmail.com> --- src/qemu/qemu_process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e450d06..0a052e8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -367,8 +367,8 @@ qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm, } virReportError(VIR_ERR_INTERNAL_ERROR, - _("no disk found with alias %s"), - alias); + _("VM %s: no disk found with alias %s"), + vm->def->name, alias); return NULL; } -- 1.8.1.2

Ping ! Can someone take a look at this pls ? It is a small changeset.. On Mon, Apr 3, 2017 at 2:05 PM, Prerna Saxena <saxenap.ltc@gmail.com> wrote:
This is a v2 of the previous set of debug enhancements posted at : https://www.redhat.com/archives/libvir-list/2017- March/msg00959.html
Changelog: --------- 1) Patch 1/3 : virNetSocketGetFD() is reverted to its old state, and only new DEBUG messages are introduced. 2) Patch 2/3 : Dropped, per list suggestions. 3) Patch 3/3 : Same as v1
Prerna Saxena (2): Debug: Add DEBUG messages for when a client has opened/closed connection. Debug: Report VM errors more concisely.
src/qemu/qemu_process.c | 4 ++-- src/rpc/virnetserverclient.c | 10 +++++++++- 2 files changed, 11 insertions(+), 3 deletions(-)
-- 1.8.1.2
participants (3)
-
Peter Krempa
-
Prerna
-
Prerna Saxena