On Fri, 2012-04-20 at 20:00 +0200, Gergely Nagy wrote:
When the journal was introduced in systemd, it started to pass /run/systemd/journal/syslog to syslogds (it was passing down /dev/log before). Since we verify that the FD to open matches the expected filename, we didn't pick up the fd due to the filename mismatch.
This can be fixed on the system() source level, where it can decide which to generate into the config. However, for setups that do not use system() for one reason or the other, this does not work.
For them, the best solution is to try and check the passed FDs with /run/systemd/journal/syslog as the filename, so if all else fails, we pick that up. This patch does just that.
Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- modules/afsocket/afunix.c | 51 ++++++++++++++++++++++++++++++-------------- 1 files changed, 35 insertions(+), 16 deletions(-)
diff --git a/modules/afsocket/afunix.c b/modules/afsocket/afunix.c index 8145f1a..dff1efe 100644 --- a/modules/afsocket/afunix.c +++ b/modules/afsocket/afunix.c @@ -71,8 +71,9 @@ afunix_sd_set_perm(LogDriver *s, gint perm) }
#if ENABLE_SYSTEMD -static gboolean -afunix_sd_acquire_socket(AFSocketSourceDriver *s, gint *result_fd) +static gint +afunix_sd_acquire_socket_with_fn(AFSocketSourceDriver *s, gint *result_fd, + const gchar *filename) { AFUnixSourceDriver *self = (AFUnixSourceDriver *) s; gint fd, fds;
I've seen that you also queued another round of this patch, so not applying now. Also, can you please give a better name to this function? Something like afunix_sd_acquire_named_socket() the "with_fn" part is difficult to understand IMHO.
@@ -82,7 +83,7 @@ afunix_sd_acquire_socket(AFSocketSourceDriver *s, gint *result_fd) fds = sd_listen_fds(0);
if (fds == 0) - return TRUE; + return 1;
msg_debug("Systemd socket activation", evt_tag_int("systemd-sockets", fds), @@ -94,19 +95,19 @@ afunix_sd_acquire_socket(AFSocketSourceDriver *s, gint *result_fd) { msg_error("Failed to acquire systemd sockets, incorrectly set LISTEN_FDS environment variable?", NULL); - return FALSE; + return -1; } else if (fds > 0) { for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + fds; fd++) { /* check if any type is available */ - if (sd_is_socket_unix(fd, 0, -1, self->filename, 0)) + if (sd_is_socket_unix(fd, 0, -1, filename, 0)) { int type = (self->super.flags & AFSOCKET_STREAM) ? SOCK_STREAM : SOCK_DGRAM;
/* check if it matches our idea of the socket type */ - if (sd_is_socket_unix(fd, type, -1, self->filename, 0)) + if (sd_is_socket_unix(fd, type, -1, filename, 0)) { *result_fd = fd; break; @@ -114,11 +115,11 @@ afunix_sd_acquire_socket(AFSocketSourceDriver *s, gint *result_fd) else { msg_error("The systemd supplied UNIX domain socket is of a different type, check the configured driver and the matching systemd unit file", - evt_tag_str("filename", self->filename), + evt_tag_str("filename", filename), evt_tag_int("systemd-sock-fd", fd), evt_tag_str("expecting", type == SOCK_STREAM ? "unix-stream()" : "unix-dgram()"), NULL); - return FALSE; + return -1; } } else @@ -130,31 +131,49 @@ afunix_sd_acquire_socket(AFSocketSourceDriver *s, gint *result_fd) */
msg_debug("Ignoring systemd supplied fd as it is not a UNIX domain socket", - evt_tag_str("filename", self->filename), + evt_tag_str("filename", filename), evt_tag_int("systemd-sock-fd", fd), NULL); } } } else - return TRUE; + return 1;
if (*result_fd != -1) { g_fd_set_nonblock(*result_fd, TRUE); g_fd_set_cloexec(*result_fd, TRUE); msg_verbose("Acquired systemd socket", - evt_tag_str("filename", self->filename), + evt_tag_str("filename", filename), evt_tag_int("systemd-sock-fd", *result_fd), NULL); + return 1; } - else + msg_debug("Failed to acquire systemd socket, trying to open ourselves", + evt_tag_str("filename", filename), + NULL); + return 0; +} + +static gboolean +afunix_sd_acquire_socket(AFSocketSourceDriver *s, gint *result_fd) +{ + AFUnixSourceDriver *self = (AFUnixSourceDriver *) s; + int status; + + status = afunix_sd_acquire_socket_with_fn(s, result_fd, self->filename); + if (status == 0) { - msg_debug("Failed to acquire systemd socket, trying to open ourselves", - evt_tag_str("filename", self->filename), - NULL); + status = afunix_sd_acquire_socket_with_fn(s, result_fd, "/run/systemd/journal/syslog"); + if (status == 1) + { + g_free(self->filename); + self->filename = g_strdup("/run/systemd/journal/syslog"); + return TRUE; + } } - return TRUE; + return !!(status == 1);
Maybe it's just me, but I have difficulties remembering magic values like 0, -1 or 1 to indicate status, especially the return statement is difficult to comprehend. I tend to indicate success with a mere boolean and if further information is needed, return them through parameters. FALSE could mean that an error occurred, TRUE with "listen_fd == -1" means there was no error but no socket could be acquired. OR, you could use an enum to name the respective return values. -- Bazsi