Re: [libvirt] [PATCH v3 4/5] Update remote driver to support the connection close callbacks

On Fri, Jul 27, 2012 at 11:39:55 +0100, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > Update the remote driver to use the virNetClient close callback > to trigger the virConnectPtr close callbacks > > Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; > --- > src/datatypes.h | 2 ++ > src/libvirt.c | 3 ++- > src/remote/remote_driver.c | 31 +++++++++++++++++++++++++++++++ > 3 files changed, 35 insertions(+), 1 deletion(-) > > diff --git a/src/datatypes.h b/src/datatypes.h > index 8ac9171..a303cdc 100644 > --- a/src/datatypes.h > +++ b/src/datatypes.h > @@ -191,6 +191,8 @@ struct _virConnect { > virConnectCloseFunc closeCallback; > void *closeOpaque; > virFreeCallback closeFreeCallback; > + bool closeDispatch; > + unsigned closeUnregisterCount; > > int refs; /* reference count */ > }; > diff --git a/src/libvirt.c b/src/libvirt.c > index 160ace7..d352cfd 100644 > --- a/src/libvirt.c > +++ b/src/libvirt.c > @@ -18711,7 +18711,8 @@ int virConnectUnregisterCloseCallback(virConnectPtr conn, > } > > conn->closeCallback = NULL; > - if (conn->closeFreeCallback) > + conn->closeUnregisterCount++; > + if (!conn->closeDispatch && conn->closeFreeCallback) > conn->closeFreeCallback(conn->closeOpaque); > conn->closeFreeCallback = NULL; > conn->closeOpaque = NULL; > diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c > index a69e69a..9ac27b2 100644 > --- a/src/remote/remote_driver.c > +++ b/src/remote/remote_driver.c > @@ -320,6 +320,32 @@ enum virDrvOpenRemoteFlags { > }; > > > +static void remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED, > + int reason, > + void *opaque) > +{ > + virConnectPtr conn = opaque; > + > + virMutexLock(&conn->lock); > + if (conn->closeCallback) { > + virConnectCloseFunc closeCallback = conn->closeCallback; > + void *closeOpaque = conn->closeOpaque; > + virFreeCallback closeFreeCallback = conn->closeFreeCallback; > + unsigned closeUnregisterCount = conn->closeUnregisterCount; > + > + VIR_DEBUG("Triggering connection close callback %p reason=%d", > + conn->closeCallback, reason); > + conn->closeDispatch = true; > + virMutexUnlock(&conn->lock); > + closeCallback(conn, reason, closeOpaque); > + virMutexLock(&conn->lock); > + conn->closeDispatch = false; > + if (conn->closeUnregisterCount != closeUnregisterCount) > + closeFreeCallback(closeOpaque); > + } > + virMutexUnlock(&conn->lock); > +} > + > /* > * URIs that this driver needs to handle: > * Looks good. However, do we need to protect against the following scenario: - remoteClientCloseFunc locks conn, remembers stuff, unlocks conn - virConnectUnregisterCloseCallback - virConnectRegisterCloseCallback - virConnectUnregisterCloseCallback - remoteClientCloseFunc finishes Now the second registered callback won't be unregistered. It doesn't seem like something that could happen in the real world, though.