From: Frantisek Sumsal Date: Tue, 5 Dec 2023 16:38:25 +0000 (+0100) Subject: journalctl: don't skip over messages not matching the cursor X-Git-Tag: v256-rc1~1560 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4207a5577a3711fb6760f1f122314fe3f4448709;p=thirdparty%2Fsystemd.git journalctl: don't skip over messages not matching the cursor When --after-cursor=/--cursor-file= is used together with a journal filter, we still skipped over the first matching entry even if it wasn't the entry the cursor points at, thus missing one "valid" entry completely. Let's fix this by checking if the entry cursor after seeking matches the user provided cursor, and skip to the next entry only when the cursors match. Resolves: #30288 --- diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index e79acc65bcb..de1534befd2 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -2164,10 +2164,12 @@ static int setup_event(Context *c, int fd, sd_event **ret) { } static int run(int argc, char *argv[]) { - bool need_seek = false, since_seeked = false, use_cursor = false, after_cursor = false; + bool need_seek = false, since_seeked = false, after_cursor = false; _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL; _cleanup_(umount_and_freep) char *mounted_dir = NULL; _cleanup_(sd_journal_closep) sd_journal *j = NULL; + _cleanup_free_ char *cursor_from_file = NULL; + const char *cursor = NULL; int n_shown, r, poll_fd = -EBADF; setlocale(LC_ALL, ""); @@ -2444,8 +2446,7 @@ static int run(int argc, char *argv[]) { } if (arg_cursor || arg_after_cursor || arg_cursor_file) { - _cleanup_free_ char *cursor_from_file = NULL; - const char *cursor = arg_cursor ?: arg_after_cursor; + cursor = arg_cursor ?: arg_after_cursor; if (arg_cursor_file) { r = read_one_line_file(arg_cursor_file, &cursor_from_file); @@ -2458,30 +2459,40 @@ static int run(int argc, char *argv[]) { } } else after_cursor = arg_after_cursor; + } - if (cursor) { - r = sd_journal_seek_cursor(j, cursor); - if (r < 0) - return log_error_errno(r, "Failed to seek to cursor: %m"); + if (cursor) { + r = sd_journal_seek_cursor(j, cursor); + if (r < 0) + return log_error_errno(r, "Failed to seek to cursor: %m"); - use_cursor = true; + r = sd_journal_step_one(j, !arg_reverse); + if (r < 0) + return log_error_errno(r, "Failed to iterate through journal: %m"); + + if (after_cursor && r > 0) { + /* With --after-cursor=/--cursor-file= we want to skip the first entry only if it's + * the entry the cursor is pointing at, otherwise, if some journal filters are used, + * we might skip the first entry of the filter match, which leads to unexpectedly + * missing journal entries. */ + int k; + + k = sd_journal_test_cursor(j, cursor); + if (k < 0) + return log_error_errno(k, "Failed to test cursor against current entry: %m"); + if (k > 0) + /* Current entry matches the one our cursor is pointing at, so let's try + * to advance the next entry. */ + r = sd_journal_step_one(j, !arg_reverse); } - } - - if (use_cursor) { - if (!arg_reverse) - r = sd_journal_next_skip(j, 1 + after_cursor); - else - r = sd_journal_previous_skip(j, 1 + after_cursor); - if (after_cursor && r < 2) { + if (r == 0) { /* We couldn't find the next entry after the cursor. */ if (arg_follow) need_seek = true; else arg_lines = 0; } - } else if (arg_until_set && (arg_reverse || arg_lines_needs_seek_end())) { /* If both --until and any of --reverse and --lines=N is specified, things get * a little tricky. We seek to the place of --until first. If only --reverse or diff --git a/test/units/testsuite-04.journal.sh b/test/units/testsuite-04.journal.sh index 3efac616e08..4d9e48717ac 100755 --- a/test/units/testsuite-04.journal.sh +++ b/test/units/testsuite-04.journal.sh @@ -241,3 +241,27 @@ diff -u /tmp/lb1 - <<'EOF' [{"index":-3,"boot_id":"5ea5fc4f82a14186b5332a788ef9435e","first_entry":1666569600994371,"last_entry":1666584266223608},{"index":-2,"boot_id":"bea6864f21ad4c9594c04a99d89948b0","first_entry":1666569601005945,"last_entry":1666584347230411},{"index":-1,"boot_id":"4c708e1fd0744336be16f3931aa861fb","first_entry":1666569601017222,"last_entry":1666584354649355},{"index":0,"boot_id":"35e8501129134edd9df5267c49f744a4","first_entry":1666569601009823,"last_entry":1666584438086856}] EOF rm -rf "$JOURNAL_DIR" /tmp/lb1 + +# Check that using --after-cursor/--cursor-file= together with journal filters doesn't +# skip over entries matched by the filter +# See: https://github.com/systemd/systemd/issues/30288 +UNIT_NAME="test-cursor-$RANDOM.service" +CURSOR_FILE="$(mktemp)" +# Generate some messages we can match against +journalctl --cursor-file="$CURSOR_FILE" -n1 +systemd-run --unit="$UNIT_NAME" --wait --service-type=exec bash -xec "echo hello; echo world" +journalctl --sync +# --after-cursor= + --unit= +# The format of the "Starting ..." message depends on StatusUnitFormat=, so match only the beginning +# which should be enough in this case +[[ "$(journalctl -n 1 -p info -o cat --unit="$UNIT_NAME" --after-cursor="$(<"$CURSOR_FILE")" _PID=1 )" =~ ^Starting\ ]] +# There should be no such messages before the cursor +[[ -z "$(journalctl -n 1 -p info -o cat --unit="$UNIT_NAME" --after-cursor="$(<"$CURSOR_FILE")" --reverse)" ]] +# --cursor-file= + a journal filter +diff <(journalctl --cursor-file="$CURSOR_FILE" -p info -o cat _SYSTEMD_UNIT="$UNIT_NAME") - <