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 <nshirokovskiy@virtuozzo.com> --- src/datatypes.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++ src/datatypes.h | 12 +++++++ src/libvirt-host.c | 29 +++-------------- src/remote/remote_driver.c | 17 ++-------- 4 files changed, 99 insertions(+), 39 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index c832d80..030a0d2 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -180,6 +180,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 1b1777d..2ee77cd 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -601,4 +601,16 @@ virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain, virAdmConnectPtr virAdmConnectNew(void); +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 9c88426..ced6a54 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 7cf61cf..e0cf33f 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 */ -- 1.8.3.1

We have reference to connection object in virConnectCloseCallbackData object thus we have to refcount it. Obviously we have problems in dispose and call functions. Let's fix it. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/datatypes.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/datatypes.c b/src/datatypes.c index 030a0d2..a4ee2b8 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -176,6 +176,7 @@ virConnectCloseCallbackDataDispose(void *obj) if (cb->freeCallback) cb->freeCallback(cb->opaque); + virObjectUnref(cb->conn); virObjectUnlock(cb); } @@ -222,6 +223,7 @@ void virConnectCloseCallbackDataUnregister(virConnectCloseCallbackDataPtr close, close->freeCallback(close->opaque); close->freeCallback = NULL; virObjectUnref(close->conn); + close->conn = NULL; cleanup: @@ -244,6 +246,8 @@ void virConnectCloseCallbackDataCall(virConnectCloseCallbackDataPtr close, close->freeCallback(close->opaque); close->callback = NULL; close->freeCallback = NULL; + virObjectUnref(close->conn); + close->conn = NULL; exit: virObjectUnlock(close); -- 1.8.3.1

If connect close is fired then following unregister will fail as we set callback to NULL and thus callback equality checking will fail. Callback is set to NULL to make it fired only one time probabaly. Instead lets use connection equality to NULL to check if callback is already fired. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/datatypes.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index a4ee2b8..e115a23 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -235,7 +235,7 @@ void virConnectCloseCallbackDataCall(virConnectCloseCallbackDataPtr close, { virObjectLock(close); - if (!close->callback) + if (!close->conn) goto exit; VIR_DEBUG("Triggering connection close callback %p reason=%d, opaque=%p", @@ -244,7 +244,6 @@ void virConnectCloseCallbackDataCall(virConnectCloseCallbackDataPtr close, if (close->freeCallback) close->freeCallback(close->opaque); - close->callback = NULL; close->freeCallback = NULL; virObjectUnref(close->conn); close->conn = NULL; -- 1.8.3.1

--- src/datatypes.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index e115a23..1b95b21 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -161,6 +161,18 @@ virConnectDispose(void *obj) } +static +void virConnectCloseCallbackDataReset(virConnectCloseCallbackDataPtr close) +{ + if (close->freeCallback) + close->freeCallback(close->opaque); + + close->freeCallback = NULL; + close->opaque = NULL; + virObjectUnref(close->conn); + close->conn = NULL; +} + /** * virConnectCloseCallbackDataDispose: * @obj: the close callback data to release @@ -174,9 +186,7 @@ virConnectCloseCallbackDataDispose(void *obj) virObjectLock(cb); - if (cb->freeCallback) - cb->freeCallback(cb->opaque); - virObjectUnref(cb->conn); + virConnectCloseCallbackDataReset(cb); virObjectUnlock(cb); } @@ -218,12 +228,8 @@ void virConnectCloseCallbackDataUnregister(virConnectCloseCallbackDataPtr close, goto cleanup; } + virConnectCloseCallbackDataReset(close); close->callback = NULL; - if (close->freeCallback) - close->freeCallback(close->opaque); - close->freeCallback = NULL; - virObjectUnref(close->conn); - close->conn = NULL; cleanup: @@ -242,11 +248,7 @@ void virConnectCloseCallbackDataCall(virConnectCloseCallbackDataPtr close, close->callback, reason, close->opaque); close->callback(close->conn, reason, close->opaque); - if (close->freeCallback) - close->freeCallback(close->opaque); - close->freeCallback = NULL; - virObjectUnref(close->conn); - close->conn = NULL; + virConnectCloseCallbackDataReset(close); exit: virObjectUnlock(close); -- 1.8.3.1

closeCallback pointer is immutable (set on connection object creation) and self-locking. --- src/libvirt-host.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/libvirt-host.c b/src/libvirt-host.c index ced6a54..b0597ee 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c @@ -1214,10 +1214,7 @@ virConnectRegisterCloseCallback(virConnectPtr conn, VIR_DEBUG("conn=%p", conn); virResetLastError(); - virCheckConnectReturn(conn, -1); - virObjectLock(conn); - virCheckNonNullArgGoto(cb, error); if (virConnectCloseCallbackDataGetCallback(conn->closeCallback) != NULL) { @@ -1229,11 +1226,9 @@ virConnectRegisterCloseCallback(virConnectPtr conn, virConnectCloseCallbackDataRegister(conn->closeCallback, conn, cb, opaque, freecb); - virObjectUnlock(conn); return 0; error: - virObjectUnlock(conn); virDispatchError(conn); return -1; } @@ -1259,10 +1254,7 @@ virConnectUnregisterCloseCallback(virConnectPtr conn, VIR_DEBUG("conn=%p", conn); virResetLastError(); - virCheckConnectReturn(conn, -1); - virObjectLock(conn); - virCheckNonNullArgGoto(cb, error); if (virConnectCloseCallbackDataGetCallback(conn->closeCallback) != cb) { @@ -1273,11 +1265,9 @@ virConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseCallbackDataUnregister(conn->closeCallback, cb); - virObjectUnlock(conn); return 0; error: - virObjectUnlock(conn); virDispatchError(conn); return -1; } -- 1.8.3.1

We don't need locks in dispose functions as they can only be run in one thread for given object. --- src/datatypes.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index 1b95b21..3ea7dff 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -182,13 +182,7 @@ void virConnectCloseCallbackDataReset(virConnectCloseCallbackDataPtr close) static void virConnectCloseCallbackDataDispose(void *obj) { - virConnectCloseCallbackDataPtr cb = obj; - - virObjectLock(cb); - - virConnectCloseCallbackDataReset(cb); - - virObjectUnlock(cb); + virConnectCloseCallbackDataReset(obj); } void virConnectCloseCallbackDataRegister(virConnectCloseCallbackDataPtr close, -- 1.8.3.1

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/datatypes.c | 31 +++++++-------------- src/datatypes.h | 4 +-- src/driver-hypervisor.h | 12 +++++++++ src/libvirt-host.c | 17 +++--------- src/remote/remote_driver.c | 67 +++++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 90 insertions(+), 41 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index 3ea7dff..b353499 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -114,22 +114,10 @@ VIR_ONCE_GLOBAL_INIT(virDataTypes) virConnectPtr virGetConnect(void) { - virConnectPtr ret; - if (virDataTypesInitialize() < 0) return NULL; - if (!(ret = virObjectLockableNew(virConnectClass))) - return NULL; - - if (!(ret->closeCallback = virObjectLockableNew(virConnectCloseCallbackDataClass))) - goto error; - - return ret; - - error: - virObjectUnref(ret); - return NULL; + return virObjectLockableNew(virConnectClass); } /** @@ -150,14 +138,6 @@ virConnectDispose(void *obj) virResetError(&conn->err); virURIFree(conn->uri); - - if (conn->closeCallback) { - virObjectLock(conn->closeCallback); - conn->closeCallback->callback = NULL; - virObjectUnlock(conn->closeCallback); - - virObjectUnref(conn->closeCallback); - } } @@ -185,6 +165,15 @@ virConnectCloseCallbackDataDispose(void *obj) virConnectCloseCallbackDataReset(obj); } +virConnectCloseCallbackDataPtr +virNewConnectCloseCallbackData(void) +{ + if (virDataTypesInitialize() < 0) + return NULL; + + return virObjectLockableNew(virConnectCloseCallbackDataClass); +} + void virConnectCloseCallbackDataRegister(virConnectCloseCallbackDataPtr close, virConnectPtr conn, virConnectCloseFunc cb, diff --git a/src/datatypes.h b/src/datatypes.h index 2ee77cd..a1aecb4 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -396,9 +396,6 @@ struct _virConnect { virError err; /* the last error */ virErrorFunc handler; /* associated handler */ void *userData; /* the user data */ - - /* Per-connection close callback */ - virConnectCloseCallbackDataPtr closeCallback; }; /** @@ -601,6 +598,7 @@ virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain, virAdmConnectPtr virAdmConnectNew(void); +virConnectCloseCallbackDataPtr virNewConnectCloseCallbackData(void); void virConnectCloseCallbackDataRegister(virConnectCloseCallbackDataPtr close, virConnectPtr conn, virConnectCloseFunc cb, diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index ae2ec4d..2cbd01b 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1212,6 +1212,16 @@ typedef int const char *password, unsigned int flags); +typedef int +(*virDrvConnectRegisterCloseCallback)(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb); + +typedef int +(*virDrvConnectUnregisterCloseCallback)(virConnectPtr conn, + virConnectCloseFunc cb); + typedef struct _virHypervisorDriver virHypervisorDriver; typedef virHypervisorDriver *virHypervisorDriverPtr; @@ -1443,6 +1453,8 @@ struct _virHypervisorDriver { virDrvDomainGetFSInfo domainGetFSInfo; virDrvDomainInterfaceAddresses domainInterfaceAddresses; virDrvDomainSetUserPassword domainSetUserPassword; + virDrvConnectRegisterCloseCallback connectRegisterCloseCallback; + virDrvConnectUnregisterCloseCallback connectUnregisterCloseCallback; }; diff --git a/src/libvirt-host.c b/src/libvirt-host.c index b0597ee..24277b7 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c @@ -1217,14 +1217,9 @@ virConnectRegisterCloseCallback(virConnectPtr conn, virCheckConnectReturn(conn, -1); virCheckNonNullArgGoto(cb, error); - if (virConnectCloseCallbackDataGetCallback(conn->closeCallback) != NULL) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("A close callback is already registered")); + if (conn->driver->connectRegisterCloseCallback && + conn->driver->connectRegisterCloseCallback(conn, cb, opaque, freecb) < 0) goto error; - } - - virConnectCloseCallbackDataRegister(conn->closeCallback, conn, cb, - opaque, freecb); return 0; @@ -1257,13 +1252,9 @@ virConnectUnregisterCloseCallback(virConnectPtr conn, virCheckConnectReturn(conn, -1); virCheckNonNullArgGoto(cb, error); - if (virConnectCloseCallbackDataGetCallback(conn->closeCallback) != cb) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("A different callback was requested")); + if (conn->driver->connectUnregisterCloseCallback && + conn->driver->connectUnregisterCloseCallback(conn, cb) < 0) goto error; - } - - virConnectCloseCallbackDataUnregister(conn->closeCallback, cb); return 0; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e0cf33f..b5b7063 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -96,6 +96,7 @@ struct private_data { bool serverEventFilter; /* Does server support modern event filtering */ virObjectEventStatePtr eventState; + virConnectCloseCallbackDataPtr closeCallback; }; enum { @@ -963,11 +964,13 @@ doRemoteOpen(virConnectPtr conn, goto failed; } - virObjectRef(conn->closeCallback); - + if (!(priv->closeCallback = virNewConnectCloseCallbackData())) + goto failed; + // ref on behalf of netclient + virObjectRef(priv->closeCallback); virNetClientSetCloseCallback(priv->client, remoteClientCloseFunc, - conn->closeCallback, virObjectFreeCallback); + priv->closeCallback, virObjectFreeCallback); if (!(priv->remoteProgram = virNetClientProgramNew(REMOTE_PROGRAM, REMOTE_PROTOCOL_VERSION, @@ -1097,6 +1100,8 @@ doRemoteOpen(virConnectPtr conn, virNetClientClose(priv->client); virObjectUnref(priv->client); priv->client = NULL; + virObjectUnref(priv->closeCallback); + priv->closeCallback = NULL; #ifdef WITH_GNUTLS virObjectUnref(priv->tls); priv->tls = NULL; @@ -1229,11 +1234,13 @@ doRemoteClose(virConnectPtr conn, struct private_data *priv) virNetClientSetCloseCallback(priv->client, NULL, - conn->closeCallback, virObjectFreeCallback); + priv->closeCallback, virObjectFreeCallback); virNetClientClose(priv->client); virObjectUnref(priv->client); priv->client = NULL; + virObjectUnref(priv->closeCallback); + priv->closeCallback = NULL; virObjectUnref(priv->remoteProgram); virObjectUnref(priv->lxcProgram); virObjectUnref(priv->qemuProgram); @@ -7887,6 +7894,56 @@ remoteDomainInterfaceAddresses(virDomainPtr dom, return rv; } +static int +remoteConnectRegisterCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb) +{ + struct private_data *priv = conn->privateData; + int ret = -1; + + remoteDriverLock(priv); + + if (virConnectCloseCallbackDataGetCallback(priv->closeCallback) != NULL) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("A close callback is already registered")); + goto cleanup; + } + + virConnectCloseCallbackDataRegister(priv->closeCallback, conn, cb, + opaque, freecb); + ret = 0; + + cleanup: + remoteDriverUnlock(priv); + + return ret; +} + +static int +remoteConnectUnregisterCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb) +{ + struct private_data *priv = conn->privateData; + int ret = -1; + + remoteDriverLock(priv); + + if (virConnectCloseCallbackDataGetCallback(priv->closeCallback) != cb) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("A different callback was requested")); + goto cleanup; + } + + virConnectCloseCallbackDataUnregister(priv->closeCallback, cb); + ret = 0; + + cleanup: + remoteDriverUnlock(priv); + + return ret; +} static int remoteDomainRename(virDomainPtr dom, const char *new_name, unsigned int flags) @@ -8279,6 +8336,8 @@ static virHypervisorDriver hypervisor_driver = { .domainInterfaceAddresses = remoteDomainInterfaceAddresses, /* 1.2.14 */ .domainSetUserPassword = remoteDomainSetUserPassword, /* 1.2.16 */ .domainRename = remoteDomainRename, /* 1.2.19 */ + .connectRegisterCloseCallback = remoteConnectRegisterCloseCallback, /* 1.3.2 */ + .connectUnregisterCloseCallback = remoteConnectUnregisterCloseCallback, /* 1.3.2 */ }; static virNetworkDriver network_driver = { -- 1.8.3.1

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/remote/remote_driver.c | 51 ++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b5b7063..363c367 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -540,6 +540,22 @@ remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED, reason); } +static bool +remoteConnectSupportsFeatureUnlocked(virConnectPtr conn, + struct private_data *priv, + int feature) +{ + remote_connect_supports_feature_args args = { feature }; + remote_connect_supports_feature_ret ret = { 0 }; + int rc; + + rc = call(conn, priv, 0, REMOTE_PROC_CONNECT_SUPPORTS_FEATURE, + (xdrproc_t)xdr_remote_connect_supports_feature_args, (char *) &args, + (xdrproc_t)xdr_remote_connect_supports_feature_ret, (char *) &ret); + + return rc != -1 && ret.supported; +} + /* helper macro to ease extraction of arguments from the URI */ #define EXTRACT_URI_ARG_STR(ARG_NAME, ARG_VAR) \ if (STRCASEEQ(var->name, ARG_NAME)) { \ @@ -1002,18 +1018,9 @@ doRemoteOpen(virConnectPtr conn, goto failed; if (virNetClientKeepAliveIsSupported(priv->client)) { - remote_connect_supports_feature_args args = - { VIR_DRV_FEATURE_PROGRAM_KEEPALIVE }; - remote_connect_supports_feature_ret ret = { 0 }; - int rc; - - rc = call(conn, priv, 0, REMOTE_PROC_CONNECT_SUPPORTS_FEATURE, - (xdrproc_t)xdr_remote_connect_supports_feature_args, (char *) &args, - (xdrproc_t)xdr_remote_connect_supports_feature_ret, (char *) &ret); - - if (rc != -1 && ret.supported) { - priv->serverKeepAlive = true; - } else { + priv->serverKeepAlive = remoteConnectSupportsFeatureUnlocked(conn, + priv, VIR_DRV_FEATURE_PROGRAM_KEEPALIVE); + if (!priv->serverKeepAlive) { VIR_INFO("Disabling keepalive protocol since it is not supported" " by the server"); } @@ -1052,22 +1059,12 @@ doRemoteOpen(virConnectPtr conn, /* Set up events */ if (!(priv->eventState = virObjectEventStateNew())) goto failed; - { - remote_connect_supports_feature_args args = - { VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK }; - remote_connect_supports_feature_ret ret = { 0 }; - int rc; - rc = call(conn, priv, 0, REMOTE_PROC_CONNECT_SUPPORTS_FEATURE, - (xdrproc_t)xdr_remote_connect_supports_feature_args, (char *) &args, - (xdrproc_t)xdr_remote_connect_supports_feature_ret, (char *) &ret); - - if (rc != -1 && ret.supported) { - priv->serverEventFilter = true; - } else { - VIR_INFO("Avoiding server event filtering since it is not " - "supported by the server"); - } + priv->serverEventFilter = remoteConnectSupportsFeatureUnlocked(conn, + priv, VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK); + if (!priv->serverEventFilter) { + VIR_INFO("Avoiding server event filtering since it is not " + "supported by the server"); } /* Successful. */ -- 1.8.3.1

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- daemon/libvirtd.h | 1 + daemon/remote.c | 85 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_internal.h | 5 +++ src/remote/remote_driver.c | 40 +++++++++++++++++++++ src/remote/remote_protocol.x | 24 ++++++++++++- src/remote_protocol-structs | 6 ++++ 6 files changed, 160 insertions(+), 1 deletion(-) diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index efd4823..7271b0f 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -60,6 +60,7 @@ struct daemonClientPrivate { size_t nnetworkEventCallbacks; daemonClientEventCallbackPtr *qemuEventCallbacks; size_t nqemuEventCallbacks; + bool closeRegistered; # if WITH_SASL virNetSASLSessionPtr sasl; diff --git a/daemon/remote.c b/daemon/remote.c index ca692a9..04d8ada 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1208,6 +1208,20 @@ remoteRelayDomainQemuMonitorEvent(virConnectPtr conn, VIR_FREE(details_p); } +static +void remoteRelayConnectionClosedEvent(virConnectPtr conn ATTRIBUTE_UNUSED, int reason, void *opaque) +{ + virNetServerClientPtr client = opaque; + + VIR_DEBUG("Relaying connection closed event, reason %d", reason); + + remote_connect_event_connection_closed_msg msg = { reason }; + remoteDispatchObjectEventSend(client, remoteProgram, + REMOTE_PROC_CONNECT_EVENT_CONNECTION_CLOSED, + (xdrproc_t)xdr_remote_connect_event_connection_closed_msg, + &msg); +} + /* * You must hold lock for at least the client * We don't free stuff here, merely disconnect the client's @@ -1270,6 +1284,12 @@ void remoteClientFreeFunc(void *data) } VIR_FREE(priv->qemuEventCallbacks); + if (priv->closeRegistered) { + if (virConnectUnregisterCloseCallback(priv->conn, + remoteRelayConnectionClosedEvent) < 0) + VIR_WARN("unexpected close callback event deregister failure"); + } + virConnectClose(priv->conn); virIdentitySetCurrent(NULL); @@ -3345,6 +3365,70 @@ remoteDispatchNodeDeviceGetParent(virNetServerPtr server ATTRIBUTE_UNUSED, return rv; } +static int +remoteDispatchConnectCloseCallbackRegister(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr) +{ + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + virMutexLock(&priv->lock); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + // on behalf of close callback + virObjectRef(client); + if (virConnectRegisterCloseCallback(priv->conn, + remoteRelayConnectionClosedEvent, + client, virObjectFreeCallback) < 0) + goto cleanup; + + priv->closeRegistered = true; + rv = 0; + + cleanup: + virMutexUnlock(&priv->lock); + if (rv < 0) + virNetMessageSaveError(rerr); + return rv; +} + +static int +remoteDispatchConnectCloseCallbackUnregister(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr) +{ + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + virMutexLock(&priv->lock); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (virConnectUnregisterCloseCallback(priv->conn, + remoteRelayConnectionClosedEvent) < 0) + goto cleanup; + + priv->closeRegistered = false; + rv = 0; + + cleanup: + virMutexUnlock(&priv->lock); + if (rv < 0) + virNetMessageSaveError(rerr); + return rv; +} /*************************** * Register / deregister events @@ -4145,6 +4229,7 @@ static int remoteDispatchConnectSupportsFeature(virNetServerPtr server ATTRIBUTE switch (args->feature) { case VIR_DRV_FEATURE_FD_PASSING: case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: + case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: supported = 1; break; diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 1313b58..96439d8 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -118,6 +118,11 @@ enum { * Support for server-side event filtering via callback ids in events. */ VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK = 14, + + /* + * Support for driver close callback rpc + */ + VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK = 15, }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 363c367..d84d2c1 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -94,6 +94,7 @@ struct private_data { char *hostname; /* Original hostname */ bool serverKeepAlive; /* Does server support keepalive protocol? */ bool serverEventFilter; /* Does server support modern event filtering */ + bool serverCloseCallback; /* Does server support driver close callback */ virObjectEventStatePtr eventState; virConnectCloseCallbackDataPtr closeCallback; @@ -347,6 +348,11 @@ remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, virNetClientPtr client ATTRIBUTE_UNUSED, void *evdata, void *opaque); +static void +remoteConnectNotifyEventConnectionClosed(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque); + static virNetClientProgramEvent remoteEvents[] = { { REMOTE_PROC_DOMAIN_EVENT_LIFECYCLE, remoteDomainBuildEventLifecycle, @@ -509,8 +515,23 @@ static virNetClientProgramEvent remoteEvents[] = { remoteDomainBuildEventCallbackMigrationIteration, sizeof(remote_domain_event_callback_migration_iteration_msg), (xdrproc_t)xdr_remote_domain_event_callback_migration_iteration_msg }, + { REMOTE_PROC_CONNECT_EVENT_CONNECTION_CLOSED, + remoteConnectNotifyEventConnectionClosed, + sizeof(remote_connect_event_connection_closed_msg), + (xdrproc_t)xdr_remote_connect_event_connection_closed_msg }, }; +static void +remoteConnectNotifyEventConnectionClosed(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque) +{ + virConnectPtr conn = opaque; + struct private_data *priv = conn->privateData; + remote_connect_event_connection_closed_msg *msg = evdata; + + virConnectCloseCallbackDataCall(priv->closeCallback, msg->reason); +} static void remoteDomainBuildQemuMonitorEvent(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, @@ -1067,6 +1088,13 @@ doRemoteOpen(virConnectPtr conn, "supported by the server"); } + priv->serverCloseCallback = remoteConnectSupportsFeatureUnlocked(conn, + priv, VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK); + if (!priv->serverCloseCallback) { + VIR_INFO("Close callback registering isn't supported " + "by the remote side."); + } + /* Successful. */ retcode = VIR_DRV_OPEN_SUCCESS; @@ -7908,6 +7936,12 @@ remoteConnectRegisterCloseCallback(virConnectPtr conn, goto cleanup; } + if (priv->serverCloseCallback && + call(conn, priv, 0, REMOTE_PROC_CONNECT_CLOSE_CALLBACK_REGISTER, + (xdrproc_t) xdr_void, (char *) NULL, + (xdrproc_t) xdr_void, (char *) NULL) == -1) + goto cleanup; + virConnectCloseCallbackDataRegister(priv->closeCallback, conn, cb, opaque, freecb); ret = 0; @@ -7933,6 +7967,12 @@ remoteConnectUnregisterCloseCallback(virConnectPtr conn, goto cleanup; } + if (priv->serverCloseCallback && + call(conn, priv, 0, REMOTE_PROC_CONNECT_CLOSE_CALLBACK_UNREGISTER, + (xdrproc_t) xdr_void, (char *) NULL, + (xdrproc_t) xdr_void, (char *) NULL) == -1) + goto cleanup; + virConnectCloseCallbackDataUnregister(priv->closeCallback, cb); ret = 0; diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index bfdbce7..c6dd51e 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3045,6 +3045,10 @@ struct remote_domain_event_callback_device_added_msg { remote_nonnull_string devAlias; }; +struct remote_connect_event_connection_closed_msg { + int reason; +}; + struct remote_connect_get_cpu_model_names_args { remote_nonnull_string arch; int need_results; @@ -5706,5 +5710,23 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_DOMAIN_EVENT_CALLBACK_MIGRATION_ITERATION = 359 + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_MIGRATION_ITERATION = 359, + + /** + * @generate: none + * @acl: none + */ + REMOTE_PROC_CONNECT_CLOSE_CALLBACK_REGISTER = 360, + + /** + * @generate: none + * @acl: none + */ + REMOTE_PROC_CONNECT_CLOSE_CALLBACK_UNREGISTER = 361, + + /** + * @generate: none + * @acl: none + */ + REMOTE_PROC_CONNECT_EVENT_CONNECTION_CLOSED = 362 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index dff54e8..11048b7 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2502,6 +2502,9 @@ struct remote_domain_event_callback_device_added_msg { remote_nonnull_domain dom; remote_nonnull_string devAlias; }; +struct remote_connect_event_connection_closed_msg { + int reason; +}; struct remote_connect_get_cpu_model_names_args { remote_nonnull_string arch; int need_results; @@ -3057,4 +3060,7 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_USER_PASSWORD = 357, REMOTE_PROC_DOMAIN_RENAME = 358, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_MIGRATION_ITERATION = 359, + REMOTE_PROC_CONNECT_CLOSE_CALLBACK_REGISTER = 360, + REMOTE_PROC_CONNECT_CLOSE_CALLBACK_UNREGISTER = 361, + REMOTE_PROC_CONNECT_EVENT_CONNECTION_CLOSED = 362, }; -- 1.8.3.1

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/vz/vz_driver.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/vz/vz_sdk.c | 4 ++++ src/vz/vz_utils.h | 3 +++ 3 files changed, 66 insertions(+) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 925ff32..00e0f67 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -254,6 +254,9 @@ vzOpenDefault(virConnectPtr conn) if (prlsdkSubscribeToPCSEvents(privconn)) goto error; + if (!(privconn->closeCallback = virNewConnectCloseCallbackData())) + goto error; + conn->privateData = privconn; if (prlsdkLoadDomains(privconn)) @@ -262,6 +265,8 @@ vzOpenDefault(virConnectPtr conn) return VIR_DRV_OPEN_SUCCESS; error: + virObjectUnref(privconn->closeCallback); + privconn->closeCallback = NULL; virObjectUnref(privconn->domains); virObjectUnref(privconn->caps); virObjectEventStateFree(privconn->domainEventState); @@ -329,6 +334,8 @@ vzConnectClose(virConnectPtr conn) virObjectUnref(privconn->caps); virObjectUnref(privconn->xmlopt); virObjectUnref(privconn->domains); + virObjectUnref(privconn->closeCallback); + privconn->closeCallback = NULL; virObjectEventStateFree(privconn->domainEventState); prlsdkDisconnect(privconn); conn->privateData = NULL; @@ -1461,6 +1468,56 @@ vzNodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED) return freeMem; } +static int +vzConnectRegisterCloseCallback(virConnectPtr conn, + virConnectCloseFunc cb, + void *opaque, + virFreeCallback freecb) +{ + vzConnPtr privconn = conn->privateData; + int ret = -1; + + vzDriverLock(privconn); + + if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != NULL) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("A close callback is already registered")); + goto cleanup; + } + + virConnectCloseCallbackDataRegister(privconn->closeCallback, conn, cb, + opaque, freecb); + ret = 0; + + cleanup: + vzDriverUnlock(privconn); + + return ret; +} + +static int +vzConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb) +{ + vzConnPtr privconn = conn->privateData; + int ret = -1; + + vzDriverLock(privconn); + + if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != cb) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("A different callback was requested")); + goto cleanup; + } + + virConnectCloseCallbackDataUnregister(privconn->closeCallback, cb); + ret = 0; + + cleanup: + vzDriverUnlock(privconn); + + return ret; +} + static virHypervisorDriver vzDriver = { .name = "vz", .connectOpen = vzConnectOpen, /* 0.10.0 */ @@ -1523,6 +1580,8 @@ static virHypervisorDriver vzDriver = { .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */ .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */ .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */ + .connectRegisterCloseCallback = vzConnectRegisterCloseCallback, /* 1.3.2 */ + .connectUnregisterCloseCallback = vzConnectUnregisterCloseCallback, /* 1.3.2 */ }; static virConnectDriver vzConnectDriver = { diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 30ea545..6798aaa 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1751,6 +1751,10 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) /* above function takes own of event */ prlEvent = PRL_INVALID_HANDLE; break; + case PET_DSP_EVT_DISP_CONNECTION_CLOSED: + virConnectCloseCallbackDataCall(privconn->closeCallback, + VIR_CONNECT_CLOSE_REASON_EOF); + break; default: VIR_DEBUG("Skipping event of type %d", prlEventType); } diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index b7a4c81..a2468b4 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -30,6 +30,7 @@ # include "conf/virdomainobjlist.h" # include "conf/domain_event.h" # include "virthread.h" +# include "datatypes.h" # define vzParseError() \ virReportErrorHelper(VIR_FROM_TEST, VIR_ERR_OPERATION_FAILED, __FILE__, \ @@ -59,6 +60,8 @@ struct _vzConn { virDomainXMLOptionPtr xmlopt; virObjectEventStatePtr domainEventState; const char *drivername; + /* Immutable pointer, self-locking APIs */ + virConnectCloseCallbackDataPtr closeCallback; }; typedef struct _vzConn vzConn; -- 1.8.3.1