]> xenbits.xensource.com Git - libvirt.git/commitdiff
rpc: Remove keepalive_required option
authorMartin Kletzander <mkletzan@redhat.com>
Mon, 20 Jul 2015 12:51:24 +0000 (14:51 +0200)
committerMartin Kletzander <mkletzan@redhat.com>
Mon, 10 Aug 2015 11:15:56 +0000 (13:15 +0200)
Since its introduction in 2011 (particularly in commit f4324e329275),
the option doesn't work.  It just effectively disables all incoming
connections.  That's because the client private data that contain the
'keepalive_supported' boolean, are initialized to zeroes so the bool is
false and the only other place where the bool is used is when checking
whether the client supports keepalive.  Thus, according to the server,
no client supports keepalive.

Removing this instead of fixing it is better because a) apparently
nobody ever tried it since 2011 (4 years without one month) and b) we
cannot know whether the client supports keepalive until we get a ping or
pong keepalive packet.  And that won't happen until after we dispatched
the ConnectOpen call.

Another two reasons would be c) the keepalive_required was tracked on
the server level, but keepalive_supported was in private data of the
client as well as the check that was made in the remote layer, thus
making all other instances of virNetServer miss this feature unless they
all implemented it for themselves and d) we can always add it back in
case there is a request and a use-case for it.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
20 files changed:
daemon/libvirtd-config.c
daemon/libvirtd-config.h
daemon/libvirtd.c
daemon/libvirtd.conf
daemon/libvirtd.h
daemon/remote.c
daemon/test_libvirtd.aug.in
src/libvirt_remote.syms
src/locking/lock_daemon.c
src/lxc/lxc_controller.c
src/rpc/virnetserver.c
src/rpc/virnetserver.h
tests/libvirtdconftest.c
tests/virnetdaemondata/input-data-no-keepalive-required.json [new file with mode: 0644]
tests/virnetdaemondata/output-data-admin-nomdns.json
tests/virnetdaemondata/output-data-anon-clients.json
tests/virnetdaemondata/output-data-initial-nomdns.json
tests/virnetdaemondata/output-data-initial.json
tests/virnetdaemondata/output-data-no-keepalive-required.json [new file with mode: 0644]
tests/virnetdaemontest.c

index 10dcc423d2db0dea7539ae111846b9d05aa3182a..c31c8b2e9ab538c65862b5432a64988e170924a3 100644 (file)
@@ -292,7 +292,6 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
 
     data->keepalive_interval = 5;
     data->keepalive_count = 5;
-    data->keepalive_required = 0;
 
     data->admin_min_workers = 5;
     data->admin_max_workers = 20;
@@ -302,7 +301,6 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
 
     data->admin_keepalive_interval = 5;
     data->admin_keepalive_count = 5;
-    data->admin_keepalive_required = 0;
 
     localhost = virGetHostname();
     if (localhost == NULL) {
@@ -471,11 +469,9 @@ daemonConfigLoadOptions(struct daemonConfig *data,
 
     GET_CONF_INT(conf, filename, keepalive_interval);
     GET_CONF_UINT(conf, filename, keepalive_count);
-    GET_CONF_UINT(conf, filename, keepalive_required);
 
     GET_CONF_INT(conf, filename, admin_keepalive_interval);
     GET_CONF_UINT(conf, filename, admin_keepalive_count);
-    GET_CONF_UINT(conf, filename, admin_keepalive_required);
 
     return 0;
 
index 9cdae1a0cb595aae353cf15794dd81bd9b9d397a..3e1971d67f05a4f9a8c14e7315984ae229280d21 100644 (file)
@@ -81,7 +81,6 @@ struct daemonConfig {
 
     int keepalive_interval;
     unsigned int keepalive_count;
-    int keepalive_required;
 
     int admin_min_workers;
     int admin_max_workers;
@@ -91,7 +90,6 @@ struct daemonConfig {
 
     int admin_keepalive_interval;
     unsigned int admin_keepalive_count;
-    int admin_keepalive_required;
 };
 
 
index 71db4a042c7fe3c596d439e7a3ec2addd7d83a03..250094bd21ddd51a15a7cedcfa3508a5ccc3ad4d 100644 (file)
@@ -1389,7 +1389,6 @@ int main(int argc, char **argv) {
                                 config->max_anonymous_clients,
                                 config->keepalive_interval,
                                 config->keepalive_count,
-                                !!config->keepalive_required,
                                 config->mdns_adv ? config->mdns_name : NULL,
                                 remoteClientInitHook,
                                 NULL,
@@ -1464,7 +1463,6 @@ int main(int argc, char **argv) {
                                    0,
                                    config->admin_keepalive_interval,
                                    config->admin_keepalive_count,
-                                   !!config->admin_keepalive_required,
                                    NULL,
                                    remoteAdmClientInitHook,
                                    NULL,
index ac06cdd7910366284a5c569864f93436fed3e2f8..514e6e4813bd894bf793b838e7d16e3949e239de 100644 (file)
 #
 #keepalive_interval = 5
 #keepalive_count = 5
+
 #
-# If set to 1, libvirtd will refuse to talk to clients that do not
-# support keepalive protocol.  Defaults to 0.
+# These configuration options are no longer used.  There is no way to
+# restrict such clients from connecting since they first need to
+# connect in order to ask for keepalive.
 #
 #keepalive_required = 1
+#admin_keepalive_required = 1
 
 # Keepalive settings for the admin interface
 #admin_keepalive_interval = 5
 #admin_keepalive_count = 5
-#
-#admin_keepalive_required = 1
index 8c1a904893ab39f7d9cebf1a239870b4874f45dc..efd4823ae18e699ff15ec3ea0d56b2dd6113e657 100644 (file)
@@ -72,7 +72,6 @@ struct daemonClientPrivate {
     virConnectPtr conn;
 
     daemonClientStreamPtr streams;
-    bool keepalive_supported;
 };
 
 /* Separate private data for admin connection */
index e9e2dcae80e0d78bf206ca3a3fa18690761622c8..3a3eb091308894cc087a7b1766e67867626d878d 100644 (file)
@@ -1290,7 +1290,7 @@ void *remoteClientInitHook(virNetServerClientPtr client,
 /*----- Functions. -----*/
 
 static int
-remoteDispatchConnectOpen(virNetServerPtr server,
+remoteDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED,
                           virNetServerClientPtr client,
                           virNetMessagePtr msg ATTRIBUTE_UNUSED,
                           virNetMessageErrorPtr rerr,
@@ -1309,12 +1309,6 @@ remoteDispatchConnectOpen(virNetServerPtr server,
         goto cleanup;
     }
 
-    if (virNetServerKeepAliveRequired(server) && !priv->keepalive_supported) {
-        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-                       _("keepalive support is required to connect"));
-        goto cleanup;
-    }
-
     name = args->name ? *args->name : NULL;
 
     /* If this connection arrived on a readonly socket, force
index 4921cbfb86b3d98c68646fdb5789841aa16b3be1..b0cb7eb1492027b4369a90ed206bc4841064b05b 100644 (file)
@@ -58,6 +58,6 @@ module Test_libvirtd =
         { "keepalive_interval" = "5" }
         { "keepalive_count" = "5" }
         { "keepalive_required" = "1" }
+        { "admin_keepalive_required" = "1" }
         { "admin_keepalive_interval" = "5" }
         { "admin_keepalive_count" = "5" }
-        { "admin_keepalive_required" = "1" }
index 6bfdcfa819bf2bc5210c688452866269307ffbb4..90a453c8be9c4c38f8d0477e7958f99fb6605e31 100644 (file)
@@ -101,7 +101,6 @@ virNetServerAddProgram;
 virNetServerAddService;
 virNetServerClose;
 virNetServerHasClients;
-virNetServerKeepAliveRequired;
 virNetServerNew;
 virNetServerNewPostExecRestart;
 virNetServerPreExecRestart;
index ecbe03a4c15460fc7384e0b866316b95646a3b3f..c03502459acdd74ea017012cad3e91e0092c00fd 100644 (file)
@@ -151,7 +151,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged)
 
     if (!(lockd->srv = virNetServerNew(1, 1, 0, config->max_clients,
                                        config->max_clients, -1, 0,
-                                       false, NULL,
+                                       NULL,
                                        virLockDaemonClientNew,
                                        virLockDaemonClientPreExecRestart,
                                        virLockDaemonClientFree,
index 110a55662be053e549415c5763a078073b3d4457..48a3597ed27438fc089243bbe220c07d7216a869 100644 (file)
@@ -925,7 +925,7 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl)
         return -1;
 
     if (!(srv = virNetServerNew(0, 0, 0, 1,
-                                0, -1, 0, false,
+                                0, -1, 0,
                                 NULL,
                                 virLXCControllerClientPrivateNew,
                                 NULL,
index 60a9714f60961f0187705d2e627b5976dab66388..80b5588bf3c993b33c1ddfab293e5a4b1d41f557 100644 (file)
@@ -69,7 +69,6 @@ struct _virNetServer {
 
     int keepaliveInterval;
     unsigned int keepaliveCount;
-    bool keepaliveRequired;
 
 #ifdef WITH_GNUTLS
     virNetTLSContextPtr tls;
@@ -312,7 +311,6 @@ virNetServerPtr virNetServerNew(size_t min_workers,
                                 size_t max_anonymous_clients,
                                 int keepaliveInterval,
                                 unsigned int keepaliveCount,
-                                bool keepaliveRequired,
                                 const char *mdnsGroupName,
                                 virNetServerClientPrivNew clientPrivNew,
                                 virNetServerClientPrivPreExecRestart clientPrivPreExecRestart,
@@ -338,7 +336,6 @@ virNetServerPtr virNetServerNew(size_t min_workers,
     srv->nclients_unauth_max = max_anonymous_clients;
     srv->keepaliveInterval = keepaliveInterval;
     srv->keepaliveCount = keepaliveCount;
-    srv->keepaliveRequired = keepaliveRequired;
     srv->clientPrivNew = clientPrivNew;
     srv->clientPrivPreExecRestart = clientPrivPreExecRestart;
     srv->clientPrivFree = clientPrivFree;
@@ -380,7 +377,6 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object,
     unsigned int max_anonymous_clients;
     unsigned int keepaliveInterval;
     unsigned int keepaliveCount;
-    bool keepaliveRequired;
     const char *mdnsGroupName = NULL;
 
     if (virJSONValueObjectGetNumberUint(object, "min_workers", &min_workers) < 0) {
@@ -423,11 +419,6 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object,
                        _("Missing keepaliveCount data in JSON document"));
         goto error;
     }
-    if (virJSONValueObjectGetBoolean(object, "keepaliveRequired", &keepaliveRequired) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Missing keepaliveRequired data in JSON document"));
-        goto error;
-    }
 
     if (virJSONValueObjectHasKey(object, "mdnsGroupName") &&
         (!(mdnsGroupName = virJSONValueObjectGetString(object, "mdnsGroupName")))) {
@@ -440,7 +431,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object,
                                 priority_workers, max_clients,
                                 max_anonymous_clients,
                                 keepaliveInterval, keepaliveCount,
-                                keepaliveRequired, mdnsGroupName,
+                                mdnsGroupName,
                                 clientPrivNew, clientPrivPreExecRestart,
                                 clientPrivFree, clientPrivOpaque)))
         goto error;
@@ -573,11 +564,6 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv)
                        _("Cannot set keepaliveCount data in JSON document"));
         goto error;
     }
-    if (virJSONValueObjectAppendBoolean(object, "keepaliveRequired", srv->keepaliveRequired) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Cannot set keepaliveRequired data in JSON document"));
-        goto error;
-    }
 
     if (srv->mdnsGroupName &&
         virJSONValueObjectAppendString(object, "mdnsGroupName", srv->mdnsGroupName) < 0) {
@@ -786,15 +772,6 @@ void virNetServerClose(virNetServerPtr srv)
     virObjectUnlock(srv);
 }
 
-bool virNetServerKeepAliveRequired(virNetServerPtr srv)
-{
-    bool required;
-    virObjectLock(srv);
-    required = srv->keepaliveRequired;
-    virObjectUnlock(srv);
-    return required;
-}
-
 static inline size_t
 virNetServerTrackPendingAuthLocked(virNetServerPtr srv)
 {
index 0e16e8fb1bf0124914a16821a3209e0bd1571603..89d8db9b9ee40895325dce831f8d6386efda23b6 100644 (file)
@@ -41,7 +41,6 @@ virNetServerPtr virNetServerNew(size_t min_workers,
                                 size_t max_anonymous_clients,
                                 int keepaliveInterval,
                                 unsigned int keepaliveCount,
-                                bool keepaliveRequired,
                                 const char *mdnsGroupName,
                                 virNetServerClientPrivNew clientPrivNew,
                                 virNetServerClientPrivPreExecRestart clientPrivPreExecRestart,
@@ -74,8 +73,6 @@ int virNetServerSetTLSContext(virNetServerPtr srv,
                               virNetTLSContextPtr tls);
 # endif
 
-bool virNetServerKeepAliveRequired(virNetServerPtr srv);
-
 size_t virNetServerTrackPendingAuth(virNetServerPtr srv);
 size_t virNetServerTrackCompletedAuth(virNetServerPtr srv);
 
index d589d51cef302de162488c497aa766b240276cf9..61d861db1d1c941dc17047117235af51441ba045 100644 (file)
@@ -227,7 +227,9 @@ mymain(void)
     for (i = 0; params[i] != 0; i++) {
         const struct testCorruptData data = { params, filedata, filename, i };
         /* Skip now ignored config param */
-        if (STRPREFIX(filedata + params[i], "log_buffer_size"))
+        if (STRPREFIX(filedata + params[i], "log_buffer_size") ||
+            STRPREFIX(filedata + params[i], "keepalive_required") ||
+            STRPREFIX(filedata + params[i], "admin_keepalive_required"))
             continue;
         if (virtTestRun("Test corruption", testCorrupt, &data) < 0)
             ret = -1;
diff --git a/tests/virnetdaemondata/input-data-no-keepalive-required.json b/tests/virnetdaemondata/input-data-no-keepalive-required.json
new file mode 100644 (file)
index 0000000..b5e4dc8
--- /dev/null
@@ -0,0 +1,124 @@
+{
+    "servers": [
+       {
+           "min_workers": 10,
+           "max_workers": 50,
+           "priority_workers": 5,
+           "max_clients": 100,
+           "keepaliveInterval": 120,
+           "keepaliveCount": 5,
+           "services": [
+               {
+                   "auth": 0,
+                   "readonly": true,
+                   "nrequests_client_max": 2,
+                   "socks": [
+                       {
+                           "fd": 100,
+                           "errfd": -1,
+                           "pid": 0,
+                           "isClient": false
+                       }
+                   ]
+               },
+               {
+                   "auth": 2,
+                   "readonly": false,
+                   "nrequests_client_max": 5,
+                   "socks": [
+                       {
+                           "fd": 101,
+                           "errfd": -1,
+                           "pid": 0,
+                           "isClient": false
+                       }
+                   ]
+               }
+           ],
+           "clients": [
+               {
+                   "auth": 1,
+                   "readonly": true,
+                   "nrequests_max": 15,
+                   "sock": {
+                       "fd": 102,
+                       "errfd": -1,
+                       "pid": -1,
+                       "isClient": true
+                   }
+               },
+               {
+                   "auth": 2,
+                   "readonly": true,
+                   "nrequests_max": 66,
+                   "sock": {
+                       "fd": 103,
+                       "errfd": -1,
+                       "pid": -1,
+                       "isClient": true
+                   }
+               }
+           ]
+       },
+       {
+           "min_workers": 2,
+           "max_workers": 50,
+           "priority_workers": 5,
+           "max_clients": 100,
+           "keepaliveInterval": 120,
+           "keepaliveCount": 5,
+           "services": [
+               {
+                   "auth": 0,
+                   "readonly": true,
+                   "nrequests_client_max": 2,
+                   "socks": [
+                       {
+                           "fd": 100,
+                           "errfd": -1,
+                           "pid": 0,
+                           "isClient": false
+                       }
+                   ]
+               },
+               {
+                   "auth": 2,
+                   "readonly": false,
+                   "nrequests_client_max": 5,
+                   "socks": [
+                       {
+                           "fd": 101,
+                           "errfd": -1,
+                           "pid": 0,
+                           "isClient": false
+                       }
+                   ]
+               }
+           ],
+           "clients": [
+               {
+                   "auth": 1,
+                   "readonly": true,
+                   "nrequests_max": 15,
+                   "sock": {
+                       "fd": 102,
+                       "errfd": -1,
+                       "pid": -1,
+                       "isClient": true
+                   }
+               },
+               {
+                   "auth": 2,
+                   "readonly": true,
+                   "nrequests_max": 66,
+                   "sock": {
+                       "fd": 103,
+                       "errfd": -1,
+                       "pid": -1,
+                       "isClient": true
+                   }
+               }
+           ]
+       }
+    ]
+}
index 5df71a0d88c8a1162cc9f3d5f8485d7c174c9718..a814aeb806142190f6586b10e336128e9748e0b5 100644 (file)
@@ -8,7 +8,6 @@
             "max_anonymous_clients": 100,
             "keepaliveInterval": 120,
             "keepaliveCount": 5,
-            "keepaliveRequired": true,
             "services": [
                 {
                     "auth": 0,
@@ -70,7 +69,6 @@
             "max_anonymous_clients": 100,
             "keepaliveInterval": 120,
             "keepaliveCount": 5,
-            "keepaliveRequired": true,
             "services": [
                 {
                     "auth": 0,
index 4e4332691aa7227fceb7233dec145f02650e05a0..05fc0ae00d3fd0f57edde6f4ab9633a55aae5c26 100644 (file)
@@ -8,7 +8,6 @@
             "max_anonymous_clients": 10,
             "keepaliveInterval": 120,
             "keepaliveCount": 5,
-            "keepaliveRequired": true,
             "services": [
                 {
                     "auth": 0,
index bef54bf94ad559b3d2dd035f5971746cf9e63103..400e47bc94636c288360e3177c37da3fa1a23511 100644 (file)
@@ -8,7 +8,6 @@
             "max_anonymous_clients": 100,
             "keepaliveInterval": 120,
             "keepaliveCount": 5,
-            "keepaliveRequired": true,
             "services": [
                 {
                     "auth": 0,
index 9afa791d91fca427cf298f240e9b38e13e37b4fc..e875cffe5c01e94c1949cca15c6ab09d08a2da3f 100644 (file)
@@ -8,7 +8,6 @@
             "max_anonymous_clients": 100,
             "keepaliveInterval": 120,
             "keepaliveCount": 5,
-            "keepaliveRequired": true,
             "mdnsGroupName": "libvirtTest",
             "services": [
                 {
diff --git a/tests/virnetdaemondata/output-data-no-keepalive-required.json b/tests/virnetdaemondata/output-data-no-keepalive-required.json
new file mode 100644 (file)
index 0000000..b5e4dc8
--- /dev/null
@@ -0,0 +1,124 @@
+{
+    "servers": [
+       {
+           "min_workers": 10,
+           "max_workers": 50,
+           "priority_workers": 5,
+           "max_clients": 100,
+           "keepaliveInterval": 120,
+           "keepaliveCount": 5,
+           "services": [
+               {
+                   "auth": 0,
+                   "readonly": true,
+                   "nrequests_client_max": 2,
+                   "socks": [
+                       {
+                           "fd": 100,
+                           "errfd": -1,
+                           "pid": 0,
+                           "isClient": false
+                       }
+                   ]
+               },
+               {
+                   "auth": 2,
+                   "readonly": false,
+                   "nrequests_client_max": 5,
+                   "socks": [
+                       {
+                           "fd": 101,
+                           "errfd": -1,
+                           "pid": 0,
+                           "isClient": false
+                       }
+                   ]
+               }
+           ],
+           "clients": [
+               {
+                   "auth": 1,
+                   "readonly": true,
+                   "nrequests_max": 15,
+                   "sock": {
+                       "fd": 102,
+                       "errfd": -1,
+                       "pid": -1,
+                       "isClient": true
+                   }
+               },
+               {
+                   "auth": 2,
+                   "readonly": true,
+                   "nrequests_max": 66,
+                   "sock": {
+                       "fd": 103,
+                       "errfd": -1,
+                       "pid": -1,
+                       "isClient": true
+                   }
+               }
+           ]
+       },
+       {
+           "min_workers": 2,
+           "max_workers": 50,
+           "priority_workers": 5,
+           "max_clients": 100,
+           "keepaliveInterval": 120,
+           "keepaliveCount": 5,
+           "services": [
+               {
+                   "auth": 0,
+                   "readonly": true,
+                   "nrequests_client_max": 2,
+                   "socks": [
+                       {
+                           "fd": 100,
+                           "errfd": -1,
+                           "pid": 0,
+                           "isClient": false
+                       }
+                   ]
+               },
+               {
+                   "auth": 2,
+                   "readonly": false,
+                   "nrequests_client_max": 5,
+                   "socks": [
+                       {
+                           "fd": 101,
+                           "errfd": -1,
+                           "pid": 0,
+                           "isClient": false
+                       }
+                   ]
+               }
+           ],
+           "clients": [
+               {
+                   "auth": 1,
+                   "readonly": true,
+                   "nrequests_max": 15,
+                   "sock": {
+                       "fd": 102,
+                       "errfd": -1,
+                       "pid": -1,
+                       "isClient": true
+                   }
+               },
+               {
+                   "auth": 2,
+                   "readonly": true,
+                   "nrequests_max": 66,
+                   "sock": {
+                       "fd": 103,
+                       "errfd": -1,
+                       "pid": -1,
+                       "isClient": true
+                   }
+               }
+           ]
+       }
+    ]
+}
index ef45018f5873c1f93956a48957b5e9c13bf9a573..fb8a6c0c0ec54756a52324e83dd791e043b21e59 100644 (file)
@@ -50,7 +50,7 @@ testCreateServer(const char *host, int family)
     }
 
     if (!(srv = virNetServerNew(10, 50, 5, 100, 10,
-                                120, 5, true,
+                                120, 5,
                                 mdns_group,
                                 NULL,
                                 NULL,