From 39b5e4d4d8622aa55251cddb03a48068655d8647 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Thu, 9 Aug 2012 12:31:34 +0100 Subject: [PATCH] Refactor RPC client private data setup Currently there is a hook function that is invoked when a new client connection comes in, which allows an app to setup private data. This setup will make it difficult to serialize client state during process re-exec(). Change to a model where the app registers a callback when creating the virNetServerPtr instance, which is used to allocate the client private data immediately during virNetClientPtr construction. Signed-off-by: Daniel P. Berrange --- daemon/libvirtd.c | 1 + daemon/remote.c | 15 ++++++--------- daemon/remote.h | 6 +++--- src/libvirt_private.syms | 1 - src/lxc/lxc_controller.c | 23 +++++++++++++++++------ src/rpc/virnetserver.c | 24 +++++++++++++----------- src/rpc/virnetserver.h | 9 +++------ src/rpc/virnetserverclient.c | 31 +++++++++++++------------------ src/rpc/virnetserverclient.h | 13 +++++++------ 9 files changed, 63 insertions(+), 60 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index fa9e7e8e97..24e20f8153 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1196,6 +1196,7 @@ int main(int argc, char **argv) { !!config->keepalive_required, config->mdns_adv ? config->mdns_name : NULL, remoteClientInitHook, + remoteClientFreeFunc, NULL))) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; diff --git a/daemon/remote.c b/daemon/remote.c index 832307e356..851bcc1cfa 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -631,7 +631,7 @@ verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); * We keep the libvirt connection open until any async * jobs have finished, then clean it up elsewhere */ -static void remoteClientFreeFunc(void *data) +void remoteClientFreeFunc(void *data) { struct daemonClientPrivate *priv = data; @@ -663,31 +663,28 @@ static void remoteClientCloseFunc(virNetServerClientPtr client) } -int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - void *opaque ATTRIBUTE_UNUSED) +void *remoteClientInitHook(virNetServerClientPtr client, + void *opaque ATTRIBUTE_UNUSED) { struct daemonClientPrivate *priv; int i; if (VIR_ALLOC(priv) < 0) { virReportOOMError(); - return -1; + return NULL; } if (virMutexInit(&priv->lock) < 0) { VIR_FREE(priv); virReportOOMError(); - return -1; + return NULL; } for (i = 0 ; i < VIR_DOMAIN_EVENT_ID_LAST ; i++) priv->domainEventCallbackID[i] = -1; - virNetServerClientSetPrivateData(client, priv, - remoteClientFreeFunc); virNetServerClientSetCloseHook(client, remoteClientCloseFunc); - return 0; + return priv; } /*----- Functions. -----*/ diff --git a/daemon/remote.h b/daemon/remote.h index 9f662cb132..2806494667 100644 --- a/daemon/remote.h +++ b/daemon/remote.h @@ -35,8 +35,8 @@ extern size_t remoteNProcs; extern virNetServerProgramProc qemuProcs[]; extern size_t qemuNProcs; -int remoteClientInitHook(virNetServerPtr srv, - virNetServerClientPtr client, - void *opaque); +void remoteClientFreeFunc(void *data); +void *remoteClientInitHook(virNetServerClientPtr client, + void *opaque); #endif /* __LIBVIRTD_REMOTE_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a859810a2c..e5d358220c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1524,7 +1524,6 @@ virNetServerClientSendMessage; virNetServerClientSetCloseHook; virNetServerClientSetDispatcher; virNetServerClientSetIdentity; -virNetServerClientSetPrivateData; virNetServerClientStartKeepAlive; virNetServerClientWantClose; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index cb9fa41294..4c3c17fe2c 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -578,16 +578,26 @@ static void virLXCControllerClientCloseHook(virNetServerClientPtr client) } } -static int virLXCControllerClientHook(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - void *opaque) +static void virLXCControllerClientPrivateFree(void *data) +{ + VIR_FREE(data); +} + +static void *virLXCControllerClientPrivateNew(virNetServerClientPtr client, + void *opaque) { virLXCControllerPtr ctrl = opaque; - virNetServerClientSetPrivateData(client, ctrl, NULL); + int *dummy; + + if (VIR_ALLOC(dummy) < 0) { + virReportOOMError(); + return NULL; + } + virNetServerClientSetCloseHook(client, virLXCControllerClientCloseHook); VIR_DEBUG("Got new client %p", client); ctrl->client = client; - return 0; + return dummy; } @@ -605,7 +615,8 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) if (!(ctrl->server = virNetServerNew(0, 0, 0, 1, -1, 0, false, NULL, - virLXCControllerClientHook, + virLXCControllerClientPrivateNew, + virLXCControllerClientPrivateFree, ctrl))) goto error; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index c3eb2e5d9c..03cf0b7475 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -104,8 +104,9 @@ struct _virNetServer { virNetServerAutoShutdownFunc autoShutdownFunc; void *autoShutdownOpaque; - virNetServerClientInitHook clientInitHook; - void *clientInitOpaque; + virNetServerClientPrivNew clientPrivNew; + virFreeCallback clientPrivFree; + void *clientPrivOpaque; }; @@ -281,16 +282,15 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc, virNetServerServiceGetAuth(svc), virNetServerServiceIsReadonly(svc), virNetServerServiceGetMaxRequests(svc), - virNetServerServiceGetTLSContext(svc)))) + virNetServerServiceGetTLSContext(svc), + srv->clientPrivNew, + srv->clientPrivFree, + srv->clientPrivOpaque))) goto error; if (virNetServerClientInit(client) < 0) goto error; - if (srv->clientInitHook && - srv->clientInitHook(srv, client, srv->clientInitOpaque) < 0) - goto error; - if (VIR_EXPAND_N(srv->clients, srv->nclients, 1) < 0) { virReportOOMError(); goto error; @@ -352,8 +352,9 @@ virNetServerPtr virNetServerNew(size_t min_workers, unsigned int keepaliveCount, bool keepaliveRequired, const char *mdnsGroupName, - virNetServerClientInitHook clientInitHook, - void *opaque) + virNetServerClientPrivNew clientPrivNew, + virFreeCallback clientPrivFree, + void *clientPrivOpaque) { virNetServerPtr srv; struct sigaction sig_action; @@ -376,8 +377,9 @@ virNetServerPtr virNetServerNew(size_t min_workers, srv->keepaliveCount = keepaliveCount; srv->keepaliveRequired = keepaliveRequired; srv->sigwrite = srv->sigread = -1; - srv->clientInitHook = clientInitHook; - srv->clientInitOpaque = opaque; + srv->clientPrivNew = clientPrivNew; + srv->clientPrivFree = clientPrivFree; + srv->clientPrivOpaque = clientPrivOpaque; srv->privileged = geteuid() == 0 ? true : false; if (mdnsGroupName && diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 7dc52ca8ca..918807297a 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -32,10 +32,6 @@ # include "virnetserverservice.h" # include "virobject.h" -typedef int (*virNetServerClientInitHook)(virNetServerPtr srv, - virNetServerClientPtr client, - void *opaque); - virNetServerPtr virNetServerNew(size_t min_workers, size_t max_workers, size_t priority_workers, @@ -44,8 +40,9 @@ virNetServerPtr virNetServerNew(size_t min_workers, unsigned int keepaliveCount, bool keepaliveRequired, const char *mdnsGroupName, - virNetServerClientInitHook clientInitHook, - void *opaque); + virNetServerClientPrivNew clientPrivNew, + virFreeCallback clientPrivFree, + void *clientPrivOpaque); typedef int (*virNetServerAutoShutdownFunc)(virNetServerPtr srv, void *opaque); diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 9f033c8841..c9703fd8c6 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -97,7 +97,7 @@ struct _virNetServerClient void *dispatchOpaque; void *privateData; - virNetServerClientFreeFunc privateDataFreeFunc; + virFreeCallback privateDataFreeFunc; virNetServerClientCloseFunc privateDataCloseFunc; virKeepAlivePtr keepalive; @@ -340,7 +340,10 @@ virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock, int auth, bool readonly, size_t nrequests_max, - virNetTLSContextPtr tls) + virNetTLSContextPtr tls, + virNetServerClientPrivNew privNew, + virFreeCallback privFree, + void *privOpaque) { virNetServerClientPtr client; @@ -378,6 +381,14 @@ virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock, } client->nrequests = 1; + if (privNew) { + if (!(client->privateData = privNew(client, privOpaque))) { + virObjectUnref(client); + goto error; + } + client->privateDataFreeFunc = privFree; + } + PROBE(RPC_SERVER_CLIENT_NEW, "client=%p sock=%p", client, client->sock); @@ -507,22 +518,6 @@ const char *virNetServerClientGetIdentity(virNetServerClientPtr client) return identity; } -void virNetServerClientSetPrivateData(virNetServerClientPtr client, - void *opaque, - virNetServerClientFreeFunc ff) -{ - virNetServerClientLock(client); - - if (client->privateData && - client->privateDataFreeFunc) - client->privateDataFreeFunc(client->privateData); - - client->privateData = opaque; - client->privateDataFreeFunc = ff; - - virNetServerClientUnlock(client); -} - void *virNetServerClientGetPrivateData(virNetServerClientPtr client) { diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index a1ff19b2d9..f950c61702 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -39,11 +39,17 @@ typedef int (*virNetServerClientFilterFunc)(virNetServerClientPtr client, virNetMessagePtr msg, void *opaque); +typedef void *(*virNetServerClientPrivNew)(virNetServerClientPtr client, + void *opaque); + virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock, int auth, bool readonly, size_t nrequests_max, - virNetTLSContextPtr tls); + virNetTLSContextPtr tls, + virNetServerClientPrivNew privNew, + virFreeCallback privFree, + void *privOpaque); int virNetServerClientAddFilter(virNetServerClientPtr client, virNetServerClientFilterFunc func, @@ -74,11 +80,6 @@ const char *virNetServerClientGetIdentity(virNetServerClientPtr client); int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, uid_t *uid, gid_t *gid, pid_t *pid); -typedef void (*virNetServerClientFreeFunc)(void *data); - -void virNetServerClientSetPrivateData(virNetServerClientPtr client, - void *opaque, - virNetServerClientFreeFunc ff); void *virNetServerClientGetPrivateData(virNetServerClientPtr client); typedef void (*virNetServerClientCloseFunc)(virNetServerClientPtr client);