Change interaction when accepting new RPC client connections

Currently the virNetServerServicePtr is responsible for
creating the virNetServerClientPtr instance when accepting
a new connection. Change this so that the virNetServerServicePtr
merely gives virNetServerPtr a virNetSocketPtr instance. The
virNetServerPtr can then create the virNetServerClientPtr
as it desires

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

src/libvirt_private.syms

@@ -1492,8 +1492,6 @@ virNetServerKeepAliveRequired;
 virNetServerNew;
 virNetServerQuit;
 virNetServerRun;
-virNetServerServiceNewTCP;
-virNetServerServiceNewUNIX;
 virNetServerSetTLSContext;
 virNetServerUpdateServices;
 
@@ -1558,7 +1556,9 @@ virNetServerProgramUnknownError;
 # virnetserverservice.h
 virNetServerServiceClose;
 virNetServerServiceGetAuth;
+virNetServerServiceGetMaxRequests;
 virNetServerServiceGetPort;
+virNetServerServiceGetTLSContext;
 virNetServerServiceIsReadonly;
 virNetServerServiceNewTCP;
 virNetServerServiceNewUNIX;

src/rpc/virnetserver.c

@@ -261,11 +261,12 @@ cleanup:
 }
 
 
-static int virNetServerDispatchNewClient(virNetServerServicePtr svc ATTRIBUTE_UNUSED,
+static int virNetServerDispatchNewClient(virNetServerServicePtr svc,
-                                         virNetServerClientPtr client,
+                                         virNetSocketPtr clientsock,
                                          void *opaque)
 {
     virNetServerPtr srv = opaque;
+    virNetServerClientPtr client = NULL;
 
     virNetServerLock(srv);
 
@@ -276,6 +277,13 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc ATTRIBUTE_UN
         goto error;
     }
 
+    if (!(client = virNetServerClientNew(clientsock,
+                                         virNetServerServiceGetAuth(svc),
+                                         virNetServerServiceIsReadonly(svc),
+                                         virNetServerServiceGetMaxRequests(svc),
+                                         virNetServerServiceGetTLSContext(svc))))
+        goto error;
+
     if (virNetServerClientInit(client) < 0)
         goto error;
 
@@ -301,6 +309,8 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc ATTRIBUTE_UN
     return 0;
 
 error:
+    virNetServerClientClose(client);
+    virObjectUnref(client);
     virNetServerUnlock(srv);
     return -1;
 }

src/rpc/virnetserverclient.c

@@ -357,7 +357,7 @@ virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock,
         return NULL;
     }
 
-    client->sock = sock;
+    client->sock = virObjectRef(sock);
     client->auth = auth;
     client->readonly = readonly;
     client->tlsCtxt = virObjectRef(tls);
@@ -385,8 +385,6 @@ virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock,
     return client;
 
 error:
-    /* XXX ref counting is better than this */
-    client->sock = NULL; /* Caller owns 'sock' upon failure */
     virObjectUnref(client);
     return NULL;
 }

src/rpc/virnetserverservice.c

@@ -69,40 +69,21 @@ static void virNetServerServiceAccept(virNetSocketPtr sock,
                                       void *opaque)
 {
     virNetServerServicePtr svc = opaque;
-    virNetServerClientPtr client = NULL;
     virNetSocketPtr clientsock = NULL;
 
     if (virNetSocketAccept(sock, &clientsock) < 0)
-        goto error;
+        goto cleanup;
 
     if (!clientsock) /* Connection already went away */
         goto cleanup;
 
-    if (!(client = virNetServerClientNew(clientsock,
-                                         svc->auth,
-                                         svc->readonly,
-                                         svc->nrequests_client_max,
-                                         svc->tls)))
-        goto error;
-
     if (!svc->dispatchFunc)
-        goto error;
+        goto cleanup;
 
-    if (svc->dispatchFunc(svc, client, svc->dispatchOpaque) < 0)
+    svc->dispatchFunc(svc, clientsock, svc->dispatchOpaque);
-        virNetServerClientClose(client);
-
-    virObjectUnref(client);
 
 cleanup:
-    return;
+    virObjectUnref(clientsock);
-
-error:
-    if (client) {
-        virNetServerClientClose(client);
-        virObjectUnref(client);
-    } else {
-        virObjectUnref(clientsock);
-    }
 }
 
 
@@ -240,6 +221,17 @@ bool virNetServerServiceIsReadonly(virNetServerServicePtr svc)
 }
 
 
+size_t virNetServerServiceGetMaxRequests(virNetServerServicePtr svc)
+{
+    return svc->nrequests_client_max;
+}
+
+virNetTLSContextPtr virNetServerServiceGetTLSContext(virNetServerServicePtr svc)
+{
+    return svc->tls;
+}
+
+
 void virNetServerServiceSetDispatcher(virNetServerServicePtr svc,
                                       virNetServerServiceDispatchFunc func,
                                       void *opaque)

src/rpc/virnetserverservice.h

@@ -34,7 +34,7 @@ enum {
 };
 
 typedef int (*virNetServerServiceDispatchFunc)(virNetServerServicePtr svc,
-                                               virNetServerClientPtr client,
+                                               virNetSocketPtr sock,
                                                void *opaque);
 
 virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename,
@@ -55,6 +55,8 @@ int virNetServerServiceGetPort(virNetServerServicePtr svc);
 
 int virNetServerServiceGetAuth(virNetServerServicePtr svc);
 bool virNetServerServiceIsReadonly(virNetServerServicePtr svc);
+size_t virNetServerServiceGetMaxRequests(virNetServerServicePtr svc);
+virNetTLSContextPtr virNetServerServiceGetTLSContext(virNetServerServicePtr svc);
 
 void virNetServerServiceSetDispatcher(virNetServerServicePtr svc,
                                       virNetServerServiceDispatchFunc func,