-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Socket activation #26
base: master
Are you sure you want to change the base?
Conversation
The call to MHD_start_daemon() was duplicated twice, with slightly different options, for ssl vs non-ssl. Use MHD_OPTION_ARRAY so that the same call can be used for both with the ssl option passed via the array. This removes some duplicated code. Right now it is not much, because ssl is the only option. It will make a much difference when there is another option. The current method will require a different call for every possible combination of options.
This doesn't really do much of anything. Mostly it changes the flow so a socket bind address failure continues instead having the rest of the loop inside an if statement. So it makes a bunch of code indented one level fewer. The real reason is so the next commit, when the socket address might not be used, doesn't need to do this. That way it doesn't look like a lot of code is being re-written when it's not.
This allows using with systemd socket units. It works like the old inetd style activation. The socket(s) to open are described in systemd socket units, and when a connection is made, systemd starts webdavd and passes the sockets to it on some file descriptors. This way webdavd doesn't need to be running until it's used. Systemd also supports a lot more options for how it can listen on a socket. E.g., sockets can be bound to a specific interface and UNIX domain sockets can be used. Multiple sockets can be used, e.g. http and https. Systemd will pass all of them to webdavd. libminihttpd supports this already and can take an existing fd as the socket. I use a systemd function that does some handy stuff, so this requires systemd's libraries. So that this isn't a hard dependency, it's protected in the build system by requiring HAVE_SYSTEMD to be set at build time, e.g. make HAVE_SYSTEMD=1. One could add support for the old inetd server, but I didn't since it's not been included with with Fedora for years.
This enables the systemd socket activation support when building the RPM. It includes two sample socket unit files. They will start webdavd if there is a connection on either the http or https port. They can be used by starting/enabling the socket units instead of the service unit. It's possible to enable either or both of the socket units. and also
I used this to put webdavd behind an nginx reverse proxy. I only wanted it to be on a certain vhost. Originally I had it on localhost:30000 and nginx would proxy dav.mydomain.com to localhost:30000. But a UNIX domain socket would be better. It is more efficient than TCP/IP over loopback and the UNIX socket has better access control. Only processes in the nginx group are allowed to open it, unlike anything on localhost for the AF_INET socket. I added UNIX sockets to webdavd, but this was a pain. You need to add the socket path to the xml, the group needs to be set so nginx can open it, the socket permissions to 0660, and then creating leading directories, and those directories' permissions, and how many listening connections are allowed, and so on. It was going to be a huge pain to add all this code to webdavd and put everything into the xml file. So instead I added socket based activation. systemd already supports all this stuff for UNIX domain sockets, and a lot more besides, so one basically gets full UNIX socket support for free. In additional to the not running until used feature. |
error = true; | ||
} | ||
} | ||
free(names); | ||
|
||
if (error) { | ||
// Just clean up stuct now and refuse to use any sockets | ||
freePassedSockets(ps); | ||
ps = NULL; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have this totally fail if any socket passed is not acceptable. I'd rather have something fail if there is any problem than to keep going and maybe work partially.
But your design elsewhere doesn't seem to be like this. I.e., failure to start a daemon will continue to the next one and not return failure.
This code here could easily change to be like that too. The code related to error
can just be deleted. Any other sockets will still work fine. Maybe the config xml doesn't even use the bad socket for any listen sections and it doesn't matter? If MHD doesn't like the socket, it'll give us an error and that listener won't start.
This allows using with systemd socket units. It works like the old inetd style activation.
The socket(s) to open are described in systemd socket units, and when a connection is made, systemd starts webdavd and passes the sockets to it on some file descriptors.
This way webdavd doesn't need to be running until it's used. Systemd also supports a lot more options for how it can listen on a socket. E.g., sockets can be bound to a specific interface and UNIX domain sockets can be used. See man:systemd.socket(5).
Multiple sockets can be used, e.g. http and https. Systemd will pass all of them to webdavd.
libminihttpd supports this already and can take an existing fd as the socket.
I use a systemd function that does some handy stuff, so this requires systemd's libraries. So that this isn't a hard dependency, it's protected in the build system by requiring HAVE_SYSTEMD to be set at build time, e.g. make HAVE_SYSTEMD=1.
One could add support for the old inetd server, but I didn't since it's not been included with with Fedora for years.
WebDAVd doesn't support shutting down after being idle for a period of time, but it could. Most of the logic is already there to do that. Without socket activation it wasn't useful since it wouldn't start up again, but with socket activation it should be possible to webdavd to exit when all the workers have shutdown.