Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

xyzzy42
Copy link
Contributor

@xyzzy42 xyzzy42 commented Jun 10, 2021

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.

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
@xyzzy42
Copy link
Contributor Author

xyzzy42 commented Jun 10, 2021

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.

Comment on lines +1962 to +1971
error = true;
}
}
free(names);

if (error) {
// Just clean up stuct now and refuse to use any sockets
freePassedSockets(ps);
ps = NULL;
}
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant