12:15 p.m.
From: "Daniel P. Berrange" <berrange(a)redhat.com>
NB, previous patch was borked due to bad rebase
Every active stream results in a reference being held on the
virNetServerClientPtr object. This meant that if a client quit
with any streams active, although all I/O was stopped the
virNetServerClientPtr object would leak. This causes libvirtd
to leak any file handles associated with open streams when a
client quit
To fix this, when we call virNetServerClientClose there is a
callback invoked which lets the daemon release the streams
and thus the extra references
* daemon/remote.c: Add a hook to close all streams
* daemon/stream.c, daemon/stream.h: Add API for releasing
all streams
* src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h:
Allow registration of a hook to trigger when closing client
---
daemon/remote.c | 11 ++++++++++-
daemon/stream.c | 38 ++++++++++++++++++++++++++++++++------
daemon/stream.h | 3 +++
src/rpc/virnetserverclient.c | 21 +++++++++++++++++++++
src/rpc/virnetserverclient.h | 5 +++++
5 files changed, 71 insertions(+), 7 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 939044c..0f088c6 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -439,6 +439,13 @@ static void remoteClientFreeFunc(void *data)
}
+static void remoteClientCloseFunc(virNetServerClientPtr client)
+{
+ struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
+
+ daemonRemoveAllClientStreams(priv->streams);
+}
+
int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED,
virNetServerClientPtr client)
@@ -460,7 +467,9 @@ int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED,
for (i = 0 ; i < VIR_DOMAIN_EVENT_ID_LAST ; i++)
priv->domainEventCallbackID[i] = -1;
- virNetServerClientSetPrivateData(client, priv, remoteClientFreeFunc);
+ virNetServerClientSetPrivateData(client, priv,
+ remoteClientFreeFunc);
+ virNetServerClientSetCloseHook(client, remoteClientCloseFunc);
return 0;
}
diff --git a/daemon/stream.c b/daemon/stream.c
index 4a8f1ee..c6865a8 100644
--- a/daemon/stream.c
+++ b/daemon/stream.c
@@ -334,13 +334,17 @@ int daemonFreeClientStream(virNetServerClientPtr client,
msg = stream->rx;
while (msg) {
virNetMessagePtr tmp = msg->next;
- /* Send a dummy reply to free up 'msg' & unblock client rx */
- memset(msg, 0, sizeof(*msg));
- msg->header.type = VIR_NET_REPLY;
- if (virNetServerClientSendMessage(client, msg) < 0) {
- virNetServerClientImmediateClose(client);
+ if (client) {
+ /* Send a dummy reply to free up 'msg' & unblock client rx */
+ memset(msg, 0, sizeof(*msg));
+ msg->header.type = VIR_NET_REPLY;
+ if (virNetServerClientSendMessage(client, msg) < 0) {
+ virNetServerClientImmediateClose(client);
+ virNetMessageFree(msg);
+ ret = -1;
+ }
+ } else {
virNetMessageFree(msg);
- ret = -1;
}
msg = tmp;
}
@@ -441,6 +445,28 @@ daemonRemoveClientStream(virNetServerClientPtr client,
}
+void
+daemonRemoveAllClientStreams(daemonClientStream *stream)
+{
+ daemonClientStream *tmp;
+
+ VIR_DEBUG("stream=%p", stream);
+
+ while (stream) {
+ tmp = stream->next;
+
+ if (!stream->closed) {
+ virStreamEventRemoveCallback(stream->st);
+ virStreamAbort(stream->st);
+ }
+
+ daemonFreeClientStream(NULL, stream);
+
+ VIR_DEBUG("next stream=%p", stream);
+ stream = tmp;
+ }
+}
+
/*
* Returns:
* -1 if fatal error occurred
diff --git a/daemon/stream.h b/daemon/stream.h
index 626ae60..7c2d8dc 100644
--- a/daemon/stream.h
+++ b/daemon/stream.h
@@ -45,4 +45,7 @@ int
daemonRemoveClientStream(virNetServerClientPtr client,
daemonClientStream *stream);
+void
+daemonRemoveAllClientStreams(daemonClientStream *stream);
+
#endif /* __LIBVIRTD_STREAM_H__ */
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index e246fa5..a73b06d 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -97,6 +97,7 @@ struct _virNetServerClient
void *privateData;
virNetServerClientFreeFunc privateDataFreeFunc;
+ virNetServerClientCloseFunc privateDataCloseFunc;
};
@@ -492,6 +493,15 @@ void *virNetServerClientGetPrivateData(virNetServerClientPtr client)
}
+void virNetServerClientSetCloseHook(virNetServerClientPtr client,
+ virNetServerClientCloseFunc cf)
+{
+ virNetServerClientLock(client);
+ client->privateDataCloseFunc = cf;
+ virNetServerClientUnlock(client);
+}
+
+
void virNetServerClientSetDispatcher(virNetServerClientPtr client,
virNetServerClientDispatchFunc func,
void *opaque)
@@ -560,6 +570,8 @@ void virNetServerClientFree(virNetServerClientPtr client)
*/
void virNetServerClientClose(virNetServerClientPtr client)
{
+ virNetServerClientCloseFunc cf;
+
virNetServerClientLock(client);
VIR_DEBUG("client=%p refs=%d", client, client->refs);
if (!client->sock) {
@@ -567,6 +579,15 @@ void virNetServerClientClose(virNetServerClientPtr client)
return;
}
+ if (client->privateDataCloseFunc) {
+ cf = client->privateDataCloseFunc;
+ client->refs++;
+ virNetServerClientUnlock(client);
+ (cf)(client);
+ virNetServerClientLock(client);
+ client->refs--;
+ }
+
/* Do now, even though we don't close the socket
* until end, to ensure we don't get invoked
* again due to tls shutdown */
diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h
index 3d2e1fb..bedb179 100644
--- a/src/rpc/virnetserverclient.h
+++ b/src/rpc/virnetserverclient.h
@@ -82,6 +82,11 @@ void virNetServerClientSetPrivateData(virNetServerClientPtr client,
virNetServerClientFreeFunc ff);
void *virNetServerClientGetPrivateData(virNetServerClientPtr client);
+typedef void (*virNetServerClientCloseFunc)(virNetServerClientPtr client);
+
+void virNetServerClientSetCloseHook(virNetServerClientPtr client,
+ virNetServerClientCloseFunc cf);
+
void virNetServerClientSetDispatcher(virNetServerClientPtr client,
virNetServerClientDispatchFunc func,
void *opaque);
--
1.7.6