From 24dbb69f214c8eae01241bee91ccd07027465628 Mon Sep 17 00:00:00 2001 From: Nikolay Shirokovskiy Date: Wed, 17 Feb 2016 15:14:54 +0300 Subject: [PATCH] factor out virConnectCloseCallbackDataPtr methods Make register and unregister functions return void because we can check the state of callback object beforehand via virConnectCloseCallbackDataGetCallback. This can be done without race conditions if we use higher level locks for registering and unregistering. The fact they return void simplifies task of consistent registering/unregistering. Signed-off-by: Nikolay Shirokovskiy --- src/datatypes.c | 80 ++++++++++++++++++++++++++++++++++++++ src/datatypes.h | 13 +++++++ src/libvirt-host.c | 29 +++----------- src/remote/remote_driver.c | 17 +------- 4 files changed, 100 insertions(+), 39 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index da6ec371f4..3e1d809b40 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -184,6 +184,86 @@ virConnectCloseCallbackDataDispose(void *obj) virObjectUnlock(cb); } +void virConnectCloseCallbackDataRegister(virConnectCloseCallbackDataPtr close, + virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb) +{ + virObjectLock(close); + + if (close->callback != NULL) { + VIR_WARN("Attempt to register callback on armed" + " close callback object %p", close); + goto cleanup; + return; + } + + close->conn = conn; + virObjectRef(close->conn); + close->callback = cb; + close->opaque = opaque; + close->freeCallback = freecb; + + cleanup: + + virObjectUnlock(close); +} + +void virConnectCloseCallbackDataUnregister(virConnectCloseCallbackDataPtr close, + virConnectCloseFunc cb) +{ + virObjectLock(close); + + if (close->callback != cb) { + VIR_WARN("Attempt to unregister different callback on " + " close callback object %p", close); + goto cleanup; + } + + close->callback = NULL; + if (close->freeCallback) + close->freeCallback(close->opaque); + close->freeCallback = NULL; + virObjectUnref(close->conn); + + cleanup: + + virObjectUnlock(close); +} + +void virConnectCloseCallbackDataCall(virConnectCloseCallbackDataPtr close, + int reason) +{ + virObjectLock(close); + + if (!close->callback) + goto exit; + + VIR_DEBUG("Triggering connection close callback %p reason=%d, opaque=%p", + close->callback, reason, close->opaque); + close->callback(close->conn, reason, close->opaque); + + if (close->freeCallback) + close->freeCallback(close->opaque); + close->callback = NULL; + close->freeCallback = NULL; + + exit: + virObjectUnlock(close); +} + +virConnectCloseFunc +virConnectCloseCallbackDataGetCallback(virConnectCloseCallbackDataPtr close) +{ + virConnectCloseFunc cb; + + virObjectLock(close); + cb = close->callback; + virObjectUnlock(close); + + return cb; +} /** * virGetDomain: diff --git a/src/datatypes.h b/src/datatypes.h index 31c636ca6c..238317a9e3 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -639,4 +639,17 @@ virAdmConnectPtr virAdmConnectNew(void); virAdmServerPtr virAdmGetServer(virAdmConnectPtr conn, const char *name); + +void virConnectCloseCallbackDataRegister(virConnectCloseCallbackDataPtr close, + virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb); +void virConnectCloseCallbackDataUnregister(virConnectCloseCallbackDataPtr close, + virConnectCloseFunc cb); +void virConnectCloseCallbackDataCall(virConnectCloseCallbackDataPtr close, + int reason); +virConnectCloseFunc +virConnectCloseCallbackDataGetCallback(virConnectCloseCallbackDataPtr close); + #endif /* __VIR_DATATYPES_H__ */ diff --git a/src/libvirt-host.c b/src/libvirt-host.c index 9c88426ec4..ced6a549a2 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c @@ -1216,35 +1216,25 @@ virConnectRegisterCloseCallback(virConnectPtr conn, virResetLastError(); virCheckConnectReturn(conn, -1); - - virObjectRef(conn); - virObjectLock(conn); - virObjectLock(conn->closeCallback); virCheckNonNullArgGoto(cb, error); - if (conn->closeCallback->callback) { + if (virConnectCloseCallbackDataGetCallback(conn->closeCallback) != NULL) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("A close callback is already registered")); goto error; } - conn->closeCallback->conn = conn; - conn->closeCallback->callback = cb; - conn->closeCallback->opaque = opaque; - conn->closeCallback->freeCallback = freecb; + virConnectCloseCallbackDataRegister(conn->closeCallback, conn, cb, + opaque, freecb); - virObjectUnlock(conn->closeCallback); virObjectUnlock(conn); - return 0; error: - virObjectUnlock(conn->closeCallback); virObjectUnlock(conn); virDispatchError(conn); - virObjectUnref(conn); return -1; } @@ -1271,31 +1261,22 @@ virConnectUnregisterCloseCallback(virConnectPtr conn, virResetLastError(); virCheckConnectReturn(conn, -1); - virObjectLock(conn); - virObjectLock(conn->closeCallback); virCheckNonNullArgGoto(cb, error); - if (conn->closeCallback->callback != cb) { + if (virConnectCloseCallbackDataGetCallback(conn->closeCallback) != cb) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("A different callback was requested")); goto error; } - conn->closeCallback->callback = NULL; - if (conn->closeCallback->freeCallback) - conn->closeCallback->freeCallback(conn->closeCallback->opaque); - conn->closeCallback->freeCallback = NULL; + virConnectCloseCallbackDataUnregister(conn->closeCallback, cb); - virObjectUnlock(conn->closeCallback); virObjectUnlock(conn); - virObjectUnref(conn); - return 0; error: - virObjectUnlock(conn->closeCallback); virObjectUnlock(conn); virDispatchError(conn); return -1; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7cf61cf562..e0cf33fdc3 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -535,21 +535,8 @@ remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED, int reason, void *opaque) { - virConnectCloseCallbackDataPtr cbdata = opaque; - - virObjectLock(cbdata); - - if (cbdata->callback) { - VIR_DEBUG("Triggering connection close callback %p reason=%d, opaque=%p", - cbdata->callback, reason, cbdata->opaque); - cbdata->callback(cbdata->conn, reason, cbdata->opaque); - - if (cbdata->freeCallback) - cbdata->freeCallback(cbdata->opaque); - cbdata->callback = NULL; - cbdata->freeCallback = NULL; - } - virObjectUnlock(cbdata); + virConnectCloseCallbackDataCall((virConnectCloseCallbackDataPtr)opaque, + reason); } /* helper macro to ease extraction of arguments from the URI */