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 <berrange@redhat.com>

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;

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. -----*/

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__ */

src/libvirt_private.syms

@@ -1524,7 +1524,6 @@ virNetServerClientSendMessage;
 virNetServerClientSetCloseHook;
 virNetServerClientSetDispatcher;
 virNetServerClientSetIdentity;
-virNetServerClientSetPrivateData;
 virNetServerClientStartKeepAlive;
 virNetServerClientWantClose;
 

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;
 

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 &&

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);
 

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)
 {

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);