Merge pull request #26021 from fbuihuu/some-journald-improvements

Some journald improvements

src/journal/journald-server.c

@@ -416,9 +416,44 @@ static int system_journal_open(Server *s, bool flush_requested, bool relinquish_
         return r;
 }
 
-static ManagedJournalFile* find_journal(Server *s, uid_t uid) {
+static int find_user_journal(Server *s, uid_t uid, ManagedJournalFile **ret) {
+        _cleanup_(managed_journal_file_closep) ManagedJournalFile *f = NULL;
         _cleanup_free_ char *p = NULL;
-        ManagedJournalFile *f;
+        int r;
+
+        assert(!uid_for_system_journal(uid));
+
+        f = ordered_hashmap_get(s->user_journals, UID_TO_PTR(uid));
+        if (f)
+                goto found;
+
+        if (asprintf(&p, "%s/user-" UID_FMT ".journal", s->system_storage.path, uid) < 0)
+                return log_oom();
+
+        /* Too many open? Then let's close one (or more) */
+        while (ordered_hashmap_size(s->user_journals) >= USER_JOURNALS_MAX) {
+                ManagedJournalFile *first;
+
+                assert_se(first = ordered_hashmap_steal_first(s->user_journals));
+                (void) managed_journal_file_close(first);
+        }
+
+        r = open_journal(s, true, p, O_RDWR|O_CREAT, s->seal, &s->system_storage.metrics, &f);
+        if (r < 0)
+                return r;
+
+        r = ordered_hashmap_put(s->user_journals, UID_TO_PTR(uid), f);
+        if (r < 0)
+                return r;
+
+        server_add_acls(f, uid);
+
+found:
+        *ret = TAKE_PTR(f);
+        return 0;
+}
+
+static ManagedJournalFile* find_journal(Server *s, uid_t uid) {
         int r;
 
         assert(s);
@@ -446,36 +481,17 @@ static ManagedJournalFile* find_journal(Server *s, uid_t uid) {
         if (!IN_SET(s->storage, STORAGE_AUTO, STORAGE_PERSISTENT))
                 return NULL;
 
-        if (uid_for_system_journal(uid))
-                return s->system_journal;
+        if (!uid_for_system_journal(uid)) {
+                ManagedJournalFile *f = NULL;
 
-        f = ordered_hashmap_get(s->user_journals, UID_TO_PTR(uid));
-        if (f)
-                return f;
+                r = find_user_journal(s, uid, &f);
+                if (r >= 0)
+                        return ASSERT_PTR(f);
 
-        if (asprintf(&p, "%s/user-" UID_FMT ".journal", s->system_storage.path, uid) < 0) {
-                log_oom();
-                return s->system_journal;
+                log_warning_errno(r, "Failed to open user journal file, falling back to system journal: %m");
         }
 
-        /* Too many open? Then let's close one (or more) */
-        while (ordered_hashmap_size(s->user_journals) >= USER_JOURNALS_MAX) {
-                assert_se(f = ordered_hashmap_steal_first(s->user_journals));
-                (void) managed_journal_file_close(f);
-        }
-
-        r = open_journal(s, true, p, O_RDWR|O_CREAT, s->seal, &s->system_storage.metrics, &f);
-        if (r < 0)
-                return s->system_journal;
-
-        r = ordered_hashmap_put(s->user_journals, UID_TO_PTR(uid), f);
-        if (r < 0) {
-                (void) managed_journal_file_close(f);
-                return s->system_journal;
-        }
-
-        server_add_acls(f, uid);
-        return f;
+        return s->system_journal;
 }
 
 static int do_rotate(
@@ -543,7 +559,7 @@ static void server_vacuum_deferred_closes(Server *s) {
         }
 }
 
-static int vacuum_offline_user_journals(Server *s) {
+static int server_archive_offline_user_journals(Server *s) {
         _cleanup_closedir_ DIR *d = NULL;
         int r;
 
@@ -559,9 +575,8 @@ static int vacuum_offline_user_journals(Server *s) {
         }
 
         for (;;) {
-                _cleanup_free_ char *u = NULL, *full = NULL;
+                _cleanup_free_ char *full = NULL;
                 _cleanup_close_ int fd = -EBADF;
-                const char *a, *b;
                 struct dirent *de;
                 ManagedJournalFile *f;
                 uid_t uid;
@@ -573,24 +588,14 @@ static int vacuum_offline_user_journals(Server *s) {
                                 log_ratelimit_warning_errno(errno, JOURNAL_LOG_RATELIMIT,
                                                             "Failed to enumerate %s, ignoring: %m",
                                                             s->system_storage.path);
-
                         break;
                 }
 
-                a = startswith(de->d_name, "user-");
-                if (!a)
-                        continue;
-                b = endswith(de->d_name, ".journal");
-                if (!b)
-                        continue;
-
-                u = strndup(a, b-a);
-                if (!u)
-                        return log_oom();
-
-                r = parse_uid(u, &uid);
+                r = journal_file_parse_uid_from_filename(de->d_name, &uid);
                 if (r < 0) {
-                        log_debug_errno(r, "Failed to parse UID from file name '%s', ignoring: %m", de->d_name);
+                        /* Don't warn if the file is not an online or offline user journal. */
+                        if (r != -EREMOTE)
+                                log_warning_errno(r, "Failed to parse UID from file name '%s', ignoring: %m", de->d_name);
                         continue;
                 }
 
@@ -680,7 +685,7 @@ void server_rotate(Server *s) {
         /* Finally, also rotate all user journals we currently do not have open. (But do so only if we
          * actually have access to /var, i.e. are not in the log-to-runtime-journal mode). */
         if (!s->runtime_journal)
-                (void) vacuum_offline_user_journals(s);
+                (void) server_archive_offline_user_journals(s);
 
         server_process_deferred_closes(s);
 }

src/libsystemd/meson.build

@@ -191,6 +191,8 @@ libsystemd_pc = custom_target(
 ############################################################
 
 tests += [
+        [files('sd-journal/test-journal-file.c')],
+
         [files('sd-journal/test-journal-send.c')],
 
         [files('sd-journal/test-journal-match.c')],

src/libsystemd/sd-journal/journal-file.c

@@ -35,6 +35,7 @@
 #include "string-util.h"
 #include "strv.h"
 #include "sync-util.h"
+#include "user-util.h"
 #include "xattr-util.h"
 
 #define DEFAULT_DATA_HASH_TABLE_SIZE (2047ULL*sizeof(HashItem))
@@ -3932,6 +3933,41 @@ fail:
         return r;
 }
 
+int journal_file_parse_uid_from_filename(const char *path, uid_t *ret_uid) {
+        _cleanup_free_ char *buf = NULL, *p = NULL;
+        const char *a, *b, *at;
+        int r;
+
+        /* This helper returns -EREMOTE when the filename doesn't match user online/offline journal
+         * pattern. Hence it currently doesn't parse archived or disposed user journals. */
+
+        assert(path);
+        assert(ret_uid);
+
+        r = path_extract_filename(path, &p);
+        if (r < 0)
+                return r;
+        if (r == O_DIRECTORY)
+                return -EISDIR;
+
+        a = startswith(p, "user-");
+        if (!a)
+                return -EREMOTE;
+        b = endswith(p, ".journal");
+        if (!b)
+                return -EREMOTE;
+
+        at = strchr(a, '@');
+        if (at)
+                return -EREMOTE;
+
+        buf = strndup(a, b-a);
+        if (!buf)
+                return -ENOMEM;
+
+        return parse_uid(buf, ret_uid);
+}
+
 int journal_file_archive(JournalFile *f, char **ret_previous_path) {
         _cleanup_free_ char *p = NULL;
 

src/libsystemd/sd-journal/journal-file.h

@@ -288,6 +288,7 @@ void journal_file_dump(JournalFile *f);
 void journal_file_print_header(JournalFile *f);
 
 int journal_file_archive(JournalFile *f, char **ret_previous_path);
+int journal_file_parse_uid_from_filename(const char *path, uid_t *uid);
 JournalFile* journal_initiate_close(JournalFile *f, Set *deferred_closes);
 
 int journal_file_dispose(int dir_fd, const char *fname);

src/libsystemd/sd-journal/test-journal-file.c

@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include <errno.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "journal-file.h"
+#include "tests.h"
+#include "user-util.h"
+
+static void test_journal_file_parse_uid_from_filename_simple(
+                const char *path,
+                uid_t expected_uid,
+                int expected_error) {
+
+        uid_t uid = UID_INVALID;
+        int r;
+
+        log_info("testing %s", path);
+
+        r = journal_file_parse_uid_from_filename(path, &uid);
+        assert_se(r == expected_error);
+        if (r < 0)
+                assert_se(uid == UID_INVALID);
+        else
+                assert_se(uid == expected_uid);
+}
+
+TEST(journal_file_parse_uid_from_filename) {
+
+        test_journal_file_parse_uid_from_filename_simple("/var/log/journal/", 0, -EISDIR);
+
+        /* The helper should return -EREMOTE for any filenames that don't look like an online or offline user
+         * journals. This includes archived and disposed journal files. */
+        test_journal_file_parse_uid_from_filename_simple("/etc/password", 0, -EREMOTE);
+        test_journal_file_parse_uid_from_filename_simple("system.journal", 0, -EREMOTE);
+        test_journal_file_parse_uid_from_filename_simple("user-1000@0005d26980bdce6e-2f2a4939583822ef.journal~", 0, -EREMOTE);
+        test_journal_file_parse_uid_from_filename_simple("user-1000@xxx-yyy-zzz.journal", 0, -EREMOTE);
+
+        test_journal_file_parse_uid_from_filename_simple("user-1000.journal", 1000, 0);
+        test_journal_file_parse_uid_from_filename_simple("user-foo.journal", 0, -EINVAL);
+        test_journal_file_parse_uid_from_filename_simple("user-65535.journal", 0, -ENXIO);
+}
+
+DEFINE_TEST_MAIN(LOG_INFO);