[libvirt] [PATCH 0/2] Fix systemd socket activation for systemd < v227

A couple of bugs were encountered using socket activated libvirtd on CentOS 7: - When resolving a tls_port/tcp_port from the config file (needed even when using socket activation on systemd < v227), the port endianness was incorrect. - The wrong name was used when checking whether libvirtd-tls.socket was passed to libvirtd, leading to this socket not being properly registered. Michael Chapman (2): util: fix byte order of port in virSocketAddrResolveService remote: fix registration of TLS socket src/remote/remote_daemon.c | 2 +- src/util/virsocketaddr.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) -- 2.20.1

The ports in the socket address structures returned by getaddrinfo() are in network byte order. Convert to host byte order before returning them. Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- src/util/virsocketaddr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index c1fd5ec3d2..bba2089436 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -265,12 +265,12 @@ int virSocketAddrResolveService(const char *service) if (tmp->ai_family == AF_INET) { struct sockaddr_in in; memcpy(&in, tmp->ai_addr, sizeof(in)); - port = in.sin_port; + port = ntohs(in.sin_port); goto cleanup; } else if (tmp->ai_family == AF_INET6) { struct sockaddr_in6 in; memcpy(&in, tmp->ai_addr, sizeof(in)); - port = in.sin6_port; + port = ntohs(in.sin6_port); goto cleanup; } tmp++; -- 2.20.1

On Tue, Sep 17, 2019 at 05:03:56PM +1000, Michael Chapman wrote:
The ports in the socket address structures returned by getaddrinfo() are in network byte order. Convert to host byte order before returning them.
Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- src/util/virsocketaddr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index c1fd5ec3d2..bba2089436 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -265,12 +265,12 @@ int virSocketAddrResolveService(const char *service) if (tmp->ai_family == AF_INET) { struct sockaddr_in in; memcpy(&in, tmp->ai_addr, sizeof(in)); - port = in.sin_port; + port = ntohs(in.sin_port); goto cleanup; } else if (tmp->ai_family == AF_INET6) { struct sockaddr_in6 in; memcpy(&in, tmp->ai_addr, sizeof(in)); - port = in.sin6_port; + port = ntohs(in.sin6_port); goto cleanup; } tmp++;
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- src/remote/remote_daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 966f3da2d0..7195ac9218 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -505,7 +505,7 @@ daemonSetupNetworking(virNetServerPtr srv, config->max_client_requests) < 0) goto cleanup; - if (((ipsock && config->listen_tls) || (act && virSystemdActivationHasName(act, "ip-tls")))) { + if (((ipsock && config->listen_tls) || (act && virSystemdActivationHasName(act, DAEMON_NAME "-tls.socket")))) { virNetTLSContextPtr ctxt = NULL; if (config->ca_file || -- 2.20.1

On Tue, Sep 17, 2019 at 05:03:57PM +1000, Michael Chapman wrote:
Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- src/remote/remote_daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 966f3da2d0..7195ac9218 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -505,7 +505,7 @@ daemonSetupNetworking(virNetServerPtr srv, config->max_client_requests) < 0) goto cleanup;
- if (((ipsock && config->listen_tls) || (act && virSystemdActivationHasName(act, "ip-tls")))) { + if (((ipsock && config->listen_tls) || (act && virSystemdActivationHasName(act, DAEMON_NAME "-tls.socket")))) { virNetTLSContextPtr ctxt = NULL;
if (config->ca_file ||
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Tue, Sep 17, 2019 at 05:03:55PM +1000, Michael Chapman wrote:
A couple of bugs were encountered using socket activated libvirtd on CentOS 7:
It actually hits on new distros too
- When resolving a tls_port/tcp_port from the config file (needed even when using socket activation on systemd < v227), the port endianness was incorrect.
Opps.
- The wrong name was used when checking whether libvirtd-tls.socket was passed to libvirtd, leading to this socket not being properly registered.
Due to the refactoring for the split daemons being messed up. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé
-
Michael Chapman