From b41b9d2ae9d1139a3d4d5eb7ad7b100ffa3ee6db Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 8 Oct 2015 16:15:38 +0200 Subject: [PATCH 01/11] systemctl: add a way to explicitly request client-side unit installing This adds support for a new environment variable SYSTEMCTL_INSTALL_CLIENT_SIDE, that ensures that systemctl executes install operations client-side instead of passing them to PID1. This is useful in debugging situations, but even beyond that. However, we don't want to make it official API, hence let's just make it an undocumented environment variable. Similar, add a second variable, SYSTEMCTL_SKIP_SYSV which allows skipping the SysV chkconfig fall-back if set. This is useful for similar reasons, and exposed as undocumented as environment variable for similar reasons, too. --- src/basic/env-util.c | 11 +++++++++++ src/basic/env-util.h | 2 ++ src/systemctl/systemctl.c | 7 +++++++ 3 files changed, 20 insertions(+) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 94cb25169839d..9ddac5d6a1c39 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -25,6 +25,7 @@ #include "alloc-util.h" #include "def.h" #include "env-util.h" +#include "parse-util.h" #include "string-util.h" #include "strv.h" #include "utf8.h" @@ -594,3 +595,13 @@ char **replace_env_argv(char **argv, char **env) { ret[k] = NULL; return ret; } + +int getenv_bool(const char *p) { + const char *e; + + e = getenv(p); + if (!e) + return -ENXIO; + + return parse_boolean(e); +} diff --git a/src/basic/env-util.h b/src/basic/env-util.h index 803aa61cad7de..6485dade1857e 100644 --- a/src/basic/env-util.h +++ b/src/basic/env-util.h @@ -47,3 +47,5 @@ char **strv_env_unset_many(char **l, ...) _sentinel_; char *strv_env_get_n(char **l, const char *name, size_t k) _pure_; char *strv_env_get(char **x, const char *n) _pure_; + +int getenv_bool(const char *p); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index f5efa1a064f9d..da816e6d0e2d1 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -296,6 +296,10 @@ static bool install_client_side(void) { if (arg_scope == UNIT_FILE_GLOBAL) return true; + /* Unsupported environment variable, mostly for debugging purposes */ + if (getenv_bool("SYSTEMCTL_INSTALL_CLIENT_SIDE") > 0) + return true; + return false; } @@ -5317,6 +5321,9 @@ static int enable_sysv_units(const char *verb, char **args) { if (arg_scope != UNIT_FILE_SYSTEM) return 0; + if (getenv_bool("SYSTEMCTL_SKIP_SYSV") > 0) + return 0; + if (!STR_IN_SET(verb, "enable", "disable", From 96d66d89c9ba3cc936a8d8217370c70292a6abb7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Oct 2015 12:43:52 +0100 Subject: [PATCH 02/11] core: constify a few things --- src/basic/fdset.c | 2 +- src/basic/fdset.h | 2 +- src/core/manager.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/basic/fdset.c b/src/basic/fdset.c index 4b11e4ea09201..42b0b2b98f635 100644 --- a/src/basic/fdset.c +++ b/src/basic/fdset.c @@ -44,7 +44,7 @@ FDSet *fdset_new(void) { return MAKE_FDSET(set_new(NULL)); } -int fdset_new_array(FDSet **ret, int *fds, unsigned n_fds) { +int fdset_new_array(FDSet **ret, const int *fds, unsigned n_fds) { unsigned i; FDSet *s; int r; diff --git a/src/basic/fdset.h b/src/basic/fdset.h index 340438d7c401c..70d8acbcff343 100644 --- a/src/basic/fdset.h +++ b/src/basic/fdset.h @@ -35,7 +35,7 @@ int fdset_consume(FDSet *s, int fd); bool fdset_contains(FDSet *s, int fd); int fdset_remove(FDSet *s, int fd); -int fdset_new_array(FDSet **ret, int *fds, unsigned n_fds); +int fdset_new_array(FDSet **ret, const int *fds, unsigned n_fds); int fdset_new_fill(FDSet **ret); int fdset_new_listen_fds(FDSet **ret, bool unset); diff --git a/src/core/manager.c b/src/core/manager.c index d161e6c57bd20..8decfd472f408 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1488,7 +1488,7 @@ static unsigned manager_dispatch_dbus_queue(Manager *m) { return n; } -static void manager_invoke_notify_message(Manager *m, Unit *u, pid_t pid, char *buf, size_t n, FDSet *fds) { +static void manager_invoke_notify_message(Manager *m, Unit *u, pid_t pid, const char *buf, size_t n, FDSet *fds) { _cleanup_strv_free_ char **tags = NULL; assert(m); @@ -1618,7 +1618,7 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t return 0; } -static void invoke_sigchld_event(Manager *m, Unit *u, siginfo_t *si) { +static void invoke_sigchld_event(Manager *m, Unit *u, const siginfo_t *si) { assert(m); assert(u); assert(si); From 3c747da38ca2f0642b4811812f6e2e2e1449a622 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Oct 2015 18:22:23 +0100 Subject: [PATCH 03/11] nspawn: fix minor memory leak When rebooting nspawn containers about 400 times we'd otherwise hit the fd limit and refuse further reboots. --- src/shared/ptyfwd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index 786752ea941b4..63e81f489490e 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -413,6 +413,7 @@ PTYForward *pty_forward_free(PTYForward *f) { sd_event_source_unref(f->stdin_event_source); sd_event_source_unref(f->stdout_event_source); sd_event_source_unref(f->master_event_source); + sd_event_source_unref(f->sigwinch_event_source); sd_event_unref(f->event); if (f->saved_stdout) From 97044145b48fa5644ecd23e73bc33980faeabda3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Oct 2015 18:23:07 +0100 Subject: [PATCH 04/11] core,nspawn: minor coding style fixes --- src/core/manager.c | 6 ++---- src/nspawn/nspawn.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 8decfd472f408..5fdeb4a99c484 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2000,8 +2000,7 @@ int manager_loop(Manager *m) { m->exit_code = MANAGER_OK; /* Release the path cache */ - set_free_free(m->unit_path_cache); - m->unit_path_cache = NULL; + m->unit_path_cache = set_free_free(m->unit_path_cache); manager_check_finished(m); @@ -2771,8 +2770,7 @@ static int create_generator_dir(Manager *m, char **generator, const char *name) return log_oom(); if (!mkdtemp(p)) { - log_error_errno(errno, "Failed to create generator directory %s: %m", - p); + log_error_errno(errno, "Failed to create generator directory %s: %m", p); free(p); return -errno; } diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 44f08ab1b4432..ff12ca64983c3 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3227,8 +3227,7 @@ int main(int argc, char *argv[]) { } for (;;) { - _cleanup_close_pair_ int kmsg_socket_pair[2] = { -1, -1 }, rtnl_socket_pair[2] = { -1, -1 }, pid_socket_pair[2] = { -1, -1 }, - uid_shift_socket_pair[2] = { -1, -1 }; + _cleanup_close_pair_ int kmsg_socket_pair[2] = { -1, -1 }, rtnl_socket_pair[2] = { -1, -1 }, pid_socket_pair[2] = { -1, -1 }, uid_shift_socket_pair[2] = { -1, -1 }; ContainerStatus container_status; _cleanup_(barrier_destroy) Barrier barrier = BARRIER_NULL; static const struct sigaction sa = { From a1a078eef248cd2aa7bc190d6296255a034acadc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Oct 2015 18:23:26 +0100 Subject: [PATCH 05/11] core: bail our earlier when doing audit Let's make sure we don't even try to create the audit socket --- src/core/manager.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 5fdeb4a99c484..2039dafea12c9 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2110,6 +2110,9 @@ void manager_send_unit_audit(Manager *m, Unit *u, int type, bool success) { const char *msg; int audit_fd, r; + if (m->running_as != MANAGER_SYSTEM) + return; + audit_fd = get_audit_fd(); if (audit_fd < 0) return; @@ -2119,9 +2122,6 @@ void manager_send_unit_audit(Manager *m, Unit *u, int type, bool success) { if (m->n_reloading > 0) return; - if (m->running_as != MANAGER_SYSTEM) - return; - if (u->type != UNIT_SERVICE) return; From c279613f861636c816f2f7df051b02c2f55a5134 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 29 Oct 2015 00:19:31 +0100 Subject: [PATCH 06/11] sysv-generator: modernize - Make sure we log each error at least once, and at most once - Replace FOREACH_WORD loops by extract_first_word() loops - Use FOREACH_DIRENT() for directory loops - Use free_and_strdup() where appropriate - Do not operate on half-loaded SysV files - Always properly free all memory --- src/sysv-generator/sysv-generator.c | 476 +++++++++++++++------------- 1 file changed, 248 insertions(+), 228 deletions(-) diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c index 0a0b9269b3f91..b2d53d3b4feff 100644 --- a/src/sysv-generator/sysv-generator.c +++ b/src/sysv-generator/sysv-generator.c @@ -26,7 +26,9 @@ #include #include "alloc-util.h" +#include "dirent-util.h" #include "fd-util.h" +#include "fileio.h" #include "hashmap.h" #include "hexdecoct.h" #include "install.h" @@ -85,9 +87,13 @@ typedef struct SysvStub { char **conflicts; bool has_lsb; bool reload; + bool loaded; } SysvStub; static void free_sysvstub(SysvStub *s) { + if (!s) + return; + free(s->name); free(s->path); free(s->description); @@ -112,19 +118,14 @@ static void free_sysvstub_hashmapp(Hashmap **h) { } static int add_symlink(const char *service, const char *where) { - _cleanup_free_ char *from = NULL, *to = NULL; + const char *from, *to; int r; assert(service); assert(where); - from = strjoin(arg_dest, "/", service, NULL); - if (!from) - return log_oom(); - - to = strjoin(arg_dest, "/", where, ".wants/", service, NULL); - if (!to) - return log_oom(); + from = strjoina(arg_dest, "/", service); + to = strjoina(arg_dest, "/", where, ".wants/", service); mkdir_parents_label(to, 0755); @@ -132,6 +133,7 @@ static int add_symlink(const char *service, const char *where) { if (r < 0) { if (errno == EEXIST) return 0; + return -errno; } @@ -139,20 +141,19 @@ static int add_symlink(const char *service, const char *where) { } static int add_alias(const char *service, const char *alias) { - _cleanup_free_ char *link = NULL; + const char *link; int r; assert(service); assert(alias); - link = strjoin(arg_dest, "/", alias, NULL); - if (!link) - return log_oom(); + link = strjoina(arg_dest, "/", alias); r = symlink(service, link); if (r < 0) { if (errno == EEXIST) return 0; + return -errno; } @@ -160,26 +161,32 @@ static int add_alias(const char *service, const char *alias) { } static int generate_unit_file(SysvStub *s) { - char **p; + _cleanup_free_ char *before = NULL, *after = NULL, *wants = NULL, *conflicts = NULL; _cleanup_fclose_ FILE *f = NULL; - _cleanup_free_ char *unit = NULL, - *before = NULL, *after = NULL, - *wants = NULL, *conflicts = NULL; + const char *unit; + char **p; int r; + assert(s); + + if (!s->loaded) + return 0; + + unit = strjoina(arg_dest, "/", s->name); + before = strv_join(s->before, " "); after = strv_join(s->after, " "); wants = strv_join(s->wants, " "); conflicts = strv_join(s->conflicts, " "); - unit = strjoin(arg_dest, "/", s->name, NULL); - if (!before || !after || !wants || !conflicts || !unit) + + if (!before || !after || !wants || !conflicts) return log_oom(); /* We might already have a symlink with the same name from a Provides:, * or from backup files like /etc/init.d/foo.bak. Real scripts always win, * so remove an existing link */ if (is_symlink(unit) > 0) { - log_warning("Overwriting existing symlink %s with real service", unit); + log_warning("Overwriting existing symlink %s with real service.", unit); (void) unlink(unit); } @@ -191,9 +198,11 @@ static int generate_unit_file(SysvStub *s) { "# Automatically generated by systemd-sysv-generator\n\n" "[Unit]\n" "Documentation=man:systemd-sysv-generator(8)\n" - "SourcePath=%s\n" - "Description=%s\n", - s->path, s->description); + "SourcePath=%s\n", + s->path); + + if (s->description) + fprintf(f, "Description=%s\n", s->description); if (!isempty(before)) fprintf(f, "Before=%s\n", before); @@ -226,13 +235,17 @@ static int generate_unit_file(SysvStub *s) { if (s->reload) fprintf(f, "ExecReload=%s reload\n", s->path); + r = fflush_and_check(f); + if (r < 0) + return log_error_errno(r, "Failed to write unit %s: %m", unit); + STRV_FOREACH(p, s->wanted_by) { r = add_symlink(s->name, *p); if (r < 0) - log_error_errno(r, "Failed to create 'Wants' symlink to %s: %m", *p); + log_warning_errno(r, "Failed to create 'Wants' symlink to %s, ignoring: %m", *p); } - return 0; + return 1; } static bool usage_contains_reload(const char *line) { @@ -262,7 +275,7 @@ static char *sysv_translate_name(const char *name) { return res; } -static int sysv_translate_facility(const char *name, const char *filename, char **_r) { +static int sysv_translate_facility(const char *name, const char *filename, char **ret) { /* We silently ignore the $ prefix here. According to the LSB * spec it simply indicates whether something is a @@ -281,31 +294,45 @@ static int sysv_translate_facility(const char *name, const char *filename, char "time", SPECIAL_TIME_SYNC_TARGET, }; - char *filename_no_sh, *e, *r; + char *filename_no_sh, *e, *m; const char *n; unsigned i; + int r; assert(name); - assert(_r); + assert(filename); + assert(ret); n = *name == '$' ? name + 1 : name; for (i = 0; i < ELEMENTSOF(table); i += 2) { - if (!streq(table[i], n)) continue; if (!table[i+1]) return 0; - r = strdup(table[i+1]); - if (!r) + m = strdup(table[i+1]); + if (!m) return log_oom(); - goto finish; + *ret = m; + return 1; + } + + /* If we don't know this name, fallback heuristics to figure + * out whether something is a target or a service alias. */ + + /* Facilities starting with $ are most likely targets */ + if (*name == '$') { + r = unit_name_build(n, NULL, ".target", ret); + if (r < 0) + return log_error_errno(r, "Failed to build name: %m"); + + return r; } - /* strip ".sh" suffix from file name for comparison */ + /* Strip ".sh" suffix from file name for comparison */ filename_no_sh = strdupa(filename); e = endswith(filename_no_sh, ".sh"); if (e) { @@ -313,103 +340,103 @@ static int sysv_translate_facility(const char *name, const char *filename, char filename = filename_no_sh; } - /* If we don't know this name, fallback heuristics to figure - * out whether something is a target or a service alias. */ - - if (*name == '$') { - int k; - - /* Facilities starting with $ are most likely targets */ - k = unit_name_build(n, NULL, ".target", &r); - if (k < 0) - return k; - - } else if (streq_ptr(n, filename)) - /* Names equaling the file name of the services are redundant */ + /* Names equaling the file name of the services are redundant */ + if (streq_ptr(n, filename)) return 0; - else - /* Everything else we assume to be normal service names */ - r = sysv_translate_name(n); - if (!r) - return -ENOMEM; -finish: - *_r = r; + /* Everything else we assume to be normal service names */ + m = sysv_translate_name(n); + if (!m) + return log_oom(); + *ret = m; return 1; } static int handle_provides(SysvStub *s, unsigned line, const char *full_text, const char *text) { - const char *word, *state_; - size_t z; int r; - FOREACH_WORD_QUOTED(word, z, text, state_) { - _cleanup_free_ char *n = NULL, *m = NULL; - UnitType t; + assert(s); + assert(full_text); + assert(text); - n = strndup(word, z); - if (!n) - return log_oom(); + for (;;) { + _cleanup_free_ char *word = NULL, *m = NULL; - r = sysv_translate_facility(n, basename(s->path), &m); + r = extract_first_word(&text, &word, NULL, EXTRACT_QUOTES|EXTRACT_RELAX); if (r < 0) - return r; + return log_error_errno(r, "Failed to parse word from provides string: %m"); if (r == 0) + break; + + r = sysv_translate_facility(word, basename(s->path), &m); + if (r <= 0) /* continue on error */ continue; - t = unit_name_to_type(m); - if (t == UNIT_SERVICE) { + switch (unit_name_to_type(m)) { + + case UNIT_SERVICE: log_debug("Adding Provides: alias '%s' for '%s'", m, s->name); r = add_alias(s->name, m); if (r < 0) log_warning_errno(r, "[%s:%u] Failed to add LSB Provides name %s, ignoring: %m", s->path, line, m); - } else if (t == UNIT_TARGET) { + break; + + case UNIT_TARGET: + /* NB: SysV targets which are provided by a * service are pulled in by the services, as * an indication that the generic service is * now available. This is strictly one-way. * The targets do NOT pull in SysV services! */ + r = strv_extend(&s->before, m); if (r < 0) return log_oom(); + r = strv_extend(&s->wants, m); if (r < 0) return log_oom(); + if (streq(m, SPECIAL_NETWORK_ONLINE_TARGET)) { r = strv_extend(&s->before, SPECIAL_NETWORK_TARGET); if (r < 0) return log_oom(); } - } else if (t == _UNIT_TYPE_INVALID) + + break; + + case _UNIT_TYPE_INVALID: log_warning("Unit name '%s' is invalid", m); - else + break; + + default: log_warning("Unknown unit type for unit '%s'", m); + } } - if (!isempty(state_)) - log_error("[%s:%u] Trailing garbage in Provides, ignoring.", s->path, line); + return 0; } static int handle_dependencies(SysvStub *s, unsigned line, const char *full_text, const char *text) { - const char *word, *state_; - size_t z; int r; - FOREACH_WORD_QUOTED(word, z, text, state_) { - _cleanup_free_ char *n = NULL, *m = NULL; - bool is_before; + assert(s); + assert(full_text); + assert(text); - n = strndup(word, z); - if (!n) - return log_oom(); + for (;;) { + _cleanup_free_ char *word = NULL, *m = NULL; + bool is_before; - r = sysv_translate_facility(n, basename(s->path), &m); - if (r < 0) { - log_warning_errno(r, "[%s:%u] Failed to translate LSB dependency %s, ignoring: %m", s->path, line, n); - continue; - } + r = extract_first_word(&text, &word, NULL, EXTRACT_QUOTES|EXTRACT_RELAX); + if (r < 0) + return log_error_errno(r, "Failed to parse word from provides string: %m"); if (r == 0) + break; + + r = sysv_translate_facility(word, basename(s->path), &m); + if (r <= 0) /* continue on error */ continue; is_before = startswith_no_case(full_text, "X-Start-Before:"); @@ -419,15 +446,14 @@ static int handle_dependencies(SysvStub *s, unsigned line, const char *full_text r = strv_extend(&s->after, m); if (r < 0) return log_oom(); + r = strv_extend(&s->wants, m); } else r = strv_extend(is_before ? &s->before : &s->after, m); - if (r < 0) return log_oom(); } - if (!isempty(state_)) - log_warning("[%s:%u] Trailing garbage in %*s, ignoring.", s->path, line, (int)(strchr(full_text, ':') - full_text), full_text); + return 0; } @@ -445,24 +471,22 @@ static int load_sysv(SysvStub *s) { _cleanup_free_ char *short_description = NULL, *long_description = NULL, *chkconfig_description = NULL; char *description; bool supports_reload = false; + char l[LINE_MAX]; assert(s); f = fopen(s->path, "re"); - if (!f) - return errno == ENOENT ? 0 : -errno; + if (!f) { + if (errno == ENOENT) + return 0; - log_debug("Loading SysV script %s", s->path); + return log_error_errno(errno, "Failed to open %s: %m", s->path); + } - while (!feof(f)) { - char l[LINE_MAX], *t; + log_debug("Loading SysV script %s", s->path); - if (!fgets(l, sizeof(l), f)) { - if (feof(f)) - break; - - return log_error_errno(errno, "Failed to read configuration file '%s': %m", s->path); - } + FOREACH_LINE(l, f, goto fail) { + char *t; line++; @@ -505,29 +529,25 @@ static int load_sysv(SysvStub *s) { if (startswith_no_case(t, "description:")) { - size_t k = strlen(t); - char *d; - const char *j; + size_t k; + const const char *j; - if (t[k-1] == '\\') { + k = strlen(t); + if (k > 0 && t[k-1] == '\\') { state = DESCRIPTION; t[k-1] = 0; } j = strstrip(t+12); - if (j && *j) { - d = strdup(j); - if (!d) - return -ENOMEM; - } else - d = NULL; + if (isempty(j)) + j = NULL; - free(chkconfig_description); - chkconfig_description = d; + r = free_and_strdup(&chkconfig_description, j); + if (r < 0) + return log_oom(); } else if (startswith_no_case(t, "pidfile:")) { - - char *fn; + const char *fn; state = NORMAL; @@ -537,12 +557,9 @@ static int load_sysv(SysvStub *s) { continue; } - fn = strdup(fn); - if (!fn) - return -ENOMEM; - - free(s->pid_file); - s->pid_file = fn; + r = free_and_strdup(&s->pid_file, fn); + if (r < 0) + return log_oom(); } } else if (state == DESCRIPTION) { @@ -550,25 +567,25 @@ static int load_sysv(SysvStub *s) { /* Try to parse Red Hat style description * continuation */ - size_t k = strlen(t); + size_t k; char *j; - if (t[k-1] == '\\') + k = strlen(t); + if (k > 0 && t[k-1] == '\\') t[k-1] = 0; else state = NORMAL; j = strstrip(t); - if (j && *j) { + if (!isempty(j)) { char *d = NULL; if (chkconfig_description) d = strjoin(chkconfig_description, " ", j, NULL); else d = strdup(j); - if (!d) - return -ENOMEM; + return log_oom(); free(chkconfig_description); chkconfig_description = d; @@ -582,6 +599,7 @@ static int load_sysv(SysvStub *s) { r = handle_provides(s, line, t, t + 9); if (r < 0) return r; + } else if (startswith_no_case(t, "Required-Start:") || startswith_no_case(t, "Should-Start:") || startswith_no_case(t, "X-Start-Before:") || @@ -593,55 +611,47 @@ static int load_sysv(SysvStub *s) { if (r < 0) return r; - } else if (startswith_no_case(t, "Description:")) { - char *d, *j; + const char *j; state = LSB_DESCRIPTION; j = strstrip(t+12); - if (j && *j) { - d = strdup(j); - if (!d) - return -ENOMEM; - } else - d = NULL; + if (isempty(j)) + j = NULL; - free(long_description); - long_description = d; + r = free_and_strdup(&long_description, j); + if (r < 0) + return log_oom(); } else if (startswith_no_case(t, "Short-Description:")) { - char *d, *j; + const char *j; state = LSB; j = strstrip(t+18); - if (j && *j) { - d = strdup(j); - if (!d) - return -ENOMEM; - } else - d = NULL; + if (isempty(j)) + j = NULL; - free(short_description); - short_description = d; + r = free_and_strdup(&short_description, j); + if (r < 0) + return log_oom(); } else if (state == LSB_DESCRIPTION) { if (startswith(l, "#\t") || startswith(l, "# ")) { - char *j; + const char *j; j = strstrip(t); - if (j && *j) { + if (!isempty(j)) { char *d = NULL; if (long_description) d = strjoin(long_description, " ", t, NULL); else d = strdup(j); - if (!d) - return -ENOMEM; + return log_oom(); free(long_description); long_description = d; @@ -672,12 +682,16 @@ static int load_sysv(SysvStub *s) { d = strappend(s->has_lsb ? "LSB: " : "SYSV: ", description); if (!d) - return -ENOMEM; + return log_oom(); s->description = d; } + s->loaded = true; return 0; + +fail: + return log_error_errno(errno, "Failed to read configuration file '%s': %m", s->path); } static int fix_order(SysvStub *s, Hashmap *all_services) { @@ -687,6 +701,9 @@ static int fix_order(SysvStub *s, Hashmap *all_services) { assert(s); + if (!s->loaded) + return 0; + if (s->sysv_start_priority < 0) return 0; @@ -694,6 +711,9 @@ static int fix_order(SysvStub *s, Hashmap *all_services) { if (s == other) continue; + if (!other->loaded) + continue; + if (other->sysv_start_priority < 0) continue; @@ -706,13 +726,12 @@ static int fix_order(SysvStub *s, Hashmap *all_services) { r = strv_extend(&s->after, other->name); if (r < 0) return log_oom(); - } - else if (other->sysv_start_priority > s->sysv_start_priority) { + + } else if (other->sysv_start_priority > s->sysv_start_priority) { r = strv_extend(&s->before, other->name); if (r < 0) return log_oom(); - } - else + } else continue; /* FIXME: Maybe we should compare the name here lexicographically? */ @@ -724,6 +743,9 @@ static int fix_order(SysvStub *s, Hashmap *all_services) { static int enumerate_sysv(const LookupPaths *lp, Hashmap *all_services) { char **path; + assert(lp); + assert(all_services); + STRV_FOREACH(path, lp->sysvinit_path) { _cleanup_closedir_ DIR *d = NULL; struct dirent *de; @@ -731,21 +753,18 @@ static int enumerate_sysv(const LookupPaths *lp, Hashmap *all_services) { d = opendir(*path); if (!d) { if (errno != ENOENT) - log_warning_errno(errno, "opendir(%s) failed: %m", *path); + log_warning_errno(errno, "Opening %s failed, ignoring: %m", *path); continue; } - while ((de = readdir(d))) { + FOREACH_DIRENT(de, d, log_error_errno(errno, "Failed to enumerate directory %s, ignoring: %m", *path)) { _cleanup_free_ char *fpath = NULL, *name = NULL; _cleanup_(free_sysvstubp) SysvStub *service = NULL; struct stat st; int r; - if (hidden_file(de->d_name)) - continue; - if (fstatat(dirfd(d), de->d_name, &st, 0) < 0) { - log_warning_errno(errno, "stat() failed on %s/%s: %m", *path, de->d_name); + log_warning_errno(errno, "stat() failed on %s/%s, ignoring: %m", *path, de->d_name); continue; } @@ -762,15 +781,15 @@ static int enumerate_sysv(const LookupPaths *lp, Hashmap *all_services) { if (hashmap_contains(all_services, name)) continue; - fpath = strjoin(*path, "/", de->d_name, NULL); - if (!fpath) - return log_oom(); - if (unit_file_lookup_state(UNIT_FILE_SYSTEM, NULL, lp, name) >= 0) { log_debug("Native unit for %s already exists, skipping", name); continue; } + fpath = strjoin(*path, "/", de->d_name, NULL); + if (!fpath) + return log_oom(); + service = new0(SysvStub, 1); if (!service) return log_oom(); @@ -778,12 +797,12 @@ static int enumerate_sysv(const LookupPaths *lp, Hashmap *all_services) { service->sysv_start_priority = -1; service->name = name; service->path = fpath; + name = fpath = NULL; r = hashmap_put(all_services, service->name, service); if (r < 0) return log_oom(); - name = fpath = NULL; service = NULL; } } @@ -792,43 +811,41 @@ static int enumerate_sysv(const LookupPaths *lp, Hashmap *all_services) { } static int set_dependencies_from_rcnd(const LookupPaths *lp, Hashmap *all_services) { - char **p; - unsigned i; - _cleanup_closedir_ DIR *d = NULL; - _cleanup_free_ char *path = NULL, *fpath = NULL; - SysvStub *service; - Iterator j; Set *runlevel_services[ELEMENTSOF(rcnd_table)] = {}; _cleanup_set_free_ Set *shutdown_services = NULL; - int r = 0; + SysvStub *service; + unsigned i; + Iterator j; + char **p; + int r; - STRV_FOREACH(p, lp->sysvrcnd_path) + assert(lp); + + STRV_FOREACH(p, lp->sysvrcnd_path) { for (i = 0; i < ELEMENTSOF(rcnd_table); i ++) { + + _cleanup_closedir_ DIR *d = NULL; + _cleanup_free_ char *path = NULL; struct dirent *de; - free(path); path = strjoin(*p, "/", rcnd_table[i].path, NULL); - if (!path) - return -ENOMEM; - - safe_closedir(d); + if (!path) { + r = log_oom(); + goto finish; + } d = opendir(path); if (!d) { if (errno != ENOENT) - log_warning_errno(errno, "opendir(%s) failed: %m", path); + log_warning_errno(errno, "Opening %s failed, ignoring: %m", path); continue; } - while ((de = readdir(d))) { - _cleanup_free_ char *name = NULL; - + FOREACH_DIRENT(de, d, log_error_errno(errno, "Failed to enumerate directory %s, ignoring: %m", path)) { + _cleanup_free_ char *name = NULL, *fpath = NULL; int a, b; - if (hidden_file(de->d_name)) - continue; - if (de->d_name[0] != 'S' && de->d_name[0] != 'K') continue; @@ -841,10 +858,9 @@ static int set_dependencies_from_rcnd(const LookupPaths *lp, Hashmap *all_servic if (a < 0 || b < 0) continue; - free(fpath); fpath = strjoin(*p, "/", de->d_name, NULL); if (!fpath) { - r = -ENOMEM; + r = log_oom(); goto finish; } @@ -856,64 +872,77 @@ static int set_dependencies_from_rcnd(const LookupPaths *lp, Hashmap *all_servic service = hashmap_get(all_services, name); if (!service){ - log_debug("Ignoring %s symlink in %s, not generating %s.", - de->d_name, rcnd_table[i].path, name); + log_debug("Ignoring %s symlink in %s, not generating %s.", de->d_name, rcnd_table[i].path, name); continue; } if (de->d_name[0] == 'S') { - if (rcnd_table[i].type == RUNLEVEL_UP) { - service->sysv_start_priority = - MAX(a*10 + b, service->sysv_start_priority); - } + if (rcnd_table[i].type == RUNLEVEL_UP) + service->sysv_start_priority = MAX(a*10 + b, service->sysv_start_priority); r = set_ensure_allocated(&runlevel_services[i], NULL); - if (r < 0) + if (r < 0) { + log_oom(); goto finish; + } r = set_put(runlevel_services[i], service); - if (r < 0) + if (r < 0) { + log_oom(); goto finish; + } } else if (de->d_name[0] == 'K' && (rcnd_table[i].type == RUNLEVEL_DOWN)) { r = set_ensure_allocated(&shutdown_services, NULL); - if (r < 0) + if (r < 0) { + log_oom(); goto finish; + } r = set_put(shutdown_services, service); - if (r < 0) + if (r < 0) { + log_oom(); goto finish; + } } } } + } for (i = 0; i < ELEMENTSOF(rcnd_table); i ++) SET_FOREACH(service, runlevel_services[i], j) { r = strv_extend(&service->before, rcnd_table[i].target); - if (r < 0) - return log_oom(); + if (r < 0) { + log_oom(); + goto finish; + } r = strv_extend(&service->wanted_by, rcnd_table[i].target); - if (r < 0) - return log_oom(); + if (r < 0) { + log_oom(); + goto finish; + } } SET_FOREACH(service, shutdown_services, j) { r = strv_extend(&service->before, SPECIAL_SHUTDOWN_TARGET); - if (r < 0) - return log_oom(); + if (r < 0) { + log_oom(); + goto finish; + } r = strv_extend(&service->conflicts, SPECIAL_SHUTDOWN_TARGET); - if (r < 0) - return log_oom(); + if (r < 0) { + log_oom(); + goto finish; + } } r = 0; finish: - for (i = 0; i < ELEMENTSOF(rcnd_table); i++) set_free(runlevel_services[i]); @@ -921,11 +950,11 @@ static int set_dependencies_from_rcnd(const LookupPaths *lp, Hashmap *all_servic } int main(int argc, char *argv[]) { - int r, q; - _cleanup_lookup_paths_free_ LookupPaths lp = {}; _cleanup_(free_sysvstub_hashmapp) Hashmap *all_services = NULL; + _cleanup_lookup_paths_free_ LookupPaths lp = {}; SysvStub *service; Iterator j; + int r; if (argc > 1 && argc != 4) { log_error("This program takes three or no arguments."); @@ -943,43 +972,34 @@ int main(int argc, char *argv[]) { r = lookup_paths_init(&lp, MANAGER_SYSTEM, true, NULL, NULL, NULL, NULL); if (r < 0) { - log_error("Failed to find lookup paths."); - return EXIT_FAILURE; + log_error_errno(r, "Failed to find lookup paths: %m"); + goto finish; } all_services = hashmap_new(&string_hash_ops); if (!all_services) { - log_oom(); - return EXIT_FAILURE; + r = log_oom(); + goto finish; } r = enumerate_sysv(&lp, all_services); - if (r < 0) { - log_error("Failed to generate units for all init scripts."); - return EXIT_FAILURE; - } + if (r < 0) + goto finish; r = set_dependencies_from_rcnd(&lp, all_services); - if (r < 0) { - log_error("Failed to read runlevels from rcnd links."); - return EXIT_FAILURE; - } + if (r < 0) + goto finish; - HASHMAP_FOREACH(service, all_services, j) { - q = load_sysv(service); - if (q < 0) - continue; - } + HASHMAP_FOREACH(service, all_services, j) + (void) load_sysv(service); HASHMAP_FOREACH(service, all_services, j) { - q = fix_order(service, all_services); - if (q < 0) - continue; - - q = generate_unit_file(service); - if (q < 0) - continue; + (void) fix_order(service, all_services); + (void) generate_unit_file(service); } - return EXIT_SUCCESS; + r = 0; + +finish: + return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } From cacea34bd161c31491349387db913b30508b6f11 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 30 Oct 2015 11:25:00 +0100 Subject: [PATCH 07/11] sysctl.d: bump number of queueable AF_UNIX/SOCK_DGRAM datagrams The default of 16 is pretty low, let's bump this to accomodate for more queued datagrams. This is useful for AF_UNIX/SOCK_DGRAM logging and sd_notify() sockets as this allows queuing more datagrams before things start to block, thus improving parallelization and logging performance. --- sysctl.d/50-default.conf | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sysctl.d/50-default.conf b/sysctl.d/50-default.conf index def151bb84981..2097551c33d83 100644 --- a/sysctl.d/50-default.conf +++ b/sysctl.d/50-default.conf @@ -35,6 +35,9 @@ net.ipv4.conf.all.promote_secondaries = 1 # Fair Queue CoDel packet scheduler to fight bufferbloat net.core.default_qdisc = fq_codel +# Make sure we can queue more than just a few datagrams in AF_UNIX sockets. +net.unix.max_dgram_qlen = 512 + # Enable hard and soft link protection fs.protected_hardlinks = 1 fs.protected_symlinks = 1 From a47806fafaec9a52a80e1795ad880b9b5008c4b8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 30 Oct 2015 11:27:29 +0100 Subject: [PATCH 08/11] sd-daemon: increase sd_notify() socket buffer size Let's make sure we don't start blocking on sd_notify() earlier than necessary, let's bump the socket buffer sizes to 8M. We already do something similar for our logging socket buffers, hence apply a similar bump here. --- src/core/manager.c | 4 ++++ src/libsystemd/sd-daemon/sd-daemon.c | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/core/manager.c b/src/core/manager.c index 2039dafea12c9..b13663e702fb3 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -86,6 +86,8 @@ #include "virt.h" #include "watchdog.h" +#define NOTIFY_RCVBUF_SIZE (8*1024*1024) + /* Initial delay and the interval for printing status messages about running jobs */ #define JOBS_IN_PROGRESS_WAIT_USEC (5*USEC_PER_SEC) #define JOBS_IN_PROGRESS_PERIOD_USEC (USEC_PER_SEC / 3) @@ -689,6 +691,8 @@ static int manager_setup_notify(Manager *m) { if (fd < 0) return log_error_errno(errno, "Failed to allocate notification socket: %m"); + fd_inc_rcvbuf(fd, NOTIFY_RCVBUF_SIZE); + if (m->running_as == MANAGER_SYSTEM) m->notify_socket = strdup("/run/systemd/notify"); else { diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c index 77b5dd52f6ee8..c7887804df43a 100644 --- a/src/libsystemd/sd-daemon/sd-daemon.c +++ b/src/libsystemd/sd-daemon/sd-daemon.c @@ -44,6 +44,8 @@ #include "strv.h" #include "util.h" +#define SNDBUF_SIZE (8*1024*1024) + static void unsetenv_all(bool unset_environment) { if (!unset_environment) @@ -440,6 +442,8 @@ _public_ int sd_pid_notify_with_fds(pid_t pid, int unset_environment, const char goto finish; } + fd_inc_sndbuf(fd, SNDBUF_SIZE); + iovec.iov_len = strlen(state); strncpy(sockaddr.un.sun_path, e, sizeof(sockaddr.un.sun_path)); From 638b56cd3c703651013033b82000d2a9f2732048 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 1 Nov 2015 21:49:19 +0100 Subject: [PATCH 09/11] sd-daemon: verify NOTIFY_SOCKET path length Better generate a real error then simply connect to the wrong socket. --- src/libsystemd/sd-daemon/sd-daemon.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c index c7887804df43a..27045e25d0c58 100644 --- a/src/libsystemd/sd-daemon/sd-daemon.c +++ b/src/libsystemd/sd-daemon/sd-daemon.c @@ -436,6 +436,11 @@ _public_ int sd_pid_notify_with_fds(pid_t pid, int unset_environment, const char goto finish; } + if (strlen(e) > sizeof(sockaddr.un.sun_path)) { + r = -EINVAL; + goto finish; + } + fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0); if (fd < 0) { r = -errno; From e22aa3d3284709234f086ebebc13a905a295b7a7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 1 Nov 2015 21:50:24 +0100 Subject: [PATCH 10/11] journald: never block when sending messages on NOTIFY_SOCKET socket Otherwise we might run into deadlocks, when journald blocks on the notify socket on PID 1, and PID 1 blocks on IPC to dbus-daemon and dbus-daemon blocks on logging to journald. Break this cycle by making sure that journald never ever blocks on PID 1. Note that this change disables support for event loop watchdog support, as these messages are sent in blocking style by sd-event. That should not be a big loss though, as people reported frequent problems with the watchdog hitting journald on excessively slow IO. Fixes: #1505. --- src/journal/journald-server.c | 130 +++++++++++++++++++++++++++++- src/journal/journald-server.h | 13 ++- src/journal/journald-stream.c | 68 ++++++++++++++-- src/journal/journald-stream.h | 4 + src/journal/journald.c | 8 -- units/systemd-journald.service.in | 1 - 6 files changed, 202 insertions(+), 22 deletions(-) diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index ecf7b7a47669f..299b0a848fb12 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -78,6 +78,8 @@ #define RECHECK_SPACE_USEC (30*USEC_PER_SEC) +#define NOTIFY_SNDBUF_SIZE (8*1024*1024) + static int determine_space_for( Server *s, JournalMetrics *metrics, @@ -1457,6 +1459,126 @@ static int server_open_hostname(Server *s) { return 0; } +static int dispatch_notify_event(sd_event_source *es, int fd, uint32_t revents, void *userdata) { + Server *s = userdata; + int r; + + assert(s); + assert(s->notify_event_source == es); + assert(s->notify_fd == fd); + + if (revents != EPOLLOUT) { + log_error("Invalid events on notify file descriptor."); + return -EINVAL; + } + + /* The $NOTIFY_SOCKET is writable again, now send exactly one + * message on it. Either it's the initial READY=1 event or an + * stdout stream event. If there's nothing to write anymore, + * turn our event source off. The next time there's something + * to send it will be turned on again. */ + + if (!s->sent_notify_ready) { + static const char p[] = + "READY=1\n" + "STATUS=Processing requests..."; + ssize_t l; + + l = send(s->notify_fd, p, strlen(p), MSG_DONTWAIT); + if (l < 0) { + if (errno == EAGAIN) + return 0; + + return log_error_errno(errno, "Failed to send READY=1 notification message: %m"); + } + + s->sent_notify_ready = true; + log_debug("Sent READY=1 notification."); + + } else if (s->stdout_streams_notify_queue) + /* Dispatch one stream notification event */ + stdout_stream_send_notify(s->stdout_streams_notify_queue); + + /* Leave us enabled if there's still more to to do. */ + if (s->stdout_streams_notify_queue) + return 0; + + /* There was nothing to do anymore, let's turn ourselves off. */ + r = sd_event_source_set_enabled(es, SD_EVENT_OFF); + if (r < 0) + return log_error_errno(r, "Failed to turn off notify event source: %m"); + + return 0; +} + +static int server_connect_notify(Server *s) { + union sockaddr_union sa = { + .un.sun_family = AF_UNIX, + }; + const char *e; + int r; + + assert(s); + assert(s->notify_fd < 0); + assert(!s->notify_event_source); + + /* + So here's the problem: we'd like to send notification + messages to PID 1, but we cannot do that via sd_notify(), + since that's synchronous, and we might end up blocking on + it. Specifically: given that PID 1 might block on + dbus-daemon during IPC, and dbus-daemon is logging to us, + and might hence block on us, we might end up in a deadlock + if we block on sending PID 1 notification messages -- by + generating a full blocking circle. To avoid this, let's + create a non-blocking socket, and connect it to the + notification socket, and then wait for POLLOUT before we + send anything. This should efficiently avoid any deadlocks, + as we'll never block on PID 1, hence PID 1 can safely block + on dbus-daemon which can safely block on us again. + + Don't think that this issue is real? It is, see: + https://github.com/systemd/systemd/issues/1505 + */ + + e = getenv("NOTIFY_SOCKET"); + if (!e) + return 0; + + if ((e[0] != '@' && e[0] != '/') || e[1] == 0) { + log_error("NOTIFY_SOCKET set to an invalid value: %s", e); + return -EINVAL; + } + + if (strlen(e) > sizeof(sa.un.sun_path)) { + log_error("NOTIFY_SOCKET path too long: %s", e); + return -EINVAL; + } + + s->notify_fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0); + if (s->notify_fd < 0) + return log_error_errno(errno, "Failed to create notify socket: %m"); + + (void) fd_inc_sndbuf(s->notify_fd, NOTIFY_SNDBUF_SIZE); + + strncpy(sa.un.sun_path, e, sizeof(sa.un.sun_path)); + if (sa.un.sun_path[0] == '@') + sa.un.sun_path[0] = 0; + + r = connect(s->notify_fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(e)); + if (r < 0) + return log_error_errno(errno, "Failed to connect to notify socket: %m"); + + r = sd_event_add_io(s->event, &s->notify_event_source, s->notify_fd, EPOLLOUT, dispatch_notify_event, s); + if (r < 0) + return log_error_errno(r, "Failed to watch notification socket: %m"); + + /* This should fire pretty soon, which we'll use to send the + * READY=1 event. */ + + return 0; +} + int server_init(Server *s) { _cleanup_fdset_free_ FDSet *fds = NULL; int n, r, fd; @@ -1465,7 +1587,7 @@ int server_init(Server *s) { assert(s); zero(*s); - s->syslog_fd = s->native_fd = s->stdout_fd = s->dev_kmsg_fd = s->audit_fd = s->hostname_fd = -1; + s->syslog_fd = s->native_fd = s->stdout_fd = s->dev_kmsg_fd = s->audit_fd = s->hostname_fd = s->notify_fd = -1; s->compress = true; s->seal = true; @@ -1511,8 +1633,6 @@ int server_init(Server *s) { if (r < 0) return log_error_errno(r, "Failed to create event loop: %m"); - sd_event_set_watchdog(s->event, true); - n = sd_listen_fds(true); if (n < 0) return log_error_errno(n, "Failed to read listening file descriptors from environment: %m"); @@ -1637,6 +1757,8 @@ int server_init(Server *s) { server_cache_boot_id(s); server_cache_machine_id(s); + (void) server_connect_notify(s); + return system_journal_open(s, false); } @@ -1685,6 +1807,7 @@ void server_done(Server *s) { sd_event_source_unref(s->sigterm_event_source); sd_event_source_unref(s->sigint_event_source); sd_event_source_unref(s->hostname_event_source); + sd_event_source_unref(s->notify_event_source); sd_event_unref(s->event); safe_close(s->syslog_fd); @@ -1693,6 +1816,7 @@ void server_done(Server *s) { safe_close(s->dev_kmsg_fd); safe_close(s->audit_fd); safe_close(s->hostname_fd); + safe_close(s->notify_fd); if (s->rate_limit) journal_rate_limit_free(s->rate_limit); diff --git a/src/journal/journald-server.h b/src/journal/journald-server.h index a2631c6017573..170602ea16614 100644 --- a/src/journal/journald-server.h +++ b/src/journal/journald-server.h @@ -26,9 +26,12 @@ #include "sd-event.h" +typedef struct Server Server; + #include "hashmap.h" #include "journal-file.h" #include "journald-rate-limit.h" +#include "journald-stream.h" #include "list.h" typedef enum Storage { @@ -48,15 +51,14 @@ typedef enum SplitMode { _SPLIT_INVALID = -1 } SplitMode; -typedef struct StdoutStream StdoutStream; - -typedef struct Server { +struct Server { int syslog_fd; int native_fd; int stdout_fd; int dev_kmsg_fd; int audit_fd; int hostname_fd; + int notify_fd; sd_event *event; @@ -71,6 +73,7 @@ typedef struct Server { sd_event_source *sigterm_event_source; sd_event_source *sigint_event_source; sd_event_source *hostname_event_source; + sd_event_source *notify_event_source; JournalFile *runtime_journal; JournalFile *system_journal; @@ -111,6 +114,7 @@ typedef struct Server { usec_t oldest_file_usec; LIST_HEAD(StdoutStream, stdout_streams); + LIST_HEAD(StdoutStream, stdout_streams_notify_queue); unsigned n_stdout_streams; char *tty_path; @@ -132,6 +136,7 @@ typedef struct Server { struct udev *udev; + bool sent_notify_ready; bool sync_scheduled; char machine_id_field[sizeof("_MACHINE_ID=") + 32]; @@ -140,7 +145,7 @@ typedef struct Server { /* Cached cgroup root, so that we don't have to query that all the time */ char *cgroup_root; -} Server; +}; #define SERVER_MACHINE_ID(s) ((s)->machine_id_field + strlen("_MACHINE_ID=")) diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c index 5300c61c02225..fb6afee1713cb 100644 --- a/src/journal/journald-stream.c +++ b/src/journal/journald-stream.c @@ -79,6 +79,7 @@ struct StdoutStream { bool forward_to_console:1; bool fdstore:1; + bool in_notify_queue:1; char buffer[LINE_MAX+1]; size_t length; @@ -88,6 +89,7 @@ struct StdoutStream { char *state_file; LIST_FIELDS(StdoutStream, stdout_stream); + LIST_FIELDS(StdoutStream, stdout_stream_notify_queue); }; void stdout_stream_free(StdoutStream *s) { @@ -98,6 +100,9 @@ void stdout_stream_free(StdoutStream *s) { assert(s->server->n_stdout_streams > 0); s->server->n_stdout_streams --; LIST_REMOVE(stdout_stream, s->server->stdout_streams, s); + + if (s->in_notify_queue) + LIST_REMOVE(stdout_stream_notify_queue, s->server->stdout_streams_notify_queue, s); } if (s->event_source) { @@ -121,7 +126,7 @@ static void stdout_stream_destroy(StdoutStream *s) { return; if (s->state_file) - unlink(s->state_file); + (void) unlink(s->state_file); stdout_stream_free(s); } @@ -200,11 +205,15 @@ static int stdout_stream_save(StdoutStream *s) { goto fail; } - /* Store the connection fd in PID 1, so that we get it passed - * in again on next start */ - if (!s->fdstore) { - sd_pid_notify_with_fds(0, false, "FDSTORE=1", &s->fd, 1); - s->fdstore = true; + if (!s->fdstore && !s->in_notify_queue) { + LIST_PREPEND(stdout_stream_notify_queue, s->server->stdout_streams_notify_queue, s); + s->in_notify_queue = true; + + if (s->server->notify_event_source) { + r = sd_event_source_set_enabled(s->server->notify_event_source, SD_EVENT_ON); + if (r < 0) + log_warning_errno(r, "Failed to enable notify event source: %m"); + } } return 0; @@ -729,3 +738,50 @@ int server_open_stdout_socket(Server *s) { return 0; } + +void stdout_stream_send_notify(StdoutStream *s) { + struct iovec iovec = { + .iov_base = (char*) "FDSTORE=1", + .iov_len = strlen("FDSTORE=1"), + }; + struct msghdr msghdr = { + .msg_iov = &iovec, + .msg_iovlen = 1, + }; + struct cmsghdr *cmsg; + ssize_t l; + + assert(s); + assert(!s->fdstore); + assert(s->in_notify_queue); + assert(s->server); + assert(s->server->notify_fd >= 0); + + /* Store the connection fd in PID 1, so that we get it passed + * in again on next start */ + + msghdr.msg_controllen = CMSG_SPACE(sizeof(int)); + msghdr.msg_control = alloca0(msghdr.msg_controllen); + + cmsg = CMSG_FIRSTHDR(&msghdr); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_len = CMSG_LEN(sizeof(int)); + + memcpy(CMSG_DATA(cmsg), &s->fd, sizeof(int)); + + l = sendmsg(s->server->notify_fd, &msghdr, MSG_DONTWAIT|MSG_NOSIGNAL); + if (l < 0) { + if (errno == EAGAIN) + return; + + log_error_errno(errno, "Failed to send stream file descriptor to service manager: %m"); + } else { + log_debug("Successfully sent stream file descriptor to service manager."); + s->fdstore = 1; + } + + LIST_REMOVE(stdout_stream_notify_queue, s->server->stdout_streams_notify_queue, s); + s->in_notify_queue = false; + +} diff --git a/src/journal/journald-stream.h b/src/journal/journald-stream.h index 257dce45df350..e3497f0dedb02 100644 --- a/src/journal/journald-stream.h +++ b/src/journal/journald-stream.h @@ -21,9 +21,13 @@ along with systemd; If not, see . ***/ +typedef struct StdoutStream StdoutStream; + #include "fdset.h" #include "journald-server.h" int server_open_stdout_socket(Server *s); int server_restore_streams(Server *s, FDSet *fds); + void stdout_stream_free(StdoutStream *s); +void stdout_stream_send_notify(StdoutStream *s); diff --git a/src/journal/journald.c b/src/journal/journald.c index 83236ceba9b46..b137e3c7beadb 100644 --- a/src/journal/journald.c +++ b/src/journal/journald.c @@ -61,10 +61,6 @@ int main(int argc, char *argv[]) { log_debug("systemd-journald running as pid "PID_FMT, getpid()); server_driver_message(&server, SD_MESSAGE_JOURNAL_START, "Journal started"); - sd_notify(false, - "READY=1\n" - "STATUS=Processing requests..."); - for (;;) { usec_t t = USEC_INFINITY, n; @@ -117,10 +113,6 @@ int main(int argc, char *argv[]) { server_driver_message(&server, SD_MESSAGE_JOURNAL_STOP, "Journal stopped"); finish: - sd_notify(false, - "STOPPING=1\n" - "STATUS=Shutting down..."); - server_done(&server); return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; diff --git a/units/systemd-journald.service.in b/units/systemd-journald.service.in index 41bfde5be3c5d..2552102bfca02 100644 --- a/units/systemd-journald.service.in +++ b/units/systemd-journald.service.in @@ -22,7 +22,6 @@ RestartSec=0 NotifyAccess=all StandardOutput=null CapabilityBoundingSet=CAP_SYS_ADMIN CAP_DAC_OVERRIDE CAP_SYS_PTRACE CAP_SYSLOG CAP_AUDIT_CONTROL CAP_AUDIT_READ CAP_CHOWN CAP_DAC_READ_SEARCH CAP_FOWNER CAP_SETUID CAP_SETGID CAP_MAC_OVERRIDE -WatchdogSec=3min FileDescriptorStoreMax=1024 # Increase the default a bit in order to allow many simultaneous From 3958325852869a5e490b5741016c93b8b9a80e11 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 1 Nov 2015 21:59:17 +0100 Subject: [PATCH 11/11] journal-remote: remove unused variable warning when building without GNUTLS. --- src/journal-remote/journal-remote.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/journal-remote/journal-remote.c b/src/journal-remote/journal-remote.c index 20be5420963dc..dc69bb8679d00 100644 --- a/src/journal-remote/journal-remote.c +++ b/src/journal-remote/journal-remote.c @@ -1256,7 +1256,6 @@ static int parse_argv(int argc, char *argv[]) { }; int c, r; - const char *p; bool type_a, type_b; assert(argc >= 0); @@ -1417,7 +1416,7 @@ static int parse_argv(int argc, char *argv[]) { case ARG_GNUTLS_LOG: { #ifdef HAVE_GNUTLS - p = optarg; + const char* p = optarg; for (;;) { _cleanup_free_ char *word = NULL;