From c4fbc6b6e4017199717b2b8dac3d790ffd8934cc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Apr 2016 16:23:29 +0200 Subject: journalctl: add some explanatory comments to get_boots() --- src/journal/journalctl.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) (limited to 'src/journal') diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index c9a2c3812d..d764ba12aa 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -1103,16 +1103,16 @@ static int get_boots( return r; if (advance_older) - r = sd_journal_seek_head(j); + r = sd_journal_seek_head(j); /* seek to oldest */ else - r = sd_journal_seek_tail(j); + r = sd_journal_seek_tail(j); /* seek to newest */ if (r < 0) return r; if (advance_older) - r = sd_journal_next(j); + r = sd_journal_next(j); /* read the oldest entry */ else - r = sd_journal_previous(j); + r = sd_journal_previous(j); /* read the most recently added entry */ if (r < 0) return r; else if (r == 0) @@ -1121,15 +1121,24 @@ static int get_boots( count = 1; goto finish; } + + /* At this point the read pointer is positioned at the oldest/newest occurence of the reference boot + * ID. After flushing the matches, one more invocation of _previous()/_next() will hence place us at + * the following entry, which must then have an older/newer boot ID */ } else { + if (advance_older) - r = sd_journal_seek_tail(j); + r = sd_journal_seek_tail(j); /* seek to newest */ else - r = sd_journal_seek_head(j); + r = sd_journal_seek_head(j); /* seek to oldest */ if (r < 0) return r; - /* No sd_journal_next/previous here. */ + /* 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. */ } for (;;) { -- cgit v1.2.3-54-g00ecf From d4723fb5016b6679b3ed254be3832d88804a9f87 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Apr 2016 16:24:05 +0200 Subject: journalctl: simplify get_boots() a bit, by getting rid of one BootId object Let's store the reference as simple sd_id128_t, since we don't actually need a BootId for it. --- src/journal/journalctl.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'src/journal') diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index d764ba12aa..fbb147d1f7 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -1072,7 +1072,7 @@ static int discover_next_boot( static int get_boots( sd_journal *j, BootId **boots, - BootId *query_ref_boot, + sd_id128_t *query_ref_boot, int ref_boot_offset) { bool skip_once; @@ -1085,19 +1085,19 @@ static int get_boots( /* Adjust for the asymmetry that offset 0 is * the last (and current) boot, while 1 is considered the * (chronological) first boot in the journal. */ - skip_once = query_ref_boot && sd_id128_is_null(query_ref_boot->id) && ref_boot_offset < 0; + skip_once = query_ref_boot && sd_id128_is_null(*query_ref_boot) && ref_boot_offset < 0; /* Advance to the earliest/latest occurrence of our reference * boot ID (taking our lookup direction into account), so that * discover_next_boot() can do its job. * If no reference is given, the journal head/tail will do, * they're "virtual" boots after all. */ - if (query_ref_boot && !sd_id128_is_null(query_ref_boot->id)) { + if (query_ref_boot && !sd_id128_is_null(*query_ref_boot)) { char match[9+32+1] = "_BOOT_ID="; sd_journal_flush_matches(j); - sd_id128_to_string(query_ref_boot->id, match + 9); + sd_id128_to_string(*query_ref_boot, match + 9); r = sd_journal_add_match(j, match, sizeof(match) - 1); if (r < 0) return r; @@ -1160,7 +1160,7 @@ static int get_boots( if (ref_boot_offset == 0) { count = 1; - query_ref_boot->id = current->id; + *query_ref_boot = current->id; break; } } else { @@ -1216,8 +1216,8 @@ static int list_boots(sd_journal *j) { static int add_boot(sd_journal *j) { char match[9+32+1] = "_BOOT_ID="; + sd_id128_t ref_boot_id; int r; - BootId ref_boot_id = {}; assert(j); @@ -1227,7 +1227,7 @@ static int add_boot(sd_journal *j) { if (arg_boot_offset == 0 && sd_id128_equal(arg_boot_id, SD_ID128_NULL)) return add_match_this_boot(j, arg_machine); - ref_boot_id.id = arg_boot_id; + ref_boot_id = arg_boot_id; r = get_boots(j, NULL, &ref_boot_id, arg_boot_offset); assert(r <= 1); if (r <= 0) { @@ -1243,7 +1243,7 @@ static int add_boot(sd_journal *j) { return r == 0 ? -ENODATA : r; } - sd_id128_to_string(ref_boot_id.id, match + 9); + sd_id128_to_string(ref_boot_id, match + 9); r = sd_journal_add_match(j, match, sizeof(match) - 1); if (r < 0) -- cgit v1.2.3-54-g00ecf From d1bf9dc9631de75c4e1fd062ac5351abebfd9592 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Apr 2016 16:37:09 +0200 Subject: journalctl: simplify discover_next_boot() a bit Drop the "read_realtime" parameter. Getting the realtime timestamp from an entry is cheap, as it is a normal header field, hence let's just get this unconditionally, and simplify our code a bit. --- src/journal/journalctl.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) (limited to 'src/journal') diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index fbb147d1f7..97310e287c 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -991,8 +991,7 @@ static void boot_id_free_all(BootId *l) { static int discover_next_boot( sd_journal *j, BootId **boot, - bool advance_older, - bool read_realtime) { + bool advance_older) { int r; char match[9+32+1] = "_BOOT_ID="; @@ -1029,11 +1028,9 @@ static int discover_next_boot( if (r < 0) return r; - if (read_realtime) { - r = sd_journal_get_realtime_usec(j, &next_boot->first); - if (r < 0) - return r; - } + r = sd_journal_get_realtime_usec(j, &next_boot->first); + if (r < 0) + return r; /* Now seek to the last occurrence of this boot ID. */ sd_id128_to_string(next_boot->id, match + 9); @@ -1057,11 +1054,9 @@ static int discover_next_boot( else if (r == 0) return -ENODATA; /* This shouldn't happen. We just came from this very boot ID. */ - if (read_realtime) { - r = sd_journal_get_realtime_usec(j, &next_boot->last); - if (r < 0) - return r; - } + r = sd_journal_get_realtime_usec(j, &next_boot->last); + if (r < 0) + return r; *boot = next_boot; next_boot = NULL; @@ -1144,7 +1139,7 @@ static int get_boots( for (;;) { _cleanup_free_ BootId *current = NULL; - r = discover_next_boot(j, ¤t, advance_older, !query_ref_boot); + r = discover_next_boot(j, ¤t, advance_older); if (r < 0) { boot_id_free_all(head); return r; -- cgit v1.2.3-54-g00ecf From 0808b92f0274ec76dd0c92d1f2a8332add9b2bfc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Apr 2016 18:06:47 +0200 Subject: journalctl: improve output of --header a bit Show the various timestamps in hexadecimal too. This is useful for matching the timestamps included in cursor strings (which are encoded in hex, too), with the references in the journal header. --- src/journal/journal-file.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'src/journal') diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index bed825cdc3..4dd1b39d02 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -2806,11 +2806,11 @@ void journal_file_print_header(JournalFile *f) { "Data Hash Table Size: %"PRIu64"\n" "Field Hash Table Size: %"PRIu64"\n" "Rotate Suggested: %s\n" - "Head Sequential Number: %"PRIu64"\n" - "Tail Sequential Number: %"PRIu64"\n" - "Head Realtime Timestamp: %s\n" - "Tail Realtime Timestamp: %s\n" - "Tail Monotonic Timestamp: %s\n" + "Head Sequential Number: %"PRIu64" (%"PRIx64")\n" + "Tail Sequential Number: %"PRIu64" (%"PRIx64")\n" + "Head Realtime Timestamp: %s (%"PRIx64")\n" + "Tail Realtime Timestamp: %s (%"PRIx64")\n" + "Tail Monotonic Timestamp: %s (%"PRIx64")\n" "Objects: %"PRIu64"\n" "Entry Objects: %"PRIu64"\n", f->path, @@ -2831,11 +2831,11 @@ void journal_file_print_header(JournalFile *f) { le64toh(f->header->data_hash_table_size) / sizeof(HashItem), le64toh(f->header->field_hash_table_size) / sizeof(HashItem), yes_no(journal_file_rotate_suggested(f, 0)), - le64toh(f->header->head_entry_seqnum), - le64toh(f->header->tail_entry_seqnum), - format_timestamp_safe(x, sizeof(x), le64toh(f->header->head_entry_realtime)), - format_timestamp_safe(y, sizeof(y), le64toh(f->header->tail_entry_realtime)), - format_timespan(z, sizeof(z), le64toh(f->header->tail_entry_monotonic), USEC_PER_MSEC), + le64toh(f->header->head_entry_seqnum), le64toh(f->header->head_entry_seqnum), + le64toh(f->header->tail_entry_seqnum), le64toh(f->header->tail_entry_seqnum), + format_timestamp_safe(x, sizeof(x), le64toh(f->header->head_entry_realtime)), le64toh(f->header->head_entry_realtime), + format_timestamp_safe(y, sizeof(y), le64toh(f->header->tail_entry_realtime)), le64toh(f->header->tail_entry_realtime), + format_timespan(z, sizeof(z), le64toh(f->header->tail_entry_monotonic), USEC_PER_MSEC), le64toh(f->header->tail_entry_monotonic), le64toh(f->header->n_objects), le64toh(f->header->n_entries)); -- cgit v1.2.3-54-g00ecf From dc00966228ff90c554fd034e588ea55eb605ec52 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Apr 2016 18:08:42 +0200 Subject: journalctl: don't trust the per-field entry tables when looking for boot IDs When appending to a journal file, journald will: a) first, append the actual entry to the end of the journal file b) second, add an offset reference to it to the global entry array stored at the beginning of the file c) third, add offset references to it to the per-field entry array stored at various places of the file The global entry array, maintained by b) is used when iterating through the journal without matches applied. The per-field entry array maintained by c) is used when iterating through the journal with a match for that specific field applied. In the wild, there are journal files where a) and b) were completed, but c) was not before the files were abandoned. This means, that in some cases log entries are at the end of these files that appear in the global entry array, but not in the per-field entry array of the _BOOT_ID= field. Now, the "journalctl --list-boots" command alternatingly uses the global entry array and the per-field entry array of the _BOOT_ID= field. It seeks to the last entry of a specific _BOOT_ID=field by having the right match installed, and then jumps to the next following entry with no match installed anymore, under the assumption this would bring it to the next boot ID. However, if the per-field entry wasn't written fully, it might actually turn out that the global entry array might know one more entry with the same _BOOT_ID, thus resulting in a indefinite loop around the same _BOOT_ID. This patch fixes that, by updating the boot search logic to always continue reading entries until the boot ID actually changed from the previous. Thus, the per-field entry array is used as quick jump index (i.e. as an optimization), but not trusted otherwise. Only the global entry array is trusted. This replaces PR #1904, which is actually very similar to this one. However, this one actually reads the boot ID directly from the entry header, and doesn't try to read it at all until the read pointer is actually really located on the first item to read. Fixes: #617 Replaces: #1904 --- src/journal/journalctl.c | 58 +++++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 20 deletions(-) (limited to 'src/journal') diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 97310e287c..4b2736c9eb 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -988,17 +988,18 @@ static void boot_id_free_all(BootId *l) { } } -static int discover_next_boot( - sd_journal *j, - BootId **boot, - bool advance_older) { +static int discover_next_boot(sd_journal *j, + sd_id128_t previous_boot_id, + bool advance_older, + BootId **ret) { - int r; - char match[9+32+1] = "_BOOT_ID="; _cleanup_free_ BootId *next_boot = NULL; + char match[9+32+1] = "_BOOT_ID="; + sd_id128_t boot_id; + int r; assert(j); - assert(boot); + assert(ret); /* We expect the journal to be on the last position of a boot * (in relation to the direction we are going), so that the next @@ -1011,22 +1012,35 @@ static int discover_next_boot( * we can actually advance to a *different* boot. */ sd_journal_flush_matches(j); - if (advance_older) - r = sd_journal_previous(j); - else - r = sd_journal_next(j); - if (r < 0) - return r; - else if (r == 0) - return 0; /* End of journal, yay. */ + do { + if (advance_older) + r = sd_journal_previous(j); + else + r = sd_journal_next(j); + if (r < 0) + return r; + else if (r == 0) + return 0; /* End of journal, yay. */ + + r = sd_journal_get_monotonic_usec(j, NULL, &boot_id); + if (r < 0) + return r; + + /* We iterate through this in a loop, until the boot ID differs from the previous one. Note that + * normally, this will only require a single iteration, as we seeked to the last entry of the previous + * boot entry already. However, it might happen that the per-journal-field entry arrays are less + * complete than the main entry array, and hence might reference an entry that's not actually the last + * one of the boot ID as last one. Let's hence use the per-field array is initial seek position to + * speed things up, but let's not trust that it is complete, and hence, manually advance as + * necessary. */ + + } while (sd_id128_equal(boot_id, previous_boot_id)); next_boot = new0(BootId, 1); if (!next_boot) return -ENOMEM; - r = sd_journal_get_monotonic_usec(j, NULL, &next_boot->id); - if (r < 0) - return r; + next_boot->id = boot_id; r = sd_journal_get_realtime_usec(j, &next_boot->first); if (r < 0) @@ -1058,7 +1072,7 @@ static int discover_next_boot( if (r < 0) return r; - *boot = next_boot; + *ret = next_boot; next_boot = NULL; return 0; @@ -1074,6 +1088,7 @@ static int get_boots( int r, count = 0; BootId *head = NULL, *tail = NULL; const bool advance_older = query_ref_boot && ref_boot_offset <= 0; + sd_id128_t previous_boot_id; assert(j); @@ -1136,10 +1151,11 @@ static int get_boots( * entry we have. */ } + previous_boot_id = SD_ID128_NULL; for (;;) { _cleanup_free_ BootId *current = NULL; - r = discover_next_boot(j, ¤t, advance_older); + r = discover_next_boot(j, previous_boot_id, advance_older, ¤t); if (r < 0) { boot_id_free_all(head); return r; @@ -1148,6 +1164,8 @@ static int get_boots( if (!current) break; + previous_boot_id = current->id; + if (query_ref_boot) { if (!skip_once) ref_boot_offset += advance_older ? 1 : -1; -- cgit v1.2.3-54-g00ecf