The systemd version in RHEL-7 lacked support for the LISTEN_FDNAMES env variable with socket activation. Since we stopped targetting RHEL-7 we can drop some considerable amount of compatibility code. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- docs/daemons.rst | 26 ------- src/locking/lock_daemon.c | 8 +- src/logging/log_daemon.c | 8 +- src/remote/libvirtd-admin.socket.in | 2 - src/remote/libvirtd-ro.socket.in | 2 - src/remote/libvirtd-tcp.socket.in | 2 - src/remote/libvirtd-tls.socket.in | 2 - src/remote/libvirtd.conf.in | 6 +- src/remote/libvirtd.socket.in | 2 - src/remote/remote_daemon.c | 19 +---- src/util/virsystemd.c | 117 +++------------------------- src/util/virsystemd.h | 15 +--- tests/virsystemdtest.c | 38 +-------- 13 files changed, 21 insertions(+), 226 deletions(-) diff --git a/docs/daemons.rst b/docs/daemons.rst index c3970eb89c..383f0f64ff 100644 --- a/docs/daemons.rst +++ b/docs/daemons.rst @@ -209,13 +209,6 @@ controlled via the system unit files ``libvirtd.socket``, ``libvirtd-ro.socket`` and ``libvirtd-admin.socket`` unit files. -Systemd releases prior to version 227 lacked support for passing the activation -socket unit names into the service. When using these old versions, the -``tcp_port``, ``tls_port`` and ``unix_sock_dir`` settings in ``libvirtd.conf`` -must be changed in lock-step with the equivalent settings in the unit files to -ensure that ``libvirtd`` can identify the sockets. - - Modular driver daemons ====================== @@ -354,13 +347,6 @@ controlled via the system unit files: ``virt${DRIVER}d.socket``, ``virt${DRIVER}d-ro.socket`` and ``virt${DRIVER}d-admin.socket`` unit files. -Systemd releases prior to version 227 lacked support for passing the activation -socket unit names into the service. When using these old versions, the -``unix_sock_dir`` setting in ``virt${DRIVER}d.conf`` must be changed in -lock-step with the equivalent setting in the unit files to ensure that -``virt${DRIVER}d`` can identify the sockets. - - Switching to modular daemons ---------------------------- @@ -639,12 +625,6 @@ controlled via the system unit files: independently controlled via the ``ListenStream`` parameter in any of the ``virtlogd.socket`` and ``virtlogd-admin.socket`` unit files. -Systemd releases prior to version 227 lacked support for passing the activation -socket unit names into the service. When using these old versions, the -``unix_sock_dir`` setting in ``virtlogd.conf`` must be changed in -lock-step with the equivalent setting in the unit files to ensure that -``virtlogd`` can identify the sockets. - Locking daemon ============== @@ -733,12 +713,6 @@ controlled via the system unit files: independently controlled via the ``ListenStream`` parameter in any of the ``virtlockd.socket`` and ``virtlockd-admin.socket`` unit files. -Systemd releases prior to version 227 lacked support for passing the activation -socket unit names into the service. When using these old versions, the -``unix_sock_dir`` setting in ``virtlockd.conf`` must be changed in -lock-step with the equivalent setting in the unit files to ensure that -``virtlockd`` can identify the sockets. - Changing command line options for daemons ========================================= diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index b44649bfbe..178e8cf830 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -986,10 +986,6 @@ int main(int argc, char **argv) { * saved state is present, therefore initialize from scratch here. */ if (rv == 0) { g_autoptr(virSystemdActivation) act = NULL; - virSystemdActivationMap actmap[] = { - { .name = "virtlockd.socket", .family = AF_UNIX, .path = sock_file }, - { .name = "virtlockd-admin.socket", .family = AF_UNIX, .path = admin_sock_file }, - }; if (godaemon) { if (chdir("/") < 0) { @@ -1016,9 +1012,7 @@ int main(int argc, char **argv) { goto cleanup; } - if (virSystemdGetActivation(actmap, - G_N_ELEMENTS(actmap), - &act) < 0) { + if (virSystemdGetActivation(&act) < 0) { ret = VIR_DAEMON_ERR_NETWORK; goto cleanup; } diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 245df9dbbd..82f625142b 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -794,10 +794,6 @@ int main(int argc, char **argv) { */ if (rv == 0) { g_autoptr(virSystemdActivation) act = NULL; - virSystemdActivationMap actmap[] = { - { .name = "virtlogd.socket", .family = AF_UNIX, .path = sock_file }, - { .name = "virtlogd-admin.socket", .family = AF_UNIX, .path = admin_sock_file }, - }; if (godaemon) { if (chdir("/") < 0) { @@ -824,9 +820,7 @@ int main(int argc, char **argv) { goto cleanup; } - if (virSystemdGetActivation(actmap, - G_N_ELEMENTS(actmap), - &act) < 0) { + if (virSystemdGetActivation(&act) < 0) { ret = VIR_DAEMON_ERR_NETWORK; goto cleanup; } diff --git a/src/remote/libvirtd-admin.socket.in b/src/remote/libvirtd-admin.socket.in index 4129abc1ab..01e1a08939 100644 --- a/src/remote/libvirtd-admin.socket.in +++ b/src/remote/libvirtd-admin.socket.in @@ -6,8 +6,6 @@ After=@service@.socket @deps@ [Socket] -# The directory must match the @sysconfdir@/libvirt/@service@.conf unix_sock_dir setting -# when using systemd version < 227 ListenStream=@runstatedir@/libvirt/@sockprefix@-admin-sock Service=@service@.service SocketMode=0600 diff --git a/src/remote/libvirtd-ro.socket.in b/src/remote/libvirtd-ro.socket.in index cb5e6bd60f..58ae1beb95 100644 --- a/src/remote/libvirtd-ro.socket.in +++ b/src/remote/libvirtd-ro.socket.in @@ -6,8 +6,6 @@ After=@service@.socket @deps@ [Socket] -# The directory must match the @sysconfdir@/libvirt/@service@.conf unix_sock_dir setting -# when using systemd version < 227 ListenStream=@runstatedir@/libvirt/@sockprefix@-sock-ro Service=@service@.service SocketMode=0666 diff --git a/src/remote/libvirtd-tcp.socket.in b/src/remote/libvirtd-tcp.socket.in index dd60317d47..6949df315e 100644 --- a/src/remote/libvirtd-tcp.socket.in +++ b/src/remote/libvirtd-tcp.socket.in @@ -6,8 +6,6 @@ After=@service@.socket @deps@ [Socket] -# This must match the @sysconfdir@/libvirt/@service@.conf tcp_port setting -# when using systemd version < 227 ListenStream=16509 Service=@service@.service diff --git a/src/remote/libvirtd-tls.socket.in b/src/remote/libvirtd-tls.socket.in index 8b89737fff..ada2b871f0 100644 --- a/src/remote/libvirtd-tls.socket.in +++ b/src/remote/libvirtd-tls.socket.in @@ -6,8 +6,6 @@ After=@service@.socket @deps@ [Socket] -# This must match the @sysconfdir@/libvirt/@service@.conf tls_port setting -# when using systemd version < 227 ListenStream=16514 Service=@service@.service diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in index 2cd20aaa7f..defe6b83d0 100644 --- a/src/remote/libvirtd.conf.in +++ b/src/remote/libvirtd.conf.in @@ -40,7 +40,7 @@ # This can be a port number, or service name # # This setting is not required or honoured if using systemd socket -# activation with systemd version >= 227 +# activation. # #tls_port = "16514" @@ -48,7 +48,7 @@ # This can be a port number, or service name # # This setting is not required or honoured if using systemd socket -# activation with systemd version >= 227 +# activation. # #tcp_port = "16509" @@ -117,7 +117,7 @@ # Set the name of the directory in which sockets will be found/created. # # This setting is not required or honoured if using systemd socket -# activation with systemd version >= 227 +# activation. # #unix_sock_dir = "@runstatedir@/libvirt" diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in index 0f349656f5..4842712648 100644 --- a/src/remote/libvirtd.socket.in +++ b/src/remote/libvirtd.socket.in @@ -4,8 +4,6 @@ Before=@service@.service @deps@ [Socket] -# The directory must match the @sysconfdir@/libvirt/@service@.conf unix_sock_dir setting -# when using systemd version < 227 ListenStream=@runstatedir@/libvirt/@sockprefix@-sock Service=@service@.service SocketMode=@mode@ diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 1b8e982a2f..26469e0d9f 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -214,25 +214,8 @@ daemonSetupNetworking(virNetServer *srv, unsigned int tcp_min_ssf = 0; #endif /* !WITH_SASL */ g_autoptr(virSystemdActivation) act = NULL; - virSystemdActivationMap actmap[] = { - { .name = DAEMON_NAME ".socket", .family = AF_UNIX, .path = sock_path }, - { .name = DAEMON_NAME "-ro.socket", .family = AF_UNIX, .path = sock_path_ro }, - { .name = DAEMON_NAME "-admin.socket", .family = AF_UNIX, .path = sock_path_adm }, -#ifdef WITH_IP - { .name = DAEMON_NAME "-tcp.socket", .family = AF_INET }, - { .name = DAEMON_NAME "-tls.socket", .family = AF_INET }, -#endif /* ! WITH_IP */ - }; - -#ifdef WITH_IP - if ((actmap[3].port = virSocketAddrResolveService(config->tcp_port)) < 0) - return -1; - - if ((actmap[4].port = virSocketAddrResolveService(config->tls_port)) < 0) - return -1; -#endif /* ! WITH_IP */ - if (virSystemdGetActivation(actmap, G_N_ELEMENTS(actmap), &act) < 0) + if (virSystemdGetActivation(&act) < 0) return -1; #ifdef WITH_IP diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index a86d4c6bb9..c95b6c93d3 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -788,98 +788,6 @@ virSystemdActivationInitFromNames(virSystemdActivation *act, } -/* - * Back compat for systemd < v227 which lacks LISTEN_FDNAMES. - * Delete when min systemd is increased ie RHEL7 dropped - */ -static int -virSystemdActivationInitFromMap(virSystemdActivation *act, - int nfds, - virSystemdActivationMap *map, - size_t nmap) -{ - int nextfd = STDERR_FILENO + 1; - size_t i; - - while (nfds) { - virSocketAddr addr; - const char *name = NULL; - - memset(&addr, 0, sizeof(addr)); - - addr.len = sizeof(addr.data); - if (getsockname(nextfd, &addr.data.sa, &addr.len) < 0) { - virReportSystemError(errno, "%s", _("Unable to get local socket name")); - goto error; - } - - VIR_DEBUG("Got socket family %d for FD %d", - addr.data.sa.sa_family, nextfd); - - for (i = 0; i < nmap && !name; i++) { - if (map[i].name == NULL) - continue; - - if (addr.data.sa.sa_family == AF_INET) { - if (map[i].family == AF_INET) { - VIR_DEBUG("Expect %d got %d", - map[i].port, ntohs(addr.data.inet4.sin_port)); - if (addr.data.inet4.sin_port == htons(map[i].port)) - name = map[i].name; - } - } else if (addr.data.sa.sa_family == AF_INET6) { - /* NB use of AF_INET here is correct. The "map" struct - * only refers to AF_INET. The socket may be AF_INET - * or AF_INET6 - */ - if (map[i].family == AF_INET) { - VIR_DEBUG("Expect %d got %d", - map[i].port, ntohs(addr.data.inet6.sin6_port)); - if (addr.data.inet6.sin6_port == htons(map[i].port)) - name = map[i].name; - } -#ifndef WIN32 - } else if (addr.data.sa.sa_family == AF_UNIX) { - if (map[i].family == AF_UNIX) { - VIR_DEBUG("Expect %s got %s", map[i].path, addr.data.un.sun_path); - if (STREQLEN(map[i].path, - addr.data.un.sun_path, - sizeof(addr.data.un.sun_path))) - name = map[i].name; - } -#endif - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unexpected socket family %d"), - addr.data.sa.sa_family); - goto error; - } - } - - if (!name) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot find name for FD %d socket family %d"), - nextfd, addr.data.sa.sa_family); - goto error; - } - - if (virSystemdActivationAddFD(act, name, nextfd) < 0) - goto error; - - nfds--; - nextfd++; - } - - return 0; - - error: - for (i = 0; i < nfds; i++) { - int fd = nextfd + i; - VIR_FORCE_CLOSE(fd); - } - return -1; -} - #ifndef WIN32 /** @@ -954,9 +862,7 @@ virSystemdGetListenFDs(void) #endif /* WIN32 */ static virSystemdActivation * -virSystemdActivationNew(virSystemdActivationMap *map, - size_t nmap, - int nfds) +virSystemdActivationNew(int nfds) { g_autoptr(virSystemdActivation) act = g_new0(virSystemdActivation, 1); const char *fdnames; @@ -966,14 +872,15 @@ virSystemdActivationNew(virSystemdActivationMap *map, act->fds = virHashNew(virSystemdActivationEntryFree); fdnames = getenv("LISTEN_FDNAMES"); - if (fdnames) { - if (virSystemdActivationInitFromNames(act, nfds, fdnames) < 0) - return NULL; - } else { - if (virSystemdActivationInitFromMap(act, nfds, map, nmap) < 0) - return NULL; + if (!fdnames) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing LISTEN_FDNAMES env from systemd socket activation")); + return NULL; } + if (virSystemdActivationInitFromNames(act, nfds, fdnames) < 0) + return NULL; + VIR_DEBUG("Created activation object for %d FDs", nfds); return g_steal_pointer(&act); } @@ -981,8 +888,6 @@ virSystemdActivationNew(virSystemdActivationMap *map, /** * virSystemdGetActivation: - * @map: mapping of socket addresses to names - * @nmap: number of entries in @map * @act: filled with allocated activation object * * Acquire an object for handling systemd activation. @@ -995,9 +900,7 @@ virSystemdActivationNew(virSystemdActivationMap *map, * Returns: 0 on success, -1 on failure */ int -virSystemdGetActivation(virSystemdActivationMap *map, - size_t nmap, - virSystemdActivation **act) +virSystemdGetActivation(virSystemdActivation **act) { int nfds = 0; @@ -1010,7 +913,7 @@ virSystemdGetActivation(virSystemdActivationMap *map, return 0; } - *act = virSystemdActivationNew(map, nmap, nfds); + *act = virSystemdActivationNew(nfds); return 0; } diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h index 93320e40b9..19fb714132 100644 --- a/src/util/virsystemd.h +++ b/src/util/virsystemd.h @@ -25,17 +25,6 @@ typedef struct _virSystemdActivation virSystemdActivation; -/* - * Back compat for systemd < v227 which lacks LISTEN_FDNAMES. - * Delete when min systemd is increased ie RHEL7 dropped - */ -typedef struct _virSystemdActivationMap { - const char *name; - int family; - int port; /* if family == AF_INET/AF_INET6 */ - const char *path; /* if family == AF_UNIX */ -} virSystemdActivationMap; - char *virSystemdMakeScopeName(const char *name, const char *drivername, bool legacy_behaviour); @@ -70,9 +59,7 @@ char *virSystemdGetMachineNameByPID(pid_t pid); char *virSystemdGetMachineUnitByPID(pid_t pid); -int virSystemdGetActivation(virSystemdActivationMap *map, - size_t nmap, - virSystemdActivation **act); +int virSystemdGetActivation(virSystemdActivation **act); bool virSystemdActivationHasName(virSystemdActivation *act, const char *name); diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 9a063dd1fc..9b513697e0 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -513,7 +513,7 @@ testActivationCreateFDs(virNetSocket **sockUNIX, static int -testActivation(bool useNames) +testActivationFDNames(const void *opaque G_GNUC_UNUSED) { virNetSocket *sockUNIX; virNetSocket **sockIP; @@ -522,7 +522,6 @@ testActivation(bool useNames) size_t i; char nfdstr[VIR_INT64_STR_BUFLEN]; char pidstr[VIR_INT64_STR_BUFLEN]; - virSystemdActivationMap map[2]; int *fds = NULL; size_t nfds = 0; g_autoptr(virSystemdActivation) act = NULL; @@ -544,21 +543,9 @@ testActivation(bool useNames) g_setenv("LISTEN_FDS", nfdstr, TRUE); g_setenv("LISTEN_PID", pidstr, TRUE); + g_setenv("LISTEN_FDNAMES", virBufferCurrentContent(&names), TRUE); - if (useNames) - g_setenv("LISTEN_FDNAMES", virBufferCurrentContent(&names), TRUE); - else - g_unsetenv("LISTEN_FDNAMES"); - - map[0].name = "demo-unix.socket"; - map[0].family = AF_UNIX; - map[0].path = demo_socket_path; - - map[1].name = "demo-ip.socket"; - map[1].family = AF_INET; - map[1].port = virNetSocketGetPort(sockIP[0]); - - if (virSystemdGetActivation(map, G_N_ELEMENTS(map), &act) < 0) + if (virSystemdGetActivation(&act) < 0) goto cleanup; if (act == NULL) { @@ -617,7 +604,7 @@ testActivationEmpty(const void *opaque G_GNUC_UNUSED) g_unsetenv("LISTEN_FDS"); - if (virSystemdGetActivation(NULL, 0, &act) < 0) + if (virSystemdGetActivation(&act) < 0) return -1; if (act != NULL) { @@ -629,21 +616,6 @@ testActivationEmpty(const void *opaque G_GNUC_UNUSED) return 0; } - -static int -testActivationFDNames(const void *opaque G_GNUC_UNUSED) -{ - return testActivation(true); -} - - -static int -testActivationFDAddrs(const void *opaque G_GNUC_UNUSED) -{ - return testActivation(false); -} - - static int mymain(void) { @@ -759,8 +731,6 @@ mymain(void) fcntl(STDERR_FILENO + 3, F_GETFL) == -1 && errno == EBADF) { if (virTestRun("Test activation names", testActivationFDNames, NULL) < 0) ret = -1; - if (virTestRun("Test activation addrs", testActivationFDAddrs, NULL) < 0) - ret = -1; } else { VIR_INFO("Skipping activation tests as FD 3/4/5 is open"); } -- 2.34.1