On Wed, Aug 13, 2014 at 04:01:42PM +0100, Daniel P. Berrange wrote:
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@xxxxxxxxxx> --- 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.
Is this really necessary? I mean I went with the approach that we can get more than 1 FD passed even though it's not needed to fix the 20s timeout (the original bug). And if we decide we need more it can be reworked that way, yes. But right now I really don't think we do, especially since race condition solved by passing TCP or TLS ports would be *really* rare; and anyone connecting during the time system daemon is starting will just try again in a while.
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
I don't think right now this approach would add as much benefit as how much code it would add (and with this potential problems, too). I think that from the .service file it is pretty obvious that we just support passing FDs for the sockets. Martin
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list