From 27c3112dcbd1b5f171c36c32550d9c6331375b0b Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 17 Sep 2019 11:16:52 +0200 Subject: [PATCH] fs-util: introduce inotify_add_watch_and_warn() helper The default message for ENOSPC is very misleading: it says that the disk is filled, but in fact the inotify watch limit is the problem. So let's introduce and use a wrapper that simply calls inotify_add_watch(2) and which fixes the error message up in case ENOSPC is returned. --- src/basic/fs-util.c | 15 +++++++ src/basic/fs-util.h | 1 + src/core/manager.c | 8 ++-- src/core/path.c | 39 +++++++++++-------- src/time-wait-sync/time-wait-sync.c | 4 +- .../tty-ask-password-agent.c | 11 ++---- 6 files changed, 49 insertions(+), 29 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index b2ac648838..a92241ca02 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -660,6 +660,21 @@ int inotify_add_watch_fd(int fd, int what, uint32_t mask) { return r; } +int inotify_add_watch_and_warn(int fd, const char *pathname, uint32_t mask) { + if (inotify_add_watch(fd, pathname, mask) < 0) { + const char *reason; + + if (errno == ENOSPC) + reason = "inotify watch limit reached"; + else + reason = strerror_safe(errno); + + return log_error_errno(errno, "Failed to add a watch for %s: %s", pathname, reason); + } + + return 0; +} + static bool unsafe_transition(const struct stat *a, const struct stat *b) { /* Returns true if the transition from a to b is safe, i.e. that we never transition from unprivileged to * privileged files or directories. Why bother? So that unprivileged code can't symlink to privileged files diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index 1f0bdd95b3..fa0f0de9a3 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -72,6 +72,7 @@ union inotify_event_buffer { }; int inotify_add_watch_fd(int fd, int what, uint32_t mask); +int inotify_add_watch_and_warn(int fd, const char *pathname, uint32_t mask); enum { CHASE_PREFIX_ROOT = 1 << 0, /* The specified path will be prefixed by the specified root before beginning the iteration */ diff --git a/src/core/manager.c b/src/core/manager.c index 600718e4bf..5f3cb6ea62 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -295,10 +295,12 @@ static int manager_check_ask_password(Manager *m) { if (m->ask_password_inotify_fd < 0) return log_error_errno(errno, "Failed to create inotify object: %m"); - if (inotify_add_watch(m->ask_password_inotify_fd, "/run/systemd/ask-password", IN_CREATE|IN_DELETE|IN_MOVE) < 0) { - log_error_errno(errno, "Failed to watch \"/run/systemd/ask-password\": %m"); + r = inotify_add_watch_and_warn(m->ask_password_inotify_fd, + "/run/systemd/ask-password", + IN_CREATE|IN_DELETE|IN_MOVE); + if (r < 0) { manager_close_ask_password(m); - return -errno; + return r; } r = sd_event_add_io(m->event, &m->ask_password_event_source, diff --git a/src/core/path.c b/src/core/path.c index ac1289a658..aee94ce7f0 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -89,24 +89,29 @@ int path_spec_watch(PathSpec *s, sd_event_io_handler_t handler) { break; } - r = log_warning_errno(errno, "Failed to add watch on %s: %s", s->path, errno == ENOSPC ? "too many watches" : strerror_safe(r)); - if (cut) - *cut = tmp; - goto fail; - } else { - exists = true; - - /* Path exists, we don't need to watch parent too closely. */ - if (oldslash) { - char *cut2 = oldslash + (oldslash == s->path); - char tmp2 = *cut2; - *cut2 = '\0'; - - (void) inotify_add_watch(s->inotify_fd, s->path, IN_MOVE_SELF); - /* Error is ignored, the worst can happen is we get spurious events. */ - - *cut2 = tmp2; + /* This second call to inotify_add_watch() should fail like the previous + * one and is done for logging the error in a comprehensive way. */ + r = inotify_add_watch_and_warn(s->inotify_fd, s->path, flags); + if (r < 0) { + if (cut) + *cut = tmp; + goto fail; } + + /* Hmm, we succeeded in adding the watch this time... let's continue. */ + } + exists = true; + + /* Path exists, we don't need to watch parent too closely. */ + if (oldslash) { + char *cut2 = oldslash + (oldslash == s->path); + char tmp2 = *cut2; + *cut2 = '\0'; + + (void) inotify_add_watch(s->inotify_fd, s->path, IN_MOVE_SELF); + /* Error is ignored, the worst can happen is we get spurious events. */ + + *cut2 = tmp2; } if (cut) diff --git a/src/time-wait-sync/time-wait-sync.c b/src/time-wait-sync/time-wait-sync.c index f4d20af2d4..5b27df1f9e 100644 --- a/src/time-wait-sync/time-wait-sync.c +++ b/src/time-wait-sync/time-wait-sync.c @@ -225,9 +225,9 @@ static int run(int argc, char * argv[]) { if (r < 0) return log_error_errno(r, "Failed to create notify event source: %m"); - r = inotify_add_watch(state.inotify_fd, "/run/systemd/", IN_CREATE); + r = inotify_add_watch_and_warn(state.inotify_fd, "/run/systemd/", IN_CREATE); if (r < 0) - return log_error_errno(errno, "Failed to watch /run/systemd/: %m"); + return r; state.run_systemd_wd = r; diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 0553abca64..f57f0ec261 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -29,6 +28,7 @@ #include "exit-status.h" #include "fd-util.h" #include "fileio.h" +#include "fs-util.h" #include "hashmap.h" #include "io-util.h" #include "macro.h" @@ -510,12 +510,9 @@ static int process_and_watch_password_files(void) { if (notify < 0) return log_error_errno(errno, "Failed to allocate directory watch: %m"); - if (inotify_add_watch(notify, "/run/systemd/ask-password", IN_CLOSE_WRITE|IN_MOVED_TO) < 0) { - if (errno == ENOSPC) - return log_error_errno(errno, "Failed to add /run/systemd/ask-password to directory watch: inotify watch limit reached"); - else - return log_error_errno(errno, "Failed to add /run/systemd/ask-password to directory watch: %m"); - } + r = inotify_add_watch_and_warn(notify, "/run/systemd/ask-password", IN_CLOSE_WRITE|IN_MOVED_TO); + if (r < 0) + return r; assert_se(sigemptyset(&mask) >= 0); assert_se(sigset_add_many(&mask, SIGINT, SIGTERM, -1) >= 0);