[PATCH v2] RPC: Don't accept client if it would overcommit max_clients

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

Currently, even if max_client limit is hit, we accept() incoming
connection request, but close it immediately. This has disadvantage of
not using listen() queue. We should accept() only those clients we
know we can serve and let all other wait in the (limited) queue.
---

diff to v1:
-moved logic from virNetServerDispatchCheck to virNetServerAddClient

 src/rpc/virnetserver.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index cb770c3..2306e10 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -115,6 +115,8 @@ struct _virNetServer {
 
 static virClassPtr virNetServerClass;
 static void virNetServerDispose(void *obj);
+static void virNetServerUpdateServicesLocked(virNetServerPtr srv,
+                                             bool enabled);
 
 static int virNetServerOnceInit(void)
 {
@@ -253,30 +255,36 @@ cleanup:
 static int virNetServerAddClient(virNetServerPtr srv,
                                  virNetServerClientPtr client)
 {
     virObjectLock(srv);
 
     if (srv->nclients >= srv->nclients_max) {
         virReportError(VIR_ERR_RPC,
                        _("Too many active clients (%zu), dropping connection from %s"),
                        srv->nclients_max, virNetServerClientRemoteAddrString(client));
         goto error;
     }
 
     if (virNetServerClientInit(client) < 0)
         goto error;
 
     if (VIR_EXPAND_N(srv->clients, srv->nclients, 1) < 0)
         goto error;
     srv->clients[srv->nclients-1] = client;
     virObjectRef(client);
 
+    if (srv->nclients == srv->nclients_max) {
+        /* Temporarily stop accepting new clients */
+        VIR_DEBUG("Temporarily suspending services due to max_clients");
+        virNetServerUpdateServicesLocked(srv, false);
+    }
+
     virNetServerClientSetDispatcher(client,
                                     virNetServerDispatchNewMessage,
                                     srv);
 
     virNetServerClientInitKeepAlive(client, srv->keepaliveInterval,
                                     srv->keepaliveCount);
 
     virObjectUnlock(srv);
     return 0;
 
@@ -1034,98 +1042,113 @@ static void virNetServerAutoShutdownTimer(int timerid ATTRIBUTE_UNUSED,
 }
 
 
+static void
+virNetServerUpdateServicesLocked(virNetServerPtr srv,
+                                 bool enabled)
+{
+    size_t i;
+
+    for (i = 0; i < srv->nservices; i++)
+        virNetServerServiceToggle(srv->services[i], enabled);
+}
+
+
 void virNetServerUpdateServices(virNetServerPtr srv,
                                 bool enabled)
 {
-    size_t i;
-
     virObjectLock(srv);
-    for (i = 0; i < srv->nservices; i++)
-        virNetServerServiceToggle(srv->services[i], enabled);
-
+    virNetServerUpdateServicesLocked(srv, enabled);
     virObjectUnlock(srv);
 }
 
 
 void virNetServerRun(virNetServerPtr srv)
 {
     int timerid = -1;
     bool timerActive = false;
     size_t i;
 
     virObjectLock(srv);
 
     if (srv->mdns &&
         virNetServerMDNSStart(srv->mdns) < 0)
         goto cleanup;
 
     srv->quit = 0;
 
     if (srv->autoShutdownTimeout &&
         (timerid = virEventAddTimeout(-1,
                                       virNetServerAutoShutdownTimer,
                                       srv, NULL)) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Failed to register shutdown timeout"));
         goto cleanup;
     }
 
     VIR_DEBUG("srv=%p quit=%d", srv, srv->quit);
     while (!srv->quit) {
         /* A shutdown timeout is specified, so check
          * if any drivers have active state, if not
          * shutdown after timeout seconds
          */
         if (srv->autoShutdownTimeout) {
             if (timerActive) {
                 if (srv->clients) {
                     VIR_DEBUG("Deactivating shutdown timer %d", timerid);
                     virEventUpdateTimeout(timerid, -1);
                     timerActive = false;
                 }
             } else {
                 if (!srv->clients) {
                     VIR_DEBUG("Activating shutdown timer %d", timerid);
                     virEventUpdateTimeout(timerid,
                                           srv->autoShutdownTimeout * 1000);
                     timerActive = true;
                 }
             }
         }
 
         virObjectUnlock(srv);
         if (virEventRunDefaultImpl() < 0) {
             virObjectLock(srv);
             VIR_DEBUG("Loop iteration error, exiting");
             break;
         }
         virObjectLock(srv);
 
     reprocess:
         for (i = 0; i < srv->nclients; i++) {
             /* Coverity 5.3.0 couldn't see that srv->clients is non-NULL
              * if srv->nclients is non-zero.  */
             sa_assert(srv->clients);
             if (virNetServerClientWantClose(srv->clients[i]))
                 virNetServerClientClose(srv->clients[i]);
             if (virNetServerClientIsClosed(srv->clients[i])) {
                 virNetServerClientPtr client = srv->clients[i];
                 if (srv->nclients > 1) {
                     memmove(srv->clients + i,
                             srv->clients + i + 1,
                             sizeof(*srv->clients) * (srv->nclients - (i + 1)));
                     VIR_SHRINK_N(srv->clients, srv->nclients, 1);
                 } else {
                     VIR_FREE(srv->clients);
                     srv->nclients = 0;
                 }
 
+                /* Enable services if we can accept a new client.
+                 * The new client can be accepted if we are at the limit. */
+                if (srv->nclients == srv->nclients_max - 1) {
+                    /* Now it makes sense to accept() a new client. */
+                    VIR_DEBUG("Re-enabling services");
+                    virNetServerUpdateServicesLocked(srv, true);
+                }
+
                 virObjectUnlock(srv);
                 virObjectUnref(client);
                 virObjectLock(srv);
 
                 goto reprocess;
             }
         }
     }
 
-- 
1.8.1.5

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list