9:33 a.m.
Notify of connection close event from parallels driver (possibly) wrapped in
the remote driver.
Discussion.
In 1 and 2 patch we forced to some decisions because we don't have a weak
reference mechanics.
1 patch.
-----------
virConnectCloseCallback is introduced because we can not reference the
connection object itself when setting a network layer callback because of how
connection close works.
A connection close procedure is next:
1. client closes connection
2. a this point nobody else referencing a connection and it is disposed
3. connection dispose unreferencing network connection
4. network connection disposes
Thus if we referece a connection in network close callback we never get step 2.
virConnectCloseCallback broke this cycle but at cost that clients MUST
unregister explicitly before closing connection. This is not good as this
unregistration is not really neaded. Client is not telling that it does not
want to receive events anymore but rather forced to obey some
implementation-driven rules.
2 patch.
-----------
We impose requirements on driver implementations which is fragile. Moreover we
again need to make explicit unregistrations. Implementation of domain events
illustrates this point. remoteDispatchConnectDomainEventRegister does not
reference NetClient and makes unregistration before NetClient is disposed but
drivers do not meet the formulated requirements. Object event system release
lock before delivering event for re-entrance purposes.
Shortly we have 2 undesired consequences here.
1. Mandatory unregistration.
2. Imposing multi-threading requirements.
Introduction of weak pointers could free us from these artifacts. Next weak
reference workflow illustrates this.
1. Take weak reference on object of interest before passing to party. This
doesn't break disposing mechanics as weak eference does not prevent from
disposing object. Object is disposed but memory is not freed yet if there are
weak references.
2. When callback is called we are safe to check if pointer dangling as we make
a weak reference before.
3. Release weak reference and this trigger memory freeing if there are no more
weak references.
daemon/libvirtd.h | 1 +
daemon/remote.c | 87 ++++++++++++++++++++++++++++++++
src/datatypes.c | 112 +++++++++++++++++++++++++++++++----------
src/datatypes.h | 21 ++++++--
src/driver-hypervisor.h | 12 +++++
src/libvirt-host.c | 79 ++++++++++--------------------
src/remote/remote_driver.c | 108 +++++++++++++++++++++++++++++++---------
src/remote/remote_protocol.x | 24 +++++++++-
src/remote_protocol-structs | 6 ++
src/vz/vz_driver.c | 26 ++++++++++
src/vz/vz_sdk.c | 32 +++++++++++-
src/vz/vz_utils.h | 3 +
12 files changed, 396 insertions(+), 115 deletions(-)
9:33 a.m.
New subject: [libvirt] [PATCH 1/3] remote: move connection close callback to driver level
1. Introduce connect(Un)RegisterCloseCallback driver functions.
2. virConnect(Un)RegisterCloseCallback now works through driver.
3. virConnectCloseCallback is factored from virConnect but mostly stay the
same. Notice however that virConnect object is not referenced in
virConnectCloseCallback anymore. It is safe. Explanation.
Previous version of callback object keeps reference to connection. This leads
to undocumented rule that all clients must exlicitly unregister close callback
before closing connection or connection will never be disposed. As callback
unregistering and close event delivering are serialized thru callback object
lock and unregistering zeros connection object we will never get dangling
pointer on delivering.
4. callback object doesn't check callback on unregistering. The reason is that
it will helps us write registering/unregistering with atomic behaviour for
remote driver as it can be seen in next patch. Moreover it is not really
meaningful to check callback on unregistering.
5. virNetClientSetCloseCallback call is removed from doRemoteClose as it is
excessive for the same reasons as in point 3. Unregistering MUST be called
and this prevents from firing event on close initiated by client.
I'm not sure where callback object should be so it stays in datatype.c
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/datatypes.c | 112 +++++++++++++++++++++++++++++++++-----------
src/datatypes.h | 21 ++++++--
src/driver-hypervisor.h | 12 +++++
src/libvirt-host.c | 79 ++++++++++---------------------
src/remote/remote_driver.c | 67 ++++++++++++++++----------
5 files changed, 179 insertions(+), 112 deletions(-)
diff --git a/src/datatypes.c b/src/datatypes.c
index 12bcfc1..07212d2 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -34,7 +34,7 @@
VIR_LOG_INIT("datatypes");
virClassPtr virConnectClass;
-virClassPtr virConnectCloseCallbackDataClass;
+virClassPtr virConnectCloseCallbackClass;
virClassPtr virDomainClass;
virClassPtr virDomainSnapshotClass;
virClassPtr virInterfaceClass;
@@ -47,7 +47,7 @@ virClassPtr virStorageVolClass;
virClassPtr virStoragePoolClass;
static void virConnectDispose(void *obj);
-static void virConnectCloseCallbackDataDispose(void *obj);
+static void virConnectCloseCallbackDispose(void *obj);
static void virDomainDispose(void *obj);
static void virDomainSnapshotDispose(void *obj);
static void virInterfaceDispose(void *obj);
@@ -78,7 +78,7 @@ virDataTypesOnceInit(void)
DECLARE_CLASS_COMMON(basename, virClassForObjectLockable())
DECLARE_CLASS_LOCKABLE(virConnect);
- DECLARE_CLASS_LOCKABLE(virConnectCloseCallbackData);
+ DECLARE_CLASS_LOCKABLE(virConnectCloseCallback);
DECLARE_CLASS(virDomain);
DECLARE_CLASS(virDomainSnapshot);
DECLARE_CLASS(virInterface);
@@ -119,14 +119,7 @@ virGetConnect(void)
if (!(ret = virObjectLockableNew(virConnectClass)))
return NULL;
- if (!(ret->closeCallback =
virObjectLockableNew(virConnectCloseCallbackDataClass)))
- goto error;
-
return ret;
-
- error:
- virObjectUnref(ret);
- return NULL;
}
/**
@@ -147,36 +140,99 @@ virConnectDispose(void *obj)
virResetError(&conn->err);
virURIFree(conn->uri);
+}
- if (conn->closeCallback) {
- virObjectLock(conn->closeCallback);
- conn->closeCallback->callback = NULL;
- virObjectUnlock(conn->closeCallback);
+virConnectCloseCallbackPtr
+virGetConnectCloseCallback(void)
+{
+ virConnectCloseCallbackPtr ret;
- virObjectUnref(conn->closeCallback);
- }
+ if (virDataTypesInitialize() < 0)
+ return NULL;
+
+ if (!(ret = virObjectLockableNew(virConnectCloseCallbackClass)))
+ return NULL;
+
+ return ret;
}
+static void
+virConnectCloseCallbackClean(virConnectCloseCallbackPtr obj)
+{
+ if (obj->freeCallback)
+ obj->freeCallback(obj->opaque);
+
+ obj->callback = NULL;
+ obj->freeCallback = NULL;
+ obj->opaque = NULL;
+ obj->conn = NULL;
+}
-/**
- * virConnectCloseCallbackDataDispose:
- * @obj: the close callback data to release
- *
- * Release resources bound to the connection close callback.
- */
static void
-virConnectCloseCallbackDataDispose(void *obj)
+virConnectCloseCallbackDispose(void *obj ATTRIBUTE_UNUSED)
+{
+ // nothing really to to here
+}
+
+int
+virConnectCloseCallbackRegister(virConnectCloseCallbackPtr obj,
+ virConnectPtr conn,
+ virConnectCloseFunc cb,
+ void *opaque,
+ virFreeCallback freecb)
{
- virConnectCloseCallbackDataPtr cb = obj;
+ int ret = -1;
+
+ virObjectLock(obj);
- virObjectLock(cb);
+ if (obj->callback) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("A close callback is already registered"));
+ goto finish;
+ }
- if (cb->freeCallback)
- cb->freeCallback(cb->opaque);
+ // connection is not referenced, thus clients MUST call
+ // unregister before closing connection and
+ // as delivering event and unregistering are serialized
+ // this is safe
+ obj->conn = conn;
+ obj->callback = cb;
+ obj->opaque = opaque;
+ obj->freeCallback = freecb;
- virObjectUnlock(cb);
+ ret = 0;
+
+ finish:
+ virObjectUnlock(obj);
+ return ret;
}
+void
+virConnectCloseCallbackUnregister(virConnectCloseCallbackPtr obj)
+{
+ virObjectLock(obj);
+ virConnectCloseCallbackClean(obj);
+ virObjectUnlock(obj);
+}
+
+void
+virConnectCloseCallbackCall(virConnectCloseCallbackPtr obj, int reason)
+{
+ virObjectLock(obj);
+
+ // in triggered on unregistered state
+ if (!obj->callback)
+ goto finish;
+
+ VIR_DEBUG("Triggering connection close callback %p reason=%d, opaque=%p",
+ obj->callback, reason, obj->opaque);
+
+ obj->callback(obj->conn, reason, obj->opaque);
+ virConnectCloseCallbackClean(obj);
+
+ finish:
+ virObjectUnlock(obj);
+}
/**
* virGetDomain:
diff --git a/src/datatypes.h b/src/datatypes.h
index c498cb0..86230e7 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -329,13 +329,13 @@ extern virClassPtr virAdmConnectClass;
__VA_ARGS__)
-typedef struct _virConnectCloseCallbackData virConnectCloseCallbackData;
-typedef virConnectCloseCallbackData *virConnectCloseCallbackDataPtr;
+typedef struct _virConnectCloseCallback virConnectCloseCallback;
+typedef virConnectCloseCallback *virConnectCloseCallbackPtr;
/**
* Internal structure holding data related to connection close callbacks.
*/
-struct _virConnectCloseCallbackData {
+struct _virConnectCloseCallback {
virObjectLockable parent;
virConnectPtr conn;
@@ -385,9 +385,6 @@ struct _virConnect {
virError err; /* the last error */
virErrorFunc handler; /* associated handlet */
void *userData; /* the user data */
-
- /* Per-connection close callback */
- virConnectCloseCallbackDataPtr closeCallback;
};
/**
@@ -586,4 +583,16 @@ virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain,
virAdmConnectPtr virAdmConnectNew(void);
+virConnectCloseCallbackPtr virGetConnectCloseCallback(void);
+int
+virConnectCloseCallbackRegister(virConnectCloseCallbackPtr obj,
+ virConnectPtr conn,
+ virConnectCloseFunc cb,
+ void *opaque,
+ virFreeCallback freecb);
+void
+virConnectCloseCallbackUnregister(virConnectCloseCallbackPtr obj);
+void
+virConnectCloseCallbackCall(virConnectCloseCallbackPtr obj, int reason);
+
#endif /* __VIR_DATATYPES_H__ */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 3275343..644e255 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1207,6 +1207,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;
@@ -1437,6 +1447,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 9c88426..2608171 100644
--- a/src/libvirt-host.c
+++ b/src/libvirt-host.c
@@ -1181,7 +1181,6 @@ virConnectIsAlive(virConnectPtr conn)
return -1;
}
-
/**
* virConnectRegisterCloseCallback:
* @conn: pointer to connection object
@@ -1205,50 +1204,37 @@ virConnectIsAlive(virConnectPtr conn)
*
* Returns 0 on success, -1 on error
*/
+
int
virConnectRegisterCloseCallback(virConnectPtr conn,
virConnectCloseFunc cb,
void *opaque,
virFreeCallback freecb)
{
- VIR_DEBUG("conn=%p", conn);
+ int ret;
+
+ VIR_DEBUG("conn=%p cb=%p opaque=%p freecb=%p",
+ conn, cb, opaque, freecb);
virResetLastError();
virCheckConnectReturn(conn, -1);
-
- virObjectRef(conn);
+ virCheckNonNullArgGoto(cb, finish);
virObjectLock(conn);
- virObjectLock(conn->closeCallback);
-
- virCheckNonNullArgGoto(cb, error);
-
- if (conn->closeCallback->callback) {
- 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;
-
- virObjectUnlock(conn->closeCallback);
+ if (conn->driver && conn->driver->connectRegisterCloseCallback)
+ ret = conn->driver->connectRegisterCloseCallback(conn, cb, opaque,
freecb);
+ else
+ ret = 0;
virObjectUnlock(conn);
- return 0;
+ finish:
+ if (ret < 0)
+ virDispatchError(conn);
- error:
- virObjectUnlock(conn->closeCallback);
- virObjectUnlock(conn);
- virDispatchError(conn);
- virObjectUnref(conn);
- return -1;
+ return ret;
}
-
/**
* virConnectUnregisterCloseCallback:
* @conn: pointer to connection object
@@ -1266,41 +1252,28 @@ int
virConnectUnregisterCloseCallback(virConnectPtr conn,
virConnectCloseFunc cb)
{
- VIR_DEBUG("conn=%p", conn);
+ int ret;
+
+ VIR_DEBUG("conn=%p, cb=%p", conn, cb);
virResetLastError();
virCheckConnectReturn(conn, -1);
-
- virObjectLock(conn);
- virObjectLock(conn->closeCallback);
-
virCheckNonNullArgGoto(cb, error);
- if (conn->closeCallback->callback != 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;
-
- virObjectUnlock(conn->closeCallback);
+ virObjectLock(conn);
+ if (conn->driver && conn->driver->connectRegisterCloseCallback)
+ ret = conn->driver->connectUnregisterCloseCallback(conn, cb);
+ else
+ ret = 0;
virObjectUnlock(conn);
- virObjectUnref(conn);
-
- return 0;
error:
- virObjectUnlock(conn->closeCallback);
- virObjectUnlock(conn);
- virDispatchError(conn);
- return -1;
-}
+ if (ret < 0)
+ virDispatchError(conn);
+ return ret;
+}
/**
* virNodeGetCPUMap:
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 273799b..9e05c8a 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -101,6 +101,7 @@ struct private_data {
bool serverEventFilter; /* Does server support modern event filtering */
virObjectEventStatePtr eventState;
+ virConnectCloseCallbackPtr closeCallback;
};
enum {
@@ -531,25 +532,7 @@ 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);
-
- /* free the connection reference that comes along with the callback
- * registration */
- virObjectUnref(cbdata->conn);
+ virConnectCloseCallbackCall(opaque, reason);
}
/* helper macro to ease extraction of arguments from the URI */
@@ -976,11 +959,14 @@ doRemoteOpen(virConnectPtr conn,
goto failed;
}
- virObjectRef(conn->closeCallback);
+ if (!(priv->closeCallback = virGetConnectCloseCallback()))
+ 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,
@@ -1044,6 +1030,8 @@ doRemoteOpen(virConnectPtr conn,
if (conn->uri == NULL) {
remote_connect_get_uri_ret uriret;
+
+
VIR_DEBUG("Trying to query remote URI");
memset(&uriret, 0, sizeof(uriret));
if (call(conn, priv, 0,
@@ -1107,6 +1095,8 @@ doRemoteOpen(virConnectPtr conn,
virObjectUnref(priv->remoteProgram);
virObjectUnref(priv->lxcProgram);
virObjectUnref(priv->qemuProgram);
+ virObjectUnref(priv->closeCallback);
+ priv->closeCallback = NULL;
virNetClientClose(priv->client);
virObjectUnref(priv->client);
priv->client = NULL;
@@ -1240,17 +1230,15 @@ doRemoteClose(virConnectPtr conn, struct private_data *priv)
priv->tls = NULL;
#endif
- virNetClientSetCloseCallback(priv->client,
- NULL,
- conn->closeCallback, virObjectFreeCallback);
-
virNetClientClose(priv->client);
virObjectUnref(priv->client);
priv->client = NULL;
virObjectUnref(priv->remoteProgram);
virObjectUnref(priv->lxcProgram);
virObjectUnref(priv->qemuProgram);
+ virObjectUnref(priv->closeCallback);
priv->remoteProgram = priv->qemuProgram = priv->lxcProgram = NULL;
+ priv->closeCallback = NULL;
/* Free hostname copy */
VIR_FREE(priv->hostname);
@@ -8041,6 +8029,33 @@ remoteDomainInterfaceAddresses(virDomainPtr dom,
return rv;
}
+static int
+remoteConnectRegisterCloseCallback(virConnectPtr conn,
+ virConnectCloseFunc cb,
+ void *opaque,
+ virFreeCallback freecb)
+{
+ struct private_data *priv = conn->privateData;
+ int ret = 0;
+
+ remoteDriverLock(priv);
+ ret = virConnectCloseCallbackRegister(priv->closeCallback, conn, cb, opaque,
freecb);
+ remoteDriverUnlock(priv);
+
+ return ret;
+}
+
+static int
+remoteConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb
ATTRIBUTE_UNUSED)
+{
+ struct private_data *priv = conn->privateData;
+
+ remoteDriverLock(priv);
+ virConnectCloseCallbackUnregister(priv->closeCallback);
+ remoteDriverUnlock(priv);
+
+ return 0;
+}
/* get_nonnull_domain and get_nonnull_network turn an on-wire
* (name, uuid) pair into virDomainPtr or virNetworkPtr object.
@@ -8391,6 +8406,8 @@ static virHypervisorDriver hypervisor_driver = {
.domainGetFSInfo = remoteDomainGetFSInfo, /* 1.2.11 */
.domainInterfaceAddresses = remoteDomainInterfaceAddresses, /* 1.2.14 */
.domainSetUserPassword = remoteDomainSetUserPassword, /* 1.2.16 */
+ .connectRegisterCloseCallback = remoteConnectRegisterCloseCallback, /* 1.3.0 */
+ .connectUnregisterCloseCallback = remoteConnectUnregisterCloseCallback, /* 1.3.0 */
};
static virNetworkDriver network_driver = {
--
1.7.1
4:22 a.m.
New subject: [libvirt] [PATCH 1/3] remote: move connection close callback to driver level
On Wed, Jun 24, 2015 at 05:33:51PM +0300, Nikolay Shirokovskiy wrote:
1. Introduce connect(Un)RegisterCloseCallback driver functions.
2. virConnect(Un)RegisterCloseCallback now works through driver.
3. virConnectCloseCallback is factored from virConnect but mostly stay the
same. Notice however that virConnect object is not referenced in
virConnectCloseCallback anymore. It is safe. Explanation.
Previous version of callback object keeps reference to connection. This leads
to undocumented rule that all clients must exlicitly unregister close callback
before closing connection or connection will never be disposed. As callback
unregistering and close event delivering are serialized thru callback object
lock and unregistering zeros connection object we will never get dangling
pointer on delivering.
4. callback object doesn't check callback on unregistering. The reason is that
it will helps us write registering/unregistering with atomic behaviour for
remote driver as it can be seen in next patch. Moreover it is not really
meaningful to check callback on unregistering.
5. virNetClientSetCloseCallback call is removed from doRemoteClose as it is
excessive for the same reasons as in point 3. Unregistering MUST be called
and this prevents from firing event on close initiated by client.
I'm not sure where callback object should be so it stays in datatype.c
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/datatypes.c | 112 +++++++++++++++++++++++++++++++++-----------
src/datatypes.h | 21 ++++++--
src/driver-hypervisor.h | 12 +++++
src/libvirt-host.c | 79 ++++++++++---------------------
src/remote/remote_driver.c | 67 ++++++++++++++++----------
5 files changed, 179 insertions(+), 112 deletions(-)
Lots of spurious whitespace changes, some of them make the commit very
messy and make syntax-check fails again after this.
Martin
9:33 a.m.
New subject: [libvirt] [PATCH 2/3] daemon: relay connection close event related functions
Add conneciton close subscription/unsubscription and event rpc.
Now remote driver firing connection close event
in 2 cases.
1. connection to daemon closed, as previously
2. as a relay of connection close event from wrapped driver
As it commented out in remoteDispatchConnectCloseCallbackRegister we impose
some multi-thread requirements on drivers implementations. This is the same
approach as in for example remoteDispatchConnectDomainEventRegister.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
daemon/libvirtd.h | 1 +
daemon/remote.c | 87 ++++++++++++++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 50 +++++++++++++++++++++--
src/remote/remote_protocol.x | 24 +++++++++++-
src/remote_protocol-structs | 6 +++
5 files changed, 162 insertions(+), 6 deletions(-)
diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h
index 8c1a904..5b2d2ca 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 e9e2dca..e0cc008 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1189,6 +1189,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_CLOSE,
+
(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
@@ -1251,6 +1265,13 @@ 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);
@@ -3489,6 +3510,72 @@ 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;
+ }
+
+ // NetClient is passed to driver but not referenced.
+ // This imposes next requirements on drivers implementation.
+ // Driver must serialize unregistering and event delivering operations.
+ // Thus as we unregister callback before unreferencing NetClient
+ // remoteRelayConnectionClosedEvent is safe to use NetClient.
+ if (virConnectRegisterCloseCallback(priv->conn, remoteRelayConnectionClosedEvent,
client, NULL) < 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
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 9e05c8a..e6236be 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -347,6 +347,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,
@@ -505,8 +510,22 @@ static virNetClientProgramEvent remoteEvents[] = {
remoteDomainBuildEventCallbackDeviceAdded,
sizeof(remote_domain_event_callback_device_added_msg),
(xdrproc_t)xdr_remote_domain_event_callback_device_added_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;
+ remote_connect_event_connection_closed_msg *msg = evdata;
+
+ virConnectCloseCallbackCall(conn->privateData, msg->reason);
+}
static void
remoteDomainBuildQemuMonitorEvent(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
@@ -8036,12 +8055,23 @@ remoteConnectRegisterCloseCallback(virConnectPtr conn,
virFreeCallback freecb)
{
struct private_data *priv = conn->privateData;
- int ret = 0;
+ int ret = -1;
remoteDriverLock(priv);
- ret = virConnectCloseCallbackRegister(priv->closeCallback, conn, cb, opaque,
freecb);
- remoteDriverUnlock(priv);
+ if (virConnectCloseCallbackRegister(priv->closeCallback, conn, cb, opaque, freecb)
< 0)
+ goto cleanup;
+
+ if (call(conn, priv, 0, REMOTE_PROC_CONNECT_CLOSE_CALLBACK_REGISTER,
+ (xdrproc_t) xdr_void, (char *) NULL,
+ (xdrproc_t) xdr_void, (char *) NULL) == -1) {
+ virConnectCloseCallbackUnregister(priv->closeCallback);
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+ remoteDriverUnlock(priv);
return ret;
}
@@ -8049,12 +8079,22 @@ static int
remoteConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb
ATTRIBUTE_UNUSED)
{
struct private_data *priv = conn->privateData;
+ int ret = -1;
remoteDriverLock(priv);
+
+ if (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;
+ }
+
virConnectCloseCallbackUnregister(priv->closeCallback);
- remoteDriverUnlock(priv);
- return 0;
+ ret = 0;
+ cleanup:
+ remoteDriverUnlock(priv);
+ return ret;
}
/* get_nonnull_domain and get_nonnull_network turn an on-wire
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 9f1be6b..3d54a69 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -3063,6 +3063,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;
@@ -5696,5 +5700,23 @@ enum remote_procedure {
* @generate:both
* @acl: domain:set_password
*/
- REMOTE_PROC_DOMAIN_SET_USER_PASSWORD = 357
+ REMOTE_PROC_DOMAIN_SET_USER_PASSWORD = 357,
+
+ /**
+ * @generate: none
+ * @acl: none
+ */
+ REMOTE_PROC_CONNECT_CLOSE_CALLBACK_REGISTER = 358,
+
+ /**
+ * @generate: none
+ * @acl: none
+ */
+ REMOTE_PROC_CONNECT_CLOSE_CALLBACK_UNREGISTER = 359,
+
+ /**
+ * @generate: none
+ * @acl: none
+ */
+ REMOTE_PROC_CONNECT_EVENT_CONNECTION_CLOSED = 360
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 48c3bd8..7a8e1c4 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;
@@ -3042,4 +3045,7 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_ADD_IOTHREAD = 355,
REMOTE_PROC_DOMAIN_DEL_IOTHREAD = 356,
REMOTE_PROC_DOMAIN_SET_USER_PASSWORD = 357,
+ REMOTE_PROC_CONNECT_CLOSE_CALLBACK_REGISTER = 358,
+ REMOTE_PROC_CONNECT_CLOSE_CALLBACK_UNREGISTER = 359,
+ REMOTE_PROC_CONNECT_EVENT_CONNECTION_CLOSED = 360,
};
--
1.7.1
9:33 a.m.
New subject: [libvirt] [PATCH 3/3] vz: implement connection close notification
Reuse virConnectCloseCallback to implement connection close event functions.
Thus we automatically meet multi-thread requirements on unregistering/notification.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
daemon/remote.c | 2 +-
src/remote/remote_driver.c | 3 ++-
src/vz/vz_driver.c | 26 ++++++++++++++++++++++++++
src/vz/vz_sdk.c | 32 ++++++++++++++++++++++++++++++--
src/vz/vz_utils.h | 3 +++
5 files changed, 62 insertions(+), 4 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index e0cc008..ca4b63d 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1198,7 +1198,7 @@ void remoteRelayConnectionClosedEvent(virConnectPtr conn
ATTRIBUTE_UNUSED, int r
remote_connect_event_connection_closed_msg msg = { reason };
remoteDispatchObjectEventSend(client, remoteProgram,
- REMOTE_PROC_CONNECT_CLOSE,
+ REMOTE_PROC_CONNECT_EVENT_CONNECTION_CLOSED,
(xdrproc_t)xdr_remote_connect_event_connection_closed_msg,
&msg);
}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index e6236be..90f813a 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -522,9 +522,10 @@ remoteConnectNotifyEventConnectionClosed(virNetClientProgramPtr prog
ATTRIBUTE_U
void *evdata, void *opaque)
{
virConnectPtr conn = opaque;
+ struct private_data *priv = conn->privateData;
remote_connect_event_connection_closed_msg *msg = evdata;
- virConnectCloseCallbackCall(conn->privateData, msg->reason);
+ virConnectCloseCallbackCall(priv->closeCallback, msg->reason);
}
static void
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index cef3c77..26caf19 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -268,6 +268,9 @@ vzOpenDefault(virConnectPtr conn)
if (prlsdkSubscribeToPCSEvents(privconn))
goto error;
+ if (!(privconn->closeCallback = virGetConnectCloseCallback()))
+ goto error;
+
conn->privateData = privconn;
if (prlsdkLoadDomains(privconn))
@@ -276,6 +279,8 @@ vzOpenDefault(virConnectPtr conn)
return VIR_DRV_OPEN_SUCCESS;
error:
+ virObjectUnref(privconn->closeCallback);
+ privconn->closeCallback = NULL;
virObjectUnref(privconn->domains);
virObjectUnref(privconn->caps);
virStoragePoolObjListFree(&privconn->pools);
@@ -350,6 +355,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;
@@ -1321,6 +1328,23 @@ vzDomainBlockStatsFlags(virDomainPtr domain,
return ret;
}
+static int
+vzConnectRegisterCloseCallback(virConnectPtr conn,
+ virConnectCloseFunc cb,
+ void *opaque,
+ virFreeCallback freecb)
+{
+ vzConnPtr privconn = conn->privateData;
+ return virConnectCloseCallbackRegister(privconn->closeCallback, conn, cb, opaque,
freecb);
+}
+
+static int
+vzConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb
ATTRIBUTE_UNUSED)
+{
+ vzConnPtr privconn = conn->privateData;
+ virConnectCloseCallbackUnregister(privconn->closeCallback);
+ return 0;
+}
static virHypervisorDriver vzDriver = {
.name = "vz",
@@ -1373,6 +1397,8 @@ static virHypervisorDriver vzDriver = {
.domainGetMaxMemory = vzDomainGetMaxMemory, /* 1.2.15 */
.domainBlockStats = vzDomainBlockStats, /* 1.3.0 */
.domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.3.0 */
+ .connectRegisterCloseCallback = vzConnectRegisterCloseCallback, /* 1.3.0 */
+ .connectUnregisterCloseCallback = vzConnectUnregisterCloseCallback, /* 1.3.0 */
};
static virConnectDriver vzConnectDriver = {
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 98f7a57..fef064c 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1736,8 +1736,7 @@ prlsdkHandleVmEvent(vzConnPtr privconn, PRL_HANDLE prlEvent)
prlEvent = PRL_INVALID_HANDLE;
break;
default:
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Can't handle event of type %d"), prlEventType);
+ VIR_DEBUG("Skipping vm event of type %d", prlEventType);
}
cleanup:
@@ -1745,6 +1744,32 @@ prlsdkHandleVmEvent(vzConnPtr privconn, PRL_HANDLE prlEvent)
return;
}
+static
+void prlsdkHandleDispatcherConnectionClosed(vzConnPtr privconn)
+{
+ virConnectCloseCallbackCall(privconn->closeCallback,
VIR_CONNECT_CLOSE_REASON_EOF);
+}
+
+static void
+prlsdkHandleDispatcherEvent(vzConnPtr privconn, PRL_HANDLE prlEvent)
+{
+ PRL_RESULT pret = PRL_ERR_FAILURE;
+ PRL_EVENT_TYPE prlEventType;
+
+ pret = PrlEvent_GetType(prlEvent, &prlEventType);
+ prlsdkCheckRetGoto(pret, error);
+
+ switch (prlEventType) {
+ case PET_DSP_EVT_DISP_CONNECTION_CLOSED:
+ prlsdkHandleDispatcherConnectionClosed(privconn);
+ break;
+ default:
+ VIR_DEBUG("Skipping dispatcher event of type %d", prlEventType);
+ }
+ error:
+ return;
+}
+
static PRL_RESULT
prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque)
{
@@ -1772,6 +1797,9 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque)
// above function takes own of event
prlEvent = PRL_INVALID_HANDLE;
break;
+ case PIE_DISPATCHER:
+ prlsdkHandleDispatcherEvent(privconn, prlEvent);
+ break;
default:
VIR_DEBUG("Skipping event of issuer type %d", prlIssuerType);
}
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index 9b46bf9..b0dc3d8 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -32,6 +32,7 @@
# include "conf/network_conf.h"
# include "virthread.h"
# include "virjson.h"
+# include "datatypes.h"
# define vzParseError() \
virReportErrorHelper(VIR_FROM_TEST, VIR_ERR_OPERATION_FAILED, __FILE__, \
@@ -69,6 +70,8 @@ struct _vzConn {
virObjectEventStatePtr domainEventState;
virStorageDriverStatePtr storageState;
const char *drivername;
+ /* Immutable pointer, self-locking APIs */
+ virConnectCloseCallbackPtr closeCallback;
};
typedef struct _vzConn vzConn;
--
1.7.1
3431
days inactive
3432
days old
4 comments
2 participants
participants (2)
-
Martin Kletzander -
Nikolay Shirokovskiy