remote: fix handling of systemd activation wrt socket ordering

The current libvirtd code for systemd socket activation assumes socket
FDs are passed in the order unix-rw, unix-ro, unix-admin.  There is in
fact no ordering guarantee made by systemd. Applications are expected
to check the address or name associated with each FD to figure out its
identity.

This rewrites libvirtd to make use of the new systemd activation APIs
to make it robust wrt socket ordering changes.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

src/remote/remote_daemon.c

@@ -56,6 +56,7 @@
 #include "virutil.h"
 #include "virgettext.h"
 #include "util/virnetdevopenvswitch.h"
+#include "virsystemd.h"
 
 #include "driver.h"
 
@@ -367,30 +368,34 @@ daemonSetupNetworking(virNetServerPtr srv,
                       bool ipsock,
                       bool privileged)
 {
-    virNetServerServicePtr svc = NULL;
-    virNetServerServicePtr svcAdm = NULL;
-    virNetServerServicePtr svcRO = NULL;
-    virNetServerServicePtr svcTCP = NULL;
-    virNetServerServicePtr svcTLS = NULL;
     gid_t unix_sock_gid = 0;
     int unix_sock_ro_mask = 0;
     int unix_sock_rw_mask = 0;
     int unix_sock_adm_mask = 0;
     int ret = -1;
+    VIR_AUTOPTR(virSystemdActivation) act = NULL;
+    virSystemdActivationMap actmap[] = {
+        { .name = "libvirtd.socket", .family = AF_UNIX, .path = sock_path },
+        { .name = "libvirtd-ro.socket", .family = AF_UNIX, .path = sock_path_ro },
+        { .name = "libvirtd-admin.socket", .family = AF_UNIX, .path = sock_path_adm },
+        { .name = "libvirtd-tcp.socket", .family = AF_INET },
+        { .name = "libvirtd-tls.socket", .family = AF_INET },
+    };
 
-    unsigned int cur_fd = STDERR_FILENO + 1;
+    if ((actmap[3].port = virSocketAddrResolveService(config->tcp_port)) < 0)
-    unsigned int nfds = virGetListenFDs();
+        return -1;
+
+    if ((actmap[4].port = virSocketAddrResolveService(config->tls_port)) < 0)
+        return -1;
+
+    if (virSystemdGetActivation(actmap, ARRAY_CARDINALITY(actmap), &act) < 0)
+        return -1;
 
     if (config->unix_sock_group) {
         if (virGetGroupID(config->unix_sock_group, &unix_sock_gid) < 0)
             return ret;
     }
 
-    if (nfds > (sock_path_ro ? 2 : 1)) {
-        VIR_ERROR(_("Too many (%u) FDs passed from caller"), nfds);
-        return ret;
-    }
-
     if (virStrToLong_i(config->unix_sock_ro_perms, NULL, 8, &unix_sock_ro_mask) != 0) {
         VIR_ERROR(_("Failed to parse mode '%s'"), config->unix_sock_ro_perms);
         goto cleanup;
@@ -406,71 +411,61 @@ daemonSetupNetworking(virNetServerPtr srv,
         goto cleanup;
     }
 
-    if (!(svc = virNetServerServiceNewFDOrUNIX(sock_path,
+    if (virNetServerAddServiceUNIX(srv,
+                                   act,
+                                   "libvirtd.socket",
+                                   sock_path,
                                    unix_sock_rw_mask,
                                    unix_sock_gid,
                                    config->auth_unix_rw,
                                    NULL,
                                    false,
                                    config->max_queued_clients,
-                                               config->max_client_requests,
+                                   config->max_client_requests) < 0)
-                                               nfds, &cur_fd)))
         goto cleanup;
-    if (sock_path_ro) {
+    if (sock_path_ro &&
-        if (!(svcRO = virNetServerServiceNewFDOrUNIX(sock_path_ro,
+        virNetServerAddServiceUNIX(srv,
+                                   act,
+                                   "libvirtd-ro.socket",
+                                   sock_path_ro,
                                    unix_sock_ro_mask,
                                    unix_sock_gid,
                                    config->auth_unix_ro,
                                    NULL,
                                    true,
                                    config->max_queued_clients,
-                                                     config->max_client_requests,
+                                   config->max_client_requests) < 0)
-                                                     nfds, &cur_fd)))
-            goto cleanup;
-    }
-
-    if (virNetServerAddService(srv, svc) < 0)
         goto cleanup;
 
-    if (svcRO &&
+    if (sock_path_adm &&
-        virNetServerAddService(srv, svcRO) < 0)
+        virNetServerAddServiceUNIX(srvAdm,
-        goto cleanup;
+                                   act,
-
+                                   "libvirtd-admin.socket",
-    if (sock_path_adm) {
+                                   sock_path_adm,
-        VIR_DEBUG("Registering unix socket %s", sock_path_adm);
-        if (!(svcAdm = virNetServerServiceNewUNIX(sock_path_adm,
                                    unix_sock_adm_mask,
                                    unix_sock_gid,
                                    REMOTE_AUTH_NONE,
                                    NULL,
                                    false,
                                    config->admin_max_queued_clients,
-                                                  config->admin_max_client_requests)))
+                                   config->admin_max_client_requests) < 0)
         goto cleanup;
 
-        if (virNetServerAddService(srvAdm, svcAdm) < 0)
+    if (((ipsock && config->listen_tcp) || act) &&
-            goto cleanup;
+        virNetServerAddServiceTCP(srv,
-    }
+                                  act,
-
+                                  "libvirtd-tcp.socket",
-    if (ipsock) {
+                                  config->listen_addr,
-        if (config->listen_tcp) {
-            VIR_DEBUG("Registering TCP socket %s:%s",
-                      config->listen_addr, config->tcp_port);
-            if (!(svcTCP = virNetServerServiceNewTCP(config->listen_addr,
                                   config->tcp_port,
                                   AF_UNSPEC,
                                   config->auth_tcp,
                                   NULL,
                                   false,
                                   config->max_queued_clients,
-                                                     config->max_client_requests)))
+                                  config->max_client_requests) < 0)
         goto cleanup;
 
-            if (virNetServerAddService(srv, svcTCP) < 0)
+    if (((ipsock && config->listen_tls) || (act && virSystemdActivationHasName(act, "ip-tls")))) {
-                goto cleanup;
-        }
-
-        if (config->listen_tls) {
         virNetTLSContextPtr ctxt = NULL;
 
         if (config->ca_file ||
@@ -514,24 +509,26 @@ daemonSetupNetworking(virNetServerPtr srv,
 
         VIR_DEBUG("Registering TLS socket %s:%s",
                   config->listen_addr, config->tls_port);
-            if (!(svcTLS =
+        if (virNetServerAddServiceTCP(srv,
-                  virNetServerServiceNewTCP(config->listen_addr,
+                                      act,
+                                      "libvirtd-tls.socket",
+                                      config->listen_addr,
                                       config->tls_port,
                                       AF_UNSPEC,
                                       config->auth_tls,
                                       ctxt,
                                       false,
                                       config->max_queued_clients,
-                                            config->max_client_requests))) {
+                                      config->max_client_requests) < 0) {
             virObjectUnref(ctxt);
             goto cleanup;
         }
-            if (virNetServerAddService(srv, svcTLS) < 0)
+        virObjectUnref(ctxt);
-                goto cleanup;
+    }
 
-            virObjectUnref(ctxt);
+    if (act &&
-        }
+        virSystemdActivationComplete(act) < 0)
-    }
+        goto cleanup;
 
 #if WITH_SASL
     if (virNetServerNeedsAuth(srv, REMOTE_AUTH_SASL) &&
@@ -543,11 +540,6 @@ daemonSetupNetworking(virNetServerPtr srv,
     ret = 0;
 
  cleanup:
-    virObjectUnref(svcTLS);
-    virObjectUnref(svcTCP);
-    virObjectUnref(svcRO);
-    virObjectUnref(svcAdm);
-    virObjectUnref(svc);
     return ret;
 }
 

src/rpc/virnetserverservice.c

@@ -28,9 +28,12 @@
 #include "viralloc.h"
 #include "virerror.h"
 #include "virthread.h"
+#include "virlog.h"
 
 #define VIR_FROM_THIS VIR_FROM_RPC
 
+VIR_LOG_INIT("rpc.netserverservice");
+
 struct _virNetServerService {
     virObject parent;
 
@@ -201,6 +204,8 @@ virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename,
     virNetSocketPtr *socks;
     size_t nsocks;
 
+    VIR_DEBUG("Creating new TCP server nodename='%s' service='%s'",
+              NULLSTR(nodename), NULLSTR(service));
     if (virNetSocketNewListenTCP(nodename,
                                  service,
                                  family,
@@ -236,6 +241,8 @@ virNetServerServicePtr virNetServerServiceNewUNIX(const char *path,
     virNetServerServicePtr svc;
     virNetSocketPtr sock;
 
+    VIR_DEBUG("Creating new UNIX server path='%s' mask=%o gid=%u",
+              path, mask, grp);
     if (virNetSocketNewListenUNIX(path,
                                   mask,
                                   -1,