7:07 a.m.
Extract common clearing of event callbacks as remoteClientFreePrivateCallbacks.
the common function also separation including the sysident handling.
---
daemon/remote.c | 73 ++++++++++++++++++++++++++++++---------------------------
1 file changed, 39 insertions(+), 34 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index cbcb6e8..2dcec1e 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1689,6 +1689,44 @@ void remoteRelayConnectionClosedEvent(virConnectPtr conn
ATTRIBUTE_UNUSED, int r
neventCallbacks = 0; \
} while (0);
+
+static void
+remoteClientFreePrivateCallbacks(struct daemonClientPrivate *priv)
+{
+ virIdentityPtr sysident = virIdentityGetSystem();
+ virIdentitySetCurrent(sysident);
+
+ DEREG_CB(priv->conn, priv->domainEventCallbacks,
+ priv->ndomainEventCallbacks,
+ virConnectDomainEventDeregisterAny, "domain");
+ DEREG_CB(priv->conn, priv->networkEventCallbacks,
+ priv->nnetworkEventCallbacks,
+ virConnectNetworkEventDeregisterAny, "network");
+ DEREG_CB(priv->conn, priv->storageEventCallbacks,
+ priv->nstorageEventCallbacks,
+ virConnectStoragePoolEventDeregisterAny, "storage");
+ DEREG_CB(priv->conn, priv->nodeDeviceEventCallbacks,
+ priv->nnodeDeviceEventCallbacks,
+ virConnectNodeDeviceEventDeregisterAny, "node device");
+ DEREG_CB(priv->conn, priv->secretEventCallbacks,
+ priv->nsecretEventCallbacks,
+ virConnectSecretEventDeregisterAny, "secret");
+ DEREG_CB(priv->conn, priv->qemuEventCallbacks,
+ priv->nqemuEventCallbacks,
+ virConnectDomainQemuMonitorEventDeregister, "qemu monitor");
+
+ if (priv->closeRegistered) {
+ if (virConnectUnregisterCloseCallback(priv->conn,
+ remoteRelayConnectionClosedEvent) < 0)
+ VIR_WARN("unexpected close callback event deregister failure");
+ }
+
+ virIdentitySetCurrent(NULL);
+ virObjectUnref(sysident);
+}
+#undef DEREG_CB
+
+
/*
* You must hold lock for at least the client
* We don't free stuff here, merely disconnect the client's
@@ -1702,44 +1740,11 @@ void remoteClientFreeFunc(void *data)
/* Deregister event delivery callback */
if (priv->conn) {
- virIdentityPtr sysident = virIdentityGetSystem();
-
- virIdentitySetCurrent(sysident);
-
- DEREG_CB(priv->conn, priv->domainEventCallbacks,
- priv->ndomainEventCallbacks,
- virConnectDomainEventDeregisterAny, "domain");
- DEREG_CB(priv->conn, priv->networkEventCallbacks,
- priv->nnetworkEventCallbacks,
- virConnectNetworkEventDeregisterAny, "network");
- DEREG_CB(priv->conn, priv->storageEventCallbacks,
- priv->nstorageEventCallbacks,
- virConnectStoragePoolEventDeregisterAny, "storage");
- DEREG_CB(priv->conn, priv->nodeDeviceEventCallbacks,
- priv->nnodeDeviceEventCallbacks,
- virConnectNodeDeviceEventDeregisterAny, "node device");
- DEREG_CB(priv->conn, priv->secretEventCallbacks,
- priv->nsecretEventCallbacks,
- virConnectSecretEventDeregisterAny, "secret");
- DEREG_CB(priv->conn, priv->qemuEventCallbacks,
- priv->nqemuEventCallbacks,
- virConnectDomainQemuMonitorEventDeregister, "qemu monitor");
-
- if (priv->closeRegistered) {
- if (virConnectUnregisterCloseCallback(priv->conn,
- remoteRelayConnectionClosedEvent) <
0)
- VIR_WARN("unexpected close callback event deregister
failure");
- }
-
+ remoteClientFreePrivateCallbacks(priv);
virConnectClose(priv->conn);
-
- virIdentitySetCurrent(NULL);
- virObjectUnref(sysident);
}
-
VIR_FREE(priv);
}
-#undef DEREG_CB
static void remoteClientCloseFunc(virNetServerClientPtr client)
--
2.8.3
7:07 a.m.
New subject: [libvirt] [PATCH v4 2/2] remote: Move the call to remoteClientFreePrivateCallbacks from FreeFunc to CloseFunc
Still because of commit id 'fe8f1c8b' where we generate a REF for the
Register and that's transparent to the consumer (e.g. how would they
know they need to ensure that Deregister is called), thus the purpose of
this patch is to find a way to Deregister if it's determined that the
consumer hasn't by the time of the "last" REF we'd have.
This solution to this problem is to alter the processing to have the
remoteClientCloseFunc handle performing the Deregister calls instead of
the remoteClientFreeFunc because there's no way FreeFunc would be called
unless the Deregister was already called.
---
daemon/remote.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 2dcec1e..c2111ae 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1738,11 +1738,9 @@ void remoteClientFreeFunc(void *data)
{
struct daemonClientPrivate *priv = data;
- /* Deregister event delivery callback */
- if (priv->conn) {
- remoteClientFreePrivateCallbacks(priv);
+ if (priv->conn)
virConnectClose(priv->conn);
- }
+
VIR_FREE(priv);
}
@@ -1752,6 +1750,10 @@ static void remoteClientCloseFunc(virNetServerClientPtr client)
struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
daemonRemoveAllClientStreams(priv->streams);
+
+ /* Deregister event delivery callback */
+ if (priv->conn)
+ remoteClientFreePrivateCallbacks(priv);
}
--
2.8.3
8:31 a.m.
New subject: [libvirt] [PATCH v4 2/2] remote: Move the call to remoteClientFreePrivateCallbacks from FreeFunc to CloseFunc
On Mon, Nov 13, 2017 at 09:07:58PM +0800, xinhua.Cao wrote:
Still because of commit id 'fe8f1c8b' where we generate a REF
for the
Register and that's transparent to the consumer (e.g. how would they
know they need to ensure that Deregister is called), thus the purpose of
this patch is to find a way to Deregister if it's determined that the
consumer hasn't by the time of the "last" REF we'd have.
This solution to this problem is to alter the processing to have the
remoteClientCloseFunc handle performing the Deregister calls instead of
the remoteClientFreeFunc because there's no way FreeFunc would be called
unless the Deregister was already called.
Oh, this would explain the missing bit that I was probably missing. This patch
looks like what I really wanted this to be handled as, but still, would you be
able to sketch out a reproducer for the unlucky ones, like me? Thanks a lot.
---
daemon/remote.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 2dcec1e..c2111ae 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1738,11 +1738,9 @@ void remoteClientFreeFunc(void *data)
{
struct daemonClientPrivate *priv = data;
- /* Deregister event delivery callback */
- if (priv->conn) {
- remoteClientFreePrivateCallbacks(priv);
+ if (priv->conn)
virConnectClose(priv->conn);
- }
+
VIR_FREE(priv);
}
@@ -1752,6 +1750,10 @@ static void remoteClientCloseFunc(virNetServerClientPtr client)
struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
daemonRemoveAllClientStreams(priv->streams);
+
+ /* Deregister event delivery callback */
+ if (priv->conn)
+ remoteClientFreePrivateCallbacks(priv);
}
--
2.8.3
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:06 p.m.
New subject: [libvirt] [PATCH v4 2/2] remote: Move the call to remoteClientFreePrivateCallbacks from FreeFunc to CloseFunc
在 2017/11/13 22:31, Martin Kletzander 写道:
On Mon, Nov 13, 2017 at 09:07:58PM +0800, xinhua.Cao wrote:
> Still because of commit id 'fe8f1c8b' where we generate a REF for the
> Register and that's transparent to the consumer (e.g. how would they
> know they need to ensure that Deregister is called), thus the purpose of
> this patch is to find a way to Deregister if it's determined that the
> consumer hasn't by the time of the "last" REF we'd have.
> This solution to this problem is to alter the processing to have the
> remoteClientCloseFunc handle performing the Deregister calls instead of
> the remoteClientFreeFunc because there's no way FreeFunc would be called
> unless the Deregister was already called.
Oh, this would explain the missing bit that I was probably missing.
This patch
looks like what I really wanted this to be handled as, but still,
would you be
able to sketch out a reproducer for the unlucky ones, like me? Thanks
a lot.
I find this unlucky one by watch the growing of libvirt memory(top -b -d
30 -p $libvirtd_pid).
the memory always grow up when I kill my client. so I watch the client
refs then find this unlucky one.
> ---
> daemon/remote.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/daemon/remote.c b/daemon/remote.c
> index 2dcec1e..c2111ae 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -1738,11 +1738,9 @@ void remoteClientFreeFunc(void *data)
> {
> struct daemonClientPrivate *priv = data;
>
> - /* Deregister event delivery callback */
> - if (priv->conn) {
> - remoteClientFreePrivateCallbacks(priv);
> + if (priv->conn)
> virConnectClose(priv->conn);
> - }
> +
> VIR_FREE(priv);
> }
>
> @@ -1752,6 +1750,10 @@ static void
> remoteClientCloseFunc(virNetServerClientPtr client)
> struct daemonClientPrivate *priv =
> virNetServerClientGetPrivateData(client);
>
> daemonRemoveAllClientStreams(priv->streams);
> +
> + /* Deregister event delivery callback */
> + if (priv->conn)
> + remoteClientFreePrivateCallbacks(priv);
> }
>
>
> --
> 2.8.3
>
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
3:28 p.m.
New subject: [libvirt] [PATCH v4 2/2] remote: Move the call to remoteClientFreePrivateCallbacks from FreeFunc to CloseFunc
On 11/13/2017 08:07 AM, xinhua.Cao wrote:
Still because of commit id 'fe8f1c8b' where we generate a REF
for the
Register and that's transparent to the consumer (e.g. how would they
know they need to ensure that Deregister is called), thus the purpose of
this patch is to find a way to Deregister if it's determined that the
consumer hasn't by the time of the "last" REF we'd have.
This solution to this problem is to alter the processing to have the
remoteClientCloseFunc handle performing the Deregister calls instead of
the remoteClientFreeFunc because there's no way FreeFunc would be called
unless the Deregister was already called.
---
daemon/remote.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
NB: Prior to pushing I want to give enough time for anyone else to pipe
in over the implementation. I know Martin had expressed some interest -
so it's essentially time to allow him to register any objections... I
figure at least 24 hours should be good enough, but it could be
Wednesday too...
3:28 p.m.
On 11/13/2017 08:07 AM, xinhua.Cao wrote:
Extract common clearing of event callbacks as
remoteClientFreePrivateCallbacks.
the common function also separation including the sysident handling.
---
daemon/remote.c | 73 ++++++++++++++++++++++++++++++---------------------------
1 file changed, 39 insertions(+), 34 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
2559
days inactive
2566
days old
5 comments
3 participants
participants (3)
-
John Ferlan -
Martin Kletzander -
xinhua.Cao