x0vncserver: Add support for systemd socket activation #1716
Conversation
|
Nice. Socket activation is something we've mostly thought about for Would you mind describing how you envision this to be used? |
unix/x0vncserver/CMakeLists.txt
Outdated
| @@ -21,6 +21,13 @@ target_include_directories(x0vncserver PUBLIC ${CMAKE_SOURCE_DIR}/unix) | |||
| target_include_directories(x0vncserver PUBLIC ${CMAKE_SOURCE_DIR}/common) | |||
| target_link_libraries(x0vncserver tx rfb network rdr unixcommon) | |||
|
|
|||
| # check for systemd support (socket activation) | |||
| pkg_check_modules(LIBSYSTEMD libsystemd) | |||
There was a problem hiding this comment.
We try to put all checks in the top CMakeLists.txt, to make it easier to see TigerVNC's dependencies.
On that note, do you think you could add this as an optional dependency to BUILDING.txt?
|
|
||
| for (int i = 0; i < count; ++i) { | ||
| /* systemd sockets start at FD 3 */ | ||
| listeners->push_back(new TcpListener(3 + i)); |
There was a problem hiding this comment.
There's a SD_LISTEN_FDS_START constant. Can't we use that to make things more clear?
There was a problem hiding this comment.
Yes. The systemd docs said "3", a constant is better than a magic number.
| static int createSystemdListeners(std::list<SocketListener*> *listeners) | ||
| { | ||
| #ifdef HAVE_SYSTEMD_H | ||
| int count = sd_listen_fds(0); |
There was a problem hiding this comment.
Maybe it's best to check for errors explicit. Less risk for future bugs.
unix/x0vncserver/x0vncserver.cxx
Outdated
| if (rfbunixpath.getValueStr()[0] != '\0') { | ||
| if (createSystemdListeners(&listeners)) { | ||
| vlog.info("Listening on systemd sockets"); | ||
| } else if (rfbunixpath.getValueStr()[0] != '\0') { |
There was a problem hiding this comment.
The else seems improper here. Let's respect all the options given.
There was a problem hiding this comment.
When using systemd, ALL listening sockets will be passed through filedescriptors. The server shouldn't attempt to open any further sockets on its own. So this flow is on purpose. Otherwise you'd be obligated to explicitly disable the other (default) sockets.
There was a problem hiding this comment.
In that case, the proposed change is not succeeding. It disables Unix sockets, which are already disabled by default. But the TCP socket will still start.
I still don't think we should prevent people from listening to both. We've had issues with such black-and-white models before. But I can agree that it might be sensible to have different defaults if it's systemd activated.
There was a problem hiding this comment.
There were some changes between this version and the one I tested with, I've updated the code to handle things as intended.
Add a a socket and service file, e.g. $ cat x0vncserver.service $ cat x0vncserver.socket Connecting to port 5900 will automatically start the server, and multiple clients can connect. 3 seconds after the last client disconnects, the server will shut down. When a new client connects, systemd will start a new instance of the server. This allows one to always have a VNC server on "standby", but it doesn't consume any resources (apart from the listening socket) when not in use. |
|
Thanks. That makes it clear how things can be set up. Perhaps that should be added to the man page? I'm still not clear how the whole system is supposed to work, though. How have you made sure there is a useful X server on :0? And that x0vncserver is authorized to talk to it? Is it the display manager that's on :0? What happens once you log in at that point? Won't the desktop be started at :1? |
|
Our particular use case is an embedded system. Since that only every runs one desktop with one user, we can get away with hardcoding the DISPLAY. Other people with other use cases may have to write their own solutions to all the problems you're mentioning. My focus here is to get systemd support into the server, because I think it'd be useful to others as well, and upstreaming it will also avoid having to maintain it locally. I'm willing to spend a reasonable bit of effort to get it into the project, but the response I'm getting so far isn't very encouraging. |
MikeLooijmans
force-pushed
the
systemd-socket-activation
branch
from
8173602 to
50147aa
Compare
systemd can pass in sockets as file descriptors 3 and beyond. Allows the server to use socket activation. When triggered by systemd, no other listening sockets (e.g. rfbport) will be activated. Signed-off-by: Mike Looijmans <[email protected]>
MikeLooijmans
force-pushed
the
systemd-socket-activation
branch
from
50147aa to
fdd429c
Compare
|
We are definitely open to merging this. But please keep in mind that TigerVNC is used by a lot of people. So we need to be careful to balance the needs of all our users. Which means a feature may need more polish than was needed in its initial use case. |
systemd can pass in sockets as file descriptors 3 and beyond. Allows the server to use socket activation.