From 8ac0810f6c509a9e69ddfc7f999572684625d08f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 Apr 2024 12:44:49 +0900 Subject: [PATCH 01/11] logs-show: drop uid argument from add_matches_for_user_units() It is always equivalent to getuid(). Let's call getuid() in the function instead. --- src/journal/journalctl-filter.c | 4 ++-- src/login/loginctl.c | 18 ++++++++---------- src/machine/machinectl.c | 9 ++++----- src/shared/logs-show.c | 6 +++--- src/shared/logs-show.h | 4 +--- src/systemctl/systemctl-show.c | 3 +-- 6 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/journal/journalctl-filter.c b/src/journal/journalctl-filter.c index 1fdde5eb37..039fa5dc8b 100644 --- a/src/journal/journalctl-filter.c +++ b/src/journal/journalctl-filter.c @@ -208,7 +208,7 @@ static int add_units(sd_journal *j) { if (r < 0) return r; } else { - r = add_matches_for_user_unit(j, u, getuid()); + r = add_matches_for_user_unit(j, u); if (r < 0) return r; r = sd_journal_add_disjunction(j); @@ -227,7 +227,7 @@ static int add_units(sd_journal *j) { return r; SET_FOREACH(u, units) { - r = add_matches_for_user_unit(j, u, getuid()); + r = add_matches_for_user_unit(j, u); if (r < 0) return r; r = sd_journal_add_disjunction(j); diff --git a/src/login/loginctl.c b/src/login/loginctl.c index 8236dc2dc8..cf3bff437a 100644 --- a/src/login/loginctl.c +++ b/src/login/loginctl.c @@ -733,16 +733,15 @@ static int print_session_status_info(sd_bus *bus, const char *path) { show_journal_by_unit( stdout, i.scope, - NULL, + /* namespace = */ NULL, arg_output, - 0, + /* n_columns = */ 0, i.timestamp.monotonic, arg_lines, - 0, get_output_flags() | OUTPUT_BEGIN_NEWLINE, SD_JOURNAL_LOCAL_ONLY, - true, - NULL); + /* system_unit = */ true, + /* ellipsized = */ NULL); } return 0; @@ -839,16 +838,15 @@ static int print_user_status_info(sd_bus *bus, const char *path) { show_journal_by_unit( stdout, i.slice, - NULL, + /* namespace = */ NULL, arg_output, - 0, + /* n_columns = */ 0, i.timestamp.monotonic, arg_lines, - 0, get_output_flags() | OUTPUT_BEGIN_NEWLINE, SD_JOURNAL_LOCAL_ONLY, - true, - NULL); + /* system_unit = */ true, + /* ellipsized = */ NULL); } return 0; diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 42b7fef720..1b63e6d203 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -605,16 +605,15 @@ static void print_machine_status_info(sd_bus *bus, MachineStatusInfo *i) { show_journal_by_unit( stdout, i->unit, - NULL, + /* namespace = */ NULL, arg_output, - 0, + /* n_columns = */ 0, i->timestamp.monotonic, arg_lines, - 0, get_output_flags() | OUTPUT_BEGIN_NEWLINE, SD_JOURNAL_LOCAL_ONLY, - true, - NULL); + /* system_unit = */ true, + /* ellipsized = */ NULL); } } diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index d5b131a187..f7cededf6f 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -1620,7 +1620,8 @@ int add_matches_for_unit(sd_journal *j, const char *unit) { return r; } -int add_matches_for_user_unit(sd_journal *j, const char *unit, uid_t uid) { +int add_matches_for_user_unit(sd_journal *j, const char *unit) { + uid_t uid = getuid(); int r; assert(j); @@ -1708,7 +1709,6 @@ int show_journal_by_unit( unsigned n_columns, usec_t not_before, unsigned how_many, - uid_t uid, OutputFlags flags, int journal_open_flags, bool system_unit, @@ -1734,7 +1734,7 @@ int show_journal_by_unit( if (system_unit) r = add_matches_for_unit(j, unit); else - r = add_matches_for_user_unit(j, unit, uid); + r = add_matches_for_user_unit(j, unit); if (r < 0) return log_error_errno(r, "Failed to add unit matches: %m"); diff --git a/src/shared/logs-show.h b/src/shared/logs-show.h index 3a8ce8b7c1..da1bbe0420 100644 --- a/src/shared/logs-show.h +++ b/src/shared/logs-show.h @@ -49,8 +49,7 @@ int add_matches_for_unit( int add_matches_for_user_unit( sd_journal *j, - const char *unit, - uid_t uid); + const char *unit); int show_journal_by_unit( FILE *f, @@ -60,7 +59,6 @@ int show_journal_by_unit( unsigned n_columns, usec_t not_before, unsigned how_many, - uid_t uid, OutputFlags flags, int journal_open_flags, bool system_unit, diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index 74d6465739..2fdf321886 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -847,10 +847,9 @@ static void print_status_info( i->id, i->log_namespace, arg_output, - 0, + /* n_columns = */ 0, i->inactive_exit_timestamp_monotonic, arg_lines, - getuid(), get_output_flags() | OUTPUT_BEGIN_NEWLINE, SD_JOURNAL_LOCAL_ONLY, arg_runtime_scope == RUNTIME_SCOPE_SYSTEM, From 131701d10a14e558e0045c9aab16e984927d6813 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 27 Apr 2024 17:50:49 +0900 Subject: [PATCH 02/11] logs-show: add missing strempty() Follow-up for 8e976dc9209853d5d4d2db3016289f2a5ab99fd9. --- src/shared/logs-show.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index f7cededf6f..5bce1c34fb 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -1688,7 +1688,7 @@ int add_match_this_boot(sd_journal *j, const char *machine) { r = id128_get_boot_for_machine(machine, &boot_id); if (r < 0) return log_error_errno(r, "Failed to get boot ID%s%s: %m", - isempty(machine) ? "" : " of container ", machine); + isempty(machine) ? "" : " of container ", strempty(machine)); r = add_match_boot_id(j, boot_id); if (r < 0) From fdd325fbb8d7819b7fda875a5c9a3bea2e750981 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 27 Apr 2024 17:44:49 +0900 Subject: [PATCH 03/11] journalctl: several cleanups for parse_boot_descriptor() - rename to parse_id_descriptor(), to make it usable for other kind of ID later. - add missing assertions, - prefix arguments for storing results with 'ret_', - drop unnecessary 'else'. --- src/journal/journalctl.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index f634d33a97..868808e432 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -104,15 +104,21 @@ STATIC_DESTRUCTOR_REGISTER(arg_output_fields, set_freep); STATIC_DESTRUCTOR_REGISTER(arg_compiled_pattern, pattern_freep); STATIC_DESTRUCTOR_REGISTER(arg_image_policy, image_policy_freep); -static int parse_boot_descriptor(const char *x, sd_id128_t *boot_id, int *offset) { +static int parse_id_descriptor(const char *x, sd_id128_t *ret_id, int *ret_offset) { sd_id128_t id = SD_ID128_NULL; int off = 0, r; + assert(x); + assert(ret_id); + assert(ret_offset); + if (streq(x, "all")) { - *boot_id = SD_ID128_NULL; - *offset = 0; + *ret_id = SD_ID128_NULL; + *ret_offset = 0; return 0; - } else if (strlen(x) >= SD_ID128_STRING_MAX - 1) { + } + + if (strlen(x) >= SD_ID128_STRING_MAX - 1) { char *t; t = strndupa_safe(x, SD_ID128_STRING_MAX - 1); @@ -134,12 +140,8 @@ static int parse_boot_descriptor(const char *x, sd_id128_t *boot_id, int *offset return r; } - if (boot_id) - *boot_id = id; - - if (offset) - *offset = off; - + *ret_id = id; + *ret_offset = off; return 1; } @@ -517,19 +519,16 @@ static int parse_argv(int argc, char *argv[]) { arg_boot_offset = 0; if (optarg) { - r = parse_boot_descriptor(optarg, &arg_boot_id, &arg_boot_offset); + r = parse_id_descriptor(optarg, &arg_boot_id, &arg_boot_offset); if (r < 0) return log_error_errno(r, "Failed to parse boot descriptor '%s'", optarg); arg_boot = r; - /* Hmm, no argument? Maybe the next - * word on the command line is - * supposed to be the argument? Let's - * see if there is one and is parsable - * as a boot descriptor... */ } else if (optind < argc) { - r = parse_boot_descriptor(argv[optind], &arg_boot_id, &arg_boot_offset); + /* Hmm, no argument? Maybe the next word on the command line is supposed to be the + * argument? Let's see if there is one and is parsable as a boot descriptor... */ + r = parse_id_descriptor(argv[optind], &arg_boot_id, &arg_boot_offset); if (r >= 0) { arg_boot = r; optind++; From 781ddf147722d878ae0c8b78356ec8556cbc5502 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 Apr 2024 13:05:13 +0900 Subject: [PATCH 04/11] journalctl: split out journal_acquire_boot() from add_boot() No functional change, just refactoring and prepration for later changes. --- src/journal/journalctl-filter.c | 49 ++++++++++----------------------- src/journal/journalctl-util.c | 46 +++++++++++++++++++++++++++++++ src/journal/journalctl-util.h | 1 + 3 files changed, 61 insertions(+), 35 deletions(-) diff --git a/src/journal/journalctl-filter.c b/src/journal/journalctl-filter.c index 039fa5dc8b..f9eb9f8c58 100644 --- a/src/journal/journalctl-filter.c +++ b/src/journal/journalctl-filter.c @@ -9,6 +9,7 @@ #include "journal-internal.h" #include "journalctl.h" #include "journalctl-filter.h" +#include "journalctl-util.h" #include "logs-show.h" #include "missing_sched.h" #include "nulstr-util.h" @@ -23,42 +24,13 @@ static int add_boot(sd_journal *j) { if (!arg_boot) return 0; - /* Take a shortcut and use the current boot_id, which we can do very quickly. - * We can do this only when we logs are coming from the current machine, - * so take the slow path if log location is specified. */ - if (arg_boot_offset == 0 && sd_id128_is_null(arg_boot_id) && - !arg_directory && !arg_file && !arg_root) - return add_match_this_boot(j, arg_machine); - - if (sd_id128_is_null(arg_boot_id)) { - r = journal_find_boot_by_offset(j, arg_boot_offset, &arg_boot_id); - if (r < 0) - return log_error_errno(r, "Failed to find journal entry from the specified boot offset (%+i): %m", - arg_boot_offset); - if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(ENODATA), - "No journal boot entry found from the specified boot offset (%+i).", - arg_boot_offset); - } else { - r = journal_find_boot_by_id(j, arg_boot_id); - if (r < 0) - return log_error_errno(r, "Failed to find journal entry from the specified boot ID (%s): %m", - SD_ID128_TO_STRING(arg_boot_id)); - if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(ENODATA), - "No journal boot entry found from the specified boot ID (%s).", - SD_ID128_TO_STRING(arg_boot_id)); - } + assert(!sd_id128_is_null(arg_boot_id)); r = add_match_boot_id(j, arg_boot_id); if (r < 0) - return log_error_errno(r, "Failed to add match: %m"); + return r; - r = sd_journal_add_conjunction(j); - if (r < 0) - return log_error_errno(r, "Failed to add conjunction: %m"); - - return 0; + return sd_journal_add_conjunction(j); } static int add_dmesg(sd_journal *j) { @@ -457,12 +429,19 @@ int add_filters(sd_journal *j, char **matches) { assert(j); - /* add_boot() must be called first! - * It may need to seek the journal to find parent boot IDs. */ - r = add_boot(j); + /* First, search boot ID, as that may set and flush matches and seek journal. */ + r = journal_acquire_boot(j); if (r < 0) return r; + /* Clear unexpected matches for safety. */ + sd_journal_flush_matches(j); + + /* Then, add filters in the below. */ + r = add_boot(j); + if (r < 0) + return log_error_errno(r, "Failed to add filter for boot: %m"); + r = add_dmesg(j); if (r < 0) return log_error_errno(r, "Failed to add filter for dmesg: %m"); diff --git a/src/journal/journalctl-util.c b/src/journal/journalctl-util.c index 575488bee1..5c94e2c5ab 100644 --- a/src/journal/journalctl-util.c +++ b/src/journal/journalctl-util.c @@ -2,9 +2,11 @@ #include +#include "id128-util.h" #include "journal-util.h" #include "journalctl.h" #include "journalctl-util.h" +#include "logs-show.h" #include "rlimit-util.h" #include "sigbus.h" #include "terminal-util.h" @@ -70,3 +72,47 @@ bool journal_boot_has_effect(sd_journal *j) { return true; } + +int journal_acquire_boot(sd_journal *j) { + int r; + + assert(j); + + if (!arg_boot) { + /* Clear relevant field for safety. */ + arg_boot_id = SD_ID128_NULL; + arg_boot_offset = 0; + return 0; + } + + /* Take a shortcut and use the current boot_id, which we can do very quickly. + * We can do this only when the logs are coming from the current machine, + * so take the slow path if log location is specified. */ + if (arg_boot_offset == 0 && sd_id128_is_null(arg_boot_id) && + !arg_directory && !arg_file && !arg_root) { + r = id128_get_boot_for_machine(arg_machine, &arg_boot_id); + if (r < 0) + return log_error_errno(r, "Failed to get boot ID%s%s: %m", + isempty(arg_machine) ? "" : " of container ", strempty(arg_machine)); + } else if (sd_id128_is_null(arg_boot_id)) { + r = journal_find_boot_by_offset(j, arg_boot_offset, &arg_boot_id); + if (r < 0) + return log_error_errno(r, "Failed to find journal entry from the specified boot offset (%+i): %m", + arg_boot_offset); + if (r == 0) + return log_error_errno(SYNTHETIC_ERRNO(ENODATA), + "No journal boot entry found from the specified boot offset (%+i).", + arg_boot_offset); + } else { + r = journal_find_boot_by_id(j, arg_boot_id); + if (r < 0) + return log_error_errno(r, "Failed to find journal entry from the specified boot ID (%s): %m", + SD_ID128_TO_STRING(arg_boot_id)); + if (r == 0) + return log_error_errno(SYNTHETIC_ERRNO(ENODATA), + "No journal boot entry found from the specified boot ID (%s).", + SD_ID128_TO_STRING(arg_boot_id)); + } + + return 1; +} diff --git a/src/journal/journalctl-util.h b/src/journal/journalctl-util.h index 38f634f828..ea3e56838d 100644 --- a/src/journal/journalctl-util.h +++ b/src/journal/journalctl-util.h @@ -8,3 +8,4 @@ char* format_timestamp_maybe_utc(char *buf, size_t l, usec_t t); int acquire_journal(sd_journal **ret); bool journal_boot_has_effect(sd_journal *j); +int journal_acquire_boot(sd_journal *j); From ae0e6de918b7f8cb89ed1d69a4631d02eeab7d0c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 26 Apr 2024 09:40:26 +0900 Subject: [PATCH 05/11] journalctl: fix --boot=0 with --file=- (from stdin) Follow-up for 592855c3189549fed93b1060b72299910c6ab1d0. --- src/journal/journalctl-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/journal/journalctl-util.c b/src/journal/journalctl-util.c index 5c94e2c5ab..2e7e652fba 100644 --- a/src/journal/journalctl-util.c +++ b/src/journal/journalctl-util.c @@ -89,7 +89,7 @@ int journal_acquire_boot(sd_journal *j) { * We can do this only when the logs are coming from the current machine, * so take the slow path if log location is specified. */ if (arg_boot_offset == 0 && sd_id128_is_null(arg_boot_id) && - !arg_directory && !arg_file && !arg_root) { + !arg_directory && !arg_file && !arg_file_stdin && !arg_root) { r = id128_get_boot_for_machine(arg_machine, &arg_boot_id); if (r < 0) return log_error_errno(r, "Failed to get boot ID%s%s: %m", From 87dfaba7e9d20d2e94cebc0a3f9fcb807d5473f5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 26 Apr 2024 12:10:39 +0900 Subject: [PATCH 06/11] logs-show: flush matches before and after finding boots Otherwise, if several matches already set, then the first seek to head or tail may move the cursor to an invalid place, hence they provide wrong ID(s). Also, reading journal after calling these function may provide unexpected data. Currently, the caller does not install any matches before calling the functions, and does not read any journal entry after journal_get_boots() succeeds or journal_find_boot_by_offset() succeeds with 0. Hence, this should not change any behavior. Just for safety. --- src/shared/logs-show.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index 5bce1c34fb..be797afdf6 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -1790,6 +1790,7 @@ static int discover_next_boot( if (r < 0) return r; if (r == 0) { + sd_journal_flush_matches(j); *ret = (BootId) {}; return 0; /* End of journal, yay. */ } @@ -1947,6 +1948,8 @@ int journal_find_boot_by_offset(sd_journal *j, int offset, sd_id128_t *ret) { * (chronological) first boot in the journal. */ advance_older = offset <= 0; + sd_journal_flush_matches(j); + if (advance_older) r = sd_journal_seek_tail(j); /* seek to newest */ else @@ -1992,6 +1995,8 @@ int journal_get_boots(sd_journal *j, BootId **ret_boots, size_t *ret_n_boots) { assert(ret_boots); assert(ret_n_boots); + sd_journal_flush_matches(j); + r = sd_journal_seek_head(j); /* seek to oldest */ if (r < 0) return r; From a4675155190d7c17dc0fa6be9134837f32ad519a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 26 Apr 2024 11:27:12 +0900 Subject: [PATCH 07/11] =?UTF-8?q?journalctl:=20fix=20support=20of=20--boot?= =?UTF-8?q?=3DID=C2=B1offset=20format?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes a regression introduced by e44f06065bf20e8d0e4adacff61350ebd36f299e. After the offending commit, if a boot ID suffixed with an offset is specified to --boot=, the boot ID was ignored. This fixes the issue. To fix the issue, this merges journal_find_boot_by_id() and journal_find_boot_by_offset(). --- src/journal/journalctl-util.c | 30 +++--- .../sd-journal/test-journal-interleaving.c | 33 +++---- src/shared/logs-show.c | 96 +++++++++---------- src/shared/logs-show.h | 3 +- 4 files changed, 82 insertions(+), 80 deletions(-) diff --git a/src/journal/journalctl-util.c b/src/journal/journalctl-util.c index 2e7e652fba..32cdaf8381 100644 --- a/src/journal/journalctl-util.c +++ b/src/journal/journalctl-util.c @@ -94,24 +94,26 @@ int journal_acquire_boot(sd_journal *j) { if (r < 0) return log_error_errno(r, "Failed to get boot ID%s%s: %m", isempty(arg_machine) ? "" : " of container ", strempty(arg_machine)); - } else if (sd_id128_is_null(arg_boot_id)) { - r = journal_find_boot_by_offset(j, arg_boot_offset, &arg_boot_id); - if (r < 0) - return log_error_errno(r, "Failed to find journal entry from the specified boot offset (%+i): %m", - arg_boot_offset); - if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(ENODATA), - "No journal boot entry found from the specified boot offset (%+i).", - arg_boot_offset); } else { - r = journal_find_boot_by_id(j, arg_boot_id); + sd_id128_t boot_id; + + r = journal_find_boot(j, arg_boot_id, arg_boot_offset, &boot_id); if (r < 0) - return log_error_errno(r, "Failed to find journal entry from the specified boot ID (%s): %m", - SD_ID128_TO_STRING(arg_boot_id)); + return log_error_errno(r, "Failed to find journal entry from the specified boot (%s%+i): %m", + sd_id128_is_null(arg_boot_id) ? "" : SD_ID128_TO_STRING(arg_boot_id), + arg_boot_offset); if (r == 0) return log_error_errno(SYNTHETIC_ERRNO(ENODATA), - "No journal boot entry found from the specified boot ID (%s).", - SD_ID128_TO_STRING(arg_boot_id)); + "No journal boot entry found from the specified boot (%s%+i).", + sd_id128_is_null(arg_boot_id) ? "" : SD_ID128_TO_STRING(arg_boot_id), + arg_boot_offset); + + log_debug("Found boot %s for %s%+i", + SD_ID128_TO_STRING(boot_id), + sd_id128_is_null(arg_boot_id) ? "" : SD_ID128_TO_STRING(arg_boot_id), + arg_boot_offset); + + arg_boot_id = boot_id; } return 1; diff --git a/src/libsystemd/sd-journal/test-journal-interleaving.c b/src/libsystemd/sd-journal/test-journal-interleaving.c index ed1918d103..6f43891535 100644 --- a/src/libsystemd/sd-journal/test-journal-interleaving.c +++ b/src/libsystemd/sd-journal/test-journal-interleaving.c @@ -459,7 +459,7 @@ TEST(skip) { static void test_boot_id_one(void (*setup)(void), size_t n_boots_expected) { char t[] = "/var/tmp/journal-boot-id-XXXXXX"; - sd_journal *j; + _cleanup_(sd_journal_closep) sd_journal *j = NULL; _cleanup_free_ BootId *boots = NULL; size_t n_boots; @@ -471,24 +471,25 @@ static void test_boot_id_one(void (*setup)(void), size_t n_boots_expected) { assert_se(journal_get_boots(j, &boots, &n_boots) >= 0); assert_se(boots); assert_se(n_boots == n_boots_expected); - sd_journal_close(j); - FOREACH_ARRAY(b, boots, n_boots) { - assert_ret(sd_journal_open_directory(&j, t, SD_JOURNAL_ASSUME_IMMUTABLE)); - assert_se(journal_find_boot_by_id(j, b->id) == 1); - sd_journal_close(j); - } - - for (int i = - (int) n_boots + 1; i <= (int) n_boots; i++) { + for (size_t i = 0; i < n_boots; i++) { sd_id128_t id; - assert_ret(sd_journal_open_directory(&j, t, SD_JOURNAL_ASSUME_IMMUTABLE)); - assert_se(journal_find_boot_by_offset(j, i, &id) == 1); - if (i <= 0) - assert_se(sd_id128_equal(id, boots[n_boots + i - 1].id)); - else - assert_se(sd_id128_equal(id, boots[i - 1].id)); - sd_journal_close(j); + /* positive offset */ + assert_se(journal_find_boot(j, SD_ID128_NULL, (int) (i + 1), &id) == 1); + assert_se(sd_id128_equal(id, boots[i].id)); + + /* negative offset */ + assert_se(journal_find_boot(j, SD_ID128_NULL, (int) (i + 1) - (int) n_boots, &id) == 1); + assert_se(sd_id128_equal(id, boots[i].id)); + + for (size_t k = 0; k < n_boots; k++) { + int offset = (int) k - (int) i; + + /* relative offset */ + assert_se(journal_find_boot(j, boots[i].id, offset, &id) == 1); + assert_se(sd_id128_equal(id, boots[k].id)); + } } test_done(t); diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index be797afdf6..069fcde4bf 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -1909,37 +1909,9 @@ static int discover_next_boot( } } -int journal_find_boot_by_id(sd_journal *j, sd_id128_t boot_id) { - int r; - - assert(j); - assert(!sd_id128_is_null(boot_id)); - - sd_journal_flush_matches(j); - - r = add_match_boot_id(j, boot_id); - if (r < 0) - return r; - - r = sd_journal_seek_head(j); /* seek to oldest */ - if (r < 0) - return r; - - r = sd_journal_next(j); /* read the oldest entry */ - if (r < 0) - return r; - - /* At this point the read pointer is positioned at the oldest occurrence of the reference boot ID. - * After flushing the matches, one more invocation of _previous() will hence place us at the - * following entry, which must then have an older boot ID */ - - sd_journal_flush_matches(j); - return r > 0; -} - -int journal_find_boot_by_offset(sd_journal *j, int offset, sd_id128_t *ret) { +int journal_find_boot(sd_journal *j, sd_id128_t boot_id, int offset, sd_id128_t *ret) { bool advance_older; - int r; + int r, offset_start; assert(j); assert(ret); @@ -1950,21 +1922,50 @@ int journal_find_boot_by_offset(sd_journal *j, int offset, sd_id128_t *ret) { sd_journal_flush_matches(j); - if (advance_older) - r = sd_journal_seek_tail(j); /* seek to newest */ - else - r = sd_journal_seek_head(j); /* seek to oldest */ - if (r < 0) - return r; + if (!sd_id128_is_null(boot_id)) { + r = add_match_boot_id(j, boot_id); + if (r < 0) + return r; - /* No sd_journal_next()/_previous() here. - * - * At this point the read pointer is positioned after the newest/before the oldest entry in the whole - * journal. The next invocation of _previous()/_next() will hence position us at the newest/oldest - * entry we have. */ + if (advance_older) + r = sd_journal_seek_head(j); /* seek to oldest */ + else + r = sd_journal_seek_tail(j); /* seek to newest */ + if (r < 0) + return r; - sd_id128_t boot_id = SD_ID128_NULL; - for (int off = !advance_older; ; off += advance_older ? -1 : 1) { + r = sd_journal_step_one(j, advance_older); + if (r < 0) + return r; + if (r == 0) { + sd_journal_flush_matches(j); + *ret = SD_ID128_NULL; + return false; + } + if (offset == 0) { + /* If boot ID is specified without an offset, then let's short cut the loop below. */ + sd_journal_flush_matches(j); + *ret = boot_id; + return true; + } + + offset_start = advance_older ? -1 : 1; + } else { + if (advance_older) + r = sd_journal_seek_tail(j); /* seek to newest */ + else + r = sd_journal_seek_head(j); /* seek to oldest */ + if (r < 0) + return r; + + offset_start = advance_older ? 0 : 1; + } + + /* At this point the cursor is positioned at the newest/oldest entry of the reference boot ID if + * specified, or whole journal otherwise. The next invocation of _previous()/_next() will hence + * position us at the newest/oldest entry we have. */ + + for (int off = offset_start; ; off += advance_older ? -1 : 1) { BootId boot; r = discover_next_boot(j, boot_id, advance_older, &boot); @@ -1978,12 +1979,11 @@ int journal_find_boot_by_offset(sd_journal *j, int offset, sd_id128_t *ret) { boot_id = boot.id; log_debug("Found boot ID %s by offset %i", SD_ID128_TO_STRING(boot_id), off); - if (off == offset) - break; + if (off == offset) { + *ret = boot_id; + return true; + } } - - *ret = boot_id; - return true; } int journal_get_boots(sd_journal *j, BootId **ret_boots, size_t *ret_n_boots) { diff --git a/src/shared/logs-show.h b/src/shared/logs-show.h index da1bbe0420..b1b42bdae7 100644 --- a/src/shared/logs-show.h +++ b/src/shared/logs-show.h @@ -70,6 +70,5 @@ void json_escape( size_t l, OutputFlags flags); -int journal_find_boot_by_id(sd_journal *j, sd_id128_t boot_id); -int journal_find_boot_by_offset(sd_journal *j, int offset, sd_id128_t *ret); +int journal_find_boot(sd_journal *j, sd_id128_t boot_id, int offset, sd_id128_t *ret); int journal_get_boots(sd_journal *j, BootId **ret_boots, size_t *ret_n_boots); From afcd9c60fe7d532fa6954a363655400ae6ba2bc2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 26 Apr 2024 11:30:34 +0900 Subject: [PATCH 08/11] logs-show: fix stored timestamp when advance_older is true Currently, the parsed timestamp is only used when advance_older is false. Hence, this does not change any behavior. But, let's fix it anyway. --- src/shared/logs-show.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index 069fcde4bf..9851fd3646 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -1841,7 +1841,7 @@ static int discover_next_boot( goto try_again; } - r = sd_journal_get_realtime_usec(j, &boot.first_usec); + r = sd_journal_get_realtime_usec(j, advance_older ? &boot.last_usec : &boot.first_usec); if (r < 0) return r; @@ -1863,7 +1863,7 @@ static int discover_next_boot( goto try_again; } - r = sd_journal_get_realtime_usec(j, &boot.last_usec); + r = sd_journal_get_realtime_usec(j, advance_older ? &boot.first_usec : &boot.last_usec); if (r < 0) return r; From 8f2bcb1fb292c75b627d749c90283daf27c38eef Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 26 Apr 2024 11:31:48 +0900 Subject: [PATCH 09/11] logs-show: use GREEDY_REALLOC_APPEND() --- src/shared/logs-show.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index 9851fd3646..c96f501f0e 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -2024,10 +2024,8 @@ int journal_get_boots(sd_journal *j, BootId **ret_boots, size_t *ret_n_boots) { * Exiting as otherwise this problem would cause an infinite loop. */ goto finish; - if (!GREEDY_REALLOC(boots, n_boots + 1)) + if (!GREEDY_REALLOC_APPEND(boots, n_boots, &boot, 1)) return -ENOMEM; - - boots[n_boots++] = boot; } finish: From 5da5d848f9d5175998e8a1030d32a7edb3371956 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 25 Apr 2024 14:22:41 +0900 Subject: [PATCH 10/11] journalctl: fail and show error message when no boot ID found No boot ID in journal should be definitly spurious. Let's warn about that and exit with failure. --- src/journal/journalctl-misc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/journal/journalctl-misc.c b/src/journal/journalctl-misc.c index 47eb72575a..65785d4d35 100644 --- a/src/journal/journalctl-misc.c +++ b/src/journal/journalctl-misc.c @@ -115,7 +115,8 @@ int action_list_boots(void) { if (r < 0) return log_error_errno(r, "Failed to determine boots: %m"); if (r == 0) - return 0; + return log_full_errno(arg_quiet ? LOG_DEBUG : LOG_ERR, SYNTHETIC_ERRNO(ENODATA), + "No boot found."); table = table_new("idx", "boot id", "first entry", "last entry"); if (!table) From d0936a72660598118052636c1340f060cffc61c7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 26 Apr 2024 13:40:40 +0900 Subject: [PATCH 11/11] journalctl: make --list-boots support -n/--lines= option Also mention that -r/--reverse is supported by the command. --- man/journalctl.xml | 12 +++++-- src/journal/journalctl-misc.c | 28 ++++++++++++---- src/journal/journalctl.c | 2 +- .../sd-journal/test-journal-interleaving.c | 32 ++++++++++++++++++- src/shared/logs-show.c | 18 +++++++++-- src/shared/logs-show.h | 7 +++- 6 files changed, 84 insertions(+), 15 deletions(-) diff --git a/man/journalctl.xml b/man/journalctl.xml index 7d6820064e..caa10a056b 100644 --- a/man/journalctl.xml +++ b/man/journalctl.xml @@ -851,10 +851,16 @@ - Show a tabular list of boot numbers (relative to the current boot), their IDs, and - the timestamps of the first and last message pertaining to the boot. + + Show a tabular list of boot numbers (relative to the current boot), their IDs, and the + timestamps of the first and last message pertaining to the boot. When specified with + option, only the + first (when the number prefixed with +) or the last (without prefix) + N entries will be shown. When specified with + , the list will be shown in the reverse order. - + + diff --git a/src/journal/journalctl-misc.c b/src/journal/journalctl-misc.c index 65785d4d35..8ca6ea2143 100644 --- a/src/journal/journalctl-misc.c +++ b/src/journal/journalctl-misc.c @@ -111,7 +111,11 @@ int action_list_boots(void) { if (r < 0) return r; - r = journal_get_boots(j, &boots, &n_boots); + r = journal_get_boots( + j, + /* advance_older = */ arg_lines_needs_seek_end(), + /* max_ids = */ arg_lines >= 0 ? (size_t) arg_lines : SIZE_MAX, + &boots, &n_boots); if (r < 0) return log_error_errno(r, "Failed to determine boots: %m"); if (r == 0) @@ -132,13 +136,25 @@ int action_list_boots(void) { (void) table_set_sort(table, (size_t) 0); (void) table_set_reverse(table, 0, arg_reverse); - FOREACH_ARRAY(i, boots, n_boots) { + for (int i = 0; i < (int) n_boots; i++) { + int index; + + if (arg_lines_needs_seek_end()) + /* With --lines=N, we only know the negative index, and the older ID is located earlier. */ + index = -i; + else if (arg_lines >= 0) + /* With --lines=+N, we only know the positive index, and the newer ID is located earlier. */ + index = i + 1; + else + /* Otherwise, show negative index. Note, in this case, newer ID is located earlier. */ + index = i + 1 - (int) n_boots; + r = table_add_many(table, - TABLE_INT, (int)(i - boots) - (int) n_boots + 1, + TABLE_INT, index, TABLE_SET_ALIGN_PERCENT, 100, - TABLE_ID128, i->id, - TABLE_TIMESTAMP, i->first_usec, - TABLE_TIMESTAMP, i->last_usec); + TABLE_ID128, boots[i].id, + TABLE_TIMESTAMP, boots[i].first_usec, + TABLE_TIMESTAMP, boots[i].last_usec); if (r < 0) return table_log_add_error(r); } diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 868808e432..45173a6813 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -957,7 +957,7 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Please specify either --reverse or --follow, not both."); - if (arg_lines >= 0 && arg_lines_oldest && (arg_reverse || arg_follow)) + if (arg_action == ACTION_SHOW && arg_lines >= 0 && arg_lines_oldest && (arg_reverse || arg_follow)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--lines=+N is unsupported when --reverse or --follow is specified."); diff --git a/src/libsystemd/sd-journal/test-journal-interleaving.c b/src/libsystemd/sd-journal/test-journal-interleaving.c index 6f43891535..d98b3ce8cb 100644 --- a/src/libsystemd/sd-journal/test-journal-interleaving.c +++ b/src/libsystemd/sd-journal/test-journal-interleaving.c @@ -468,7 +468,10 @@ static void test_boot_id_one(void (*setup)(void), size_t n_boots_expected) { setup(); assert_ret(sd_journal_open_directory(&j, t, SD_JOURNAL_ASSUME_IMMUTABLE)); - assert_se(journal_get_boots(j, &boots, &n_boots) >= 0); + assert_se(journal_get_boots( + j, + /* advance_older = */ false, /* max_ids = */ SIZE_MAX, + &boots, &n_boots) >= 0); assert_se(boots); assert_se(n_boots == n_boots_expected); @@ -492,6 +495,33 @@ static void test_boot_id_one(void (*setup)(void), size_t n_boots_expected) { } } + for (size_t i = 0; i <= n_boots_expected + 1; i++) { + _cleanup_free_ BootId *boots_limited = NULL; + size_t n_boots_limited; + + assert_se(journal_get_boots( + j, + /* advance_older = */ false, /* max_ids = */ i, + &boots_limited, &n_boots_limited) >= 0); + assert_se(boots_limited || i == 0); + assert_se(n_boots_limited == MIN(i, n_boots_expected)); + assert_se(memcmp_safe(boots, boots_limited, n_boots_limited * sizeof(BootId)) == 0); + } + + for (size_t i = 0; i <= n_boots_expected + 1; i++) { + _cleanup_free_ BootId *boots_limited = NULL; + size_t n_boots_limited; + + assert_se(journal_get_boots( + j, + /* advance_older = */ true, /* max_ids = */ i, + &boots_limited, &n_boots_limited) >= 0); + assert_se(boots_limited || i == 0); + assert_se(n_boots_limited == MIN(i, n_boots_expected)); + for (size_t k = 0; k < n_boots_limited; k++) + assert_se(memcmp(&boots[n_boots - k - 1], &boots_limited[k], sizeof(BootId)) == 0); + } + test_done(t); } diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index c96f501f0e..c71c868889 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -1986,7 +1986,13 @@ int journal_find_boot(sd_journal *j, sd_id128_t boot_id, int offset, sd_id128_t } } -int journal_get_boots(sd_journal *j, BootId **ret_boots, size_t *ret_n_boots) { +int journal_get_boots( + sd_journal *j, + bool advance_older, + size_t max_ids, + BootId **ret_boots, + size_t *ret_n_boots) { + _cleanup_free_ BootId *boots = NULL; size_t n_boots = 0; int r; @@ -1997,7 +2003,10 @@ int journal_get_boots(sd_journal *j, BootId **ret_boots, size_t *ret_n_boots) { sd_journal_flush_matches(j); - r = sd_journal_seek_head(j); /* seek to oldest */ + if (advance_older) + r = sd_journal_seek_tail(j); /* seek to newest */ + else + r = sd_journal_seek_head(j); /* seek to oldest */ if (r < 0) return r; @@ -2010,7 +2019,10 @@ int journal_get_boots(sd_journal *j, BootId **ret_boots, size_t *ret_n_boots) { for (;;) { BootId boot; - r = discover_next_boot(j, previous_boot_id, /* advance_older = */ false, &boot); + if (n_boots >= max_ids) + break; + + r = discover_next_boot(j, previous_boot_id, advance_older, &boot); if (r < 0) return r; if (r == 0) diff --git a/src/shared/logs-show.h b/src/shared/logs-show.h index b1b42bdae7..7e7b2af901 100644 --- a/src/shared/logs-show.h +++ b/src/shared/logs-show.h @@ -71,4 +71,9 @@ void json_escape( OutputFlags flags); int journal_find_boot(sd_journal *j, sd_id128_t boot_id, int offset, sd_id128_t *ret); -int journal_get_boots(sd_journal *j, BootId **ret_boots, size_t *ret_n_boots); +int journal_get_boots( + sd_journal *j, + bool advance_older, + size_t max_ids, + BootId **ret_boots, + size_t *ret_n_boots);