Re: [libvirt] [PATCH v3 4/9] daemon: support passing FDs from the calling process

On Wed, Jul 23, 2014 at 04:27:08PM +0200, Martin Kletzander wrote: > First FD is the RW unix socket to listen on, second one (if > applicable) is the RO unix socket. > > Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; > --- > daemon/libvirtd.c | 45 +++++++++++++++++++++++++++------------------ > 1 file changed, 27 insertions(+), 18 deletions(-) > > diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c > index 4c926b3..15f0ce2 100644 > --- a/daemon/libvirtd.c > +++ b/daemon/libvirtd.c > @@ -56,6 +56,7 @@ > #include "virstring.h" > #include "locking/lock_manager.h" > #include "viraccessmanager.h" > +#include "virutil.h" > > #ifdef WITH_DRIVER_MODULES > # include "driver.h" > @@ -476,11 +477,19 @@ static int daemonSetupNetworking(virNetServerPtr srv, > int unix_sock_ro_mask = 0; > int unix_sock_rw_mask = 0; > > + unsigned int cur_fd = STDERR_FILENO + 1; > + unsigned int nfds = virGetListenFDs(); > + > if (config->unix_sock_group) { > if (virGetGroupID(config->unix_sock_group, &unix_sock_gid) < 0) > return -1; > } > > + if (nfds && nfds > ((int)!!sock_path + (int)!!sock_path_ro)) { > + VIR_ERROR(_("Too many (%u) FDs passed from caller"), nfds); > + return -1; > + } > + > 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 error; > @@ -491,30 +500,30 @@ static int daemonSetupNetworking(virNetServerPtr srv, > goto error; > } > > - VIR_DEBUG("Registering unix socket %s", sock_path); > - if (!(svc = virNetServerServiceNewUNIX(sock_path, > - unix_sock_rw_mask, > - unix_sock_gid, > - config->auth_unix_rw, > + if (!(svc = virNetServerServiceNewFDOrUNIX(sock_path, > + unix_sock_rw_mask, > + unix_sock_gid, > + config->auth_unix_rw, > #if WITH_GNUTLS > - NULL, > + NULL, > #endif > - false, > - config->max_queued_clients, > - config->max_client_requests))) > + false, > + config->max_queued_clients, > + config->max_client_requests, > + nfds, &cur_fd))) > goto error; > if (sock_path_ro) { > - VIR_DEBUG("Registering unix socket %s", sock_path_ro); > - if (!(svcRO = virNetServerServiceNewUNIX(sock_path_ro, > - unix_sock_ro_mask, > - unix_sock_gid, > - config->auth_unix_ro, > + if (!(svcRO = virNetServerServiceNewFDOrUNIX(sock_path_ro, > + unix_sock_ro_mask, > + unix_sock_gid, > + config->auth_unix_ro, > #if WITH_GNUTLS > - NULL, > + NULL, > #endif > - true, > - config->max_queued_clients, > - config->max_client_requests))) > + true, > + config->max_queued_clients, > + config->max_client_requests, > + nfds, &cur_fd))) > goto error; > } I'm wondering how we deal with TCP socket listening too. eg if someone modifies the systemd config so that it listens on the TCP sockets too then we're not setting up libvirt to deal with those.

Rather than doing this NewFDOrUnix() approach, I'm wondering if we would be better off doing. if (getenv(LISTEN_FDS) != NULL) { ....call NewFD() on all the FDs we've been given... } else { .. do normal NewUnix/NewTCP/NewTLS setup... } though it gets harder than that because we'll need to call getsockname on the FD to determine if the socket we've received is a UNIX or TCP socket and what port number it is on (so we can see if it is TCP or TLS protocol) and we need to passin the TLS context in some cases. So perhaps the virNetServerServiceNewTCP and virNetServerServiceNewTLS method should simply accept a pre-opened FD. so we could do int unixfd = virGetInheritedUNIXSocket("/some/path"); int unixrofd = virGetInheritedTCPSocket("/some/other/path") int tcpfd = virGetInheritedTCPSocket(16509) int tlsfd = virGetInheritedTLSSocket(16514) and then pass these FDs into the regular virNetServerServiceNew* methods. Assume '-1' means pre-open the socket the normal way