all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Daniel Kral" <d.kral@proxmox.com>
To: "Maximiliano Sandoval" <m.sandoval@proxmox.com>,
	<pve-devel@lists.proxmox.com>
Subject: Re: [PATCH proxmox-mini-journalreader v4 1/2] add -B parameter to lists boots
Date: Mon, 17 Aug 2026 11:43:00 +0200	[thread overview]
Message-ID: <DKR4DI5M0LK2.2ZG2GMPOB22MO@proxmox.com> (raw)
In-Reply-To: <20260807135757.459145-2-m.sandoval@proxmox.com>

Looks good to me and works as expected with the added -n, -b, -e, -j and
-J functionality for me, thanks for working on this!

-j and -J aren't separate functionalities for -B, but it seems fine to
me as there were different semantics to handle for the full journal
output and it's different here.

Some small comments left inline, with those addressed consider this as

Reviewed-by: Daniel Kral <d.kral@proxmox.com>
Tested-by: Daniel Kral <d.kral@proxmox.com>


On Fri Aug 7, 2026 at 3:57 PM CEST, Maximiliano Sandoval wrote:
> We usually require the output from `last` or similar in support to query the
> previously booted kernel versions. We add this helper to avoid adding a
> dependency on either last or wtmpdb on the ISO images.
>
> The program, when executed with -B, will print all boots unless it is restricted
> with -n or -b, -e.
>
> glib is added as a dependency for simplicity of memory management.
>
> As per SD_JOURNAL_QUERY_UNIQUE(3), sd_journal_query_unique ignore matches hence
> why we need to iterate twice:
>
>      Note that these functions currently are not influenced by matches
>      set with sd_journal_add_match() but this might change in a later
>      version of this software.
>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>
> Notes:
>     Sample output
>     
>     $ proxmox-mini-journalreader -B -n 3 -j |jq
>     {
>       "data": [
>         {
>           "first_timestamp": "1785911171",
>           "last_timestamp": "1785942175",
>           "boot_id": "912937846ddd40e2b1acb57fb18c9cc2",
>           "kernel_version": "7.0.14-8-pve"
>         },
>         {
>           "first_timestamp": "1785998528",
>           "last_timestamp": "1786029768",
>           "boot_id": "09b4f7cd09f14086af52fff5e31931bd",
>           "kernel_version": "7.0.14-8-pve"
>         },
>         {
>           "first_timestamp": "1786084575",
>           "last_timestamp": "1786109309",
>           "boot_id": "f4fade4c1ce648be98886e8aabdf28db",
>           "kernel_version": "7.0.14-9-pve"
>         }
>       ],
>       "success": 1
>     }
>     
>     $ proxmox-mini-journalreader -B -n 3
>     2026-08-05 08:26	2026-08-05 17:02	912937846ddd40e2b1acb57fb18c9cc2	7.0.14-8-pve
>     2026-08-06 08:42	2026-08-06 17:22	09b4f7cd09f14086af52fff5e31931bd	7.0.14-8-pve
>     2026-08-07 08:36	2026-08-07 14:50	f4fade4c1ce648be98886e8aabdf28db	7.0.14-9-pve
>     
>     This seems quick enough for ~400 boots:
>     
>     time ./proxmox-mini-journalreader -B | wc -l
>     419
>     
>     real	0m0,133s
>     user	0m0,004s
>     sys	0m0,003s
>     
>     It might be even quicker than systemd itself:
>     
>     $ time journalctl --list-boots | wc -l
>     420
>     
>     real	0m0,216s
>     user	0m0,003s
>     sys	0m0,004s
>     
>     although it does not compare with `last` since they use their own database:
>     
>     $ time last
>     [...]
>     real	0m0,005s
>     user	0m0,000s
>     sys	0m0,003s
>     
>     Open questions:
>     
>     - We assume the first line in the journal is the "Linux version" line. This is
>     true as of now in practice. One could add a match like the following:
>     
>          sd_journal_add_match(j, "_TRANSPORT=kernel", strlen("_TRANSPORT=kernel"));
>     
>       if this is undesirable.
>     
>     Differences from v3:
>      - Close journal after listing boots
>      - Style fixes
>      - Support -j, we print timestamps in seconds since epoch
>      - Support -n, -b and -e
>      - Rename BootInfo->version to BootInfo->kernel_version
>     
>     Differences from v2:
>      - Mark regex_extract_match and format_timestamp functions as static char *
>      - Move the regex out of the for loop so it is only compiled once
>      - Add G_GNUC_UNUSED hint so that user_data does not produce an unused warning. We need
>        that parameter as part of the expected signature
>      - Remove duplicated check for regex != NULL
>     
>     Differences from v1:
>      - Replace uint with size_t
>      - Remove the use of a second journal
>      - Copy only 32 chars instead of 33. We were copying \0 which is added by
>        g_strndup anyways and 32 is the actual length of a BOOT_ID
>      - use strlen("BOOT_ID=") instead of sizeof("BOOT_ID") so it does not work by accident
>      - Use proper signature for GCompareFunc
>      - Add a couple of guards for empty messages
>
>  debian/control           |   2 +-
>  src/Makefile             |   2 +-
>  src/mini-journalreader.c | 190 ++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 191 insertions(+), 3 deletions(-)
>
> diff --git a/debian/control b/debian/control
> index 8280f6d..d7d8cc0 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -2,7 +2,7 @@ Source: proxmox-mini-journalreader
>  Section: admin
>  Priority: optional
>  Maintainer: Proxmox Support Team <support@proxmox.com>
> -Build-Depends: debhelper-compat (= 13), libsystemd-dev, pkg-config, scdoc,
> +Build-Depends: debhelper-compat (= 13), libsystemd-dev, pkg-config, scdoc, libglib2.0-dev,
>  Standards-Version: 4.6.2
>  
>  Package: proxmox-mini-journalreader
> diff --git a/src/Makefile b/src/Makefile
> index e64e066..c89e74a 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -5,7 +5,7 @@ LIBEXEC_DIR ?= $(DESTDIR)/usr/libexec/
>  MAN1_DIR ?= $(DESTDIR)/usr/share/man/man1
>  MANPAGE ?= $(PROGRAM).1
>  
> -LIBS := libsystemd
> +LIBS := libsystemd glib-2.0
>  CFLAGS += -Wall -Wextra -Wl,-z,relro -g -O2 --std=gnu11
>  CFLAGS += -fstack-protector-strong -D_FORTIFY_SOURCE=2
>  CFLAGS += $(shell pkg-config --cflags $(LIBS))
> diff --git a/src/mini-journalreader.c b/src/mini-journalreader.c
> index fc78f65..ff1a413 100644
> --- a/src/mini-journalreader.c
> +++ b/src/mini-journalreader.c
> @@ -21,6 +21,7 @@
>  
>  #include <errno.h>
>  #include <fnmatch.h>
> +#include <glib.h>
>  #include <stdbool.h>
>  #include <stdint.h>
>  #include <stdio.h>
> @@ -37,6 +38,182 @@ bool json = false;
>  bool structured = false;
>  bool first_line = true;
>  
> +typedef struct {
> +    char *boot_id;
> +    char *kernel_version;
> +    uint64_t first_timestamp;
> +    uint64_t last_timestamp;
> +} BootInfo;
> +
> +static void boot_info_free(BootInfo *info) {
> +    g_free(info->boot_id);
> +    g_free(info->kernel_version);
> +    g_free(info);
> +}
> +
> +static int boot_info_cmp(gconstpointer a, gconstpointer b) {
> +    const BootInfo *entry1 = a;
> +    const BootInfo *entry2 = b;
> +
> +    if (entry1->first_timestamp < entry2->first_timestamp) {
> +        return -1;
> +    }
> +    if (entry1->first_timestamp > entry2->first_timestamp) {
> +        return 1;
> +    }
> +    return 0;
> +}
> +
> +static char *regex_extract_match(GRegex *regex, const char *message) {
> +    g_autoptr(GMatchInfo) match_info = NULL;
> +    int start_pos = 0, end_pos = 0;
> +
> +    g_return_val_if_fail(message != NULL, g_strdup("unknown"));
> +
> +    g_regex_match(regex, message, G_REGEX_MATCH_DEFAULT, &match_info);
> +
> +    if (!g_match_info_matches(match_info)) {
> +        return g_strdup("unknown");
> +    }
> +
> +    g_match_info_fetch_pos(match_info, 1, &start_pos, &end_pos);
> +
> +    return g_strndup(message + start_pos, end_pos - start_pos);
> +}
> +
> +static char *format_timestamp(uint64_t timestamp) {
> +    g_autoptr(GDateTime) time = NULL;
> +
> +    if (timestamp == 0) {
> +        return g_strdup("-");
> +    }
> +
> +    time = g_date_time_new_from_unix_local_usec(timestamp);
> +    return g_date_time_format(time, "%Y-%m-%d %H:%M");
> +}
> +
> +static void print_boot(BootInfo *info) {
> +    g_return_if_fail(info != NULL);
> +
> +    if (json) {
> +        uint64_t start = info->first_timestamp / (1000 * 1000); // usec to sec
> +        uint64_t end = info->last_timestamp / (1000 * 1000);
> +        g_print("{ \"first_timestamp\": \"%ld\", \"last_timestamp\": \"%ld\", \"boot_id\": \"%s\", \"kernel_version\": \"%s\"}",
> +                start, end, info->boot_id, info->kernel_version);

nit: first_timestamp and last_timestamp could be JSON numbers

> +    } else {
> +        g_autofree char *start = format_timestamp(info->first_timestamp);
> +        g_autofree char *end = format_timestamp(info->last_timestamp);
> +
> +        g_print("%s\t%s\t%s\t%s\n", start, end, info->boot_id, info->kernel_version);
> +    }
> +}
> +
> +static int list_boots_and_kernels(sd_journal *j, bool have_number, uint64_t number, uint64_t begin, uint64_t end) {
> +    const void *data;
> +    const size_t prefix_len = strlen("_BOOT_ID=");
> +    size_t len;
> +    int r;
> +    g_autoptr(GPtrArray) boot_ids = NULL;
> +    g_autoptr(GPtrArray) boot_array = NULL;
> +    g_autoptr(GRegex) regex = NULL;
> +    g_autoptr(GError) error = NULL;
> +    end = (end == 0) ? G_MAXUINT64 : end;
> +
> +    boot_ids = g_ptr_array_new_with_free_func(g_free);
> +
> +    r = sd_journal_query_unique(j, "_BOOT_ID");
> +    if (r < 0) {
> +        g_printerr("Failed to query _BOOT_ID identifier: %s\n", strerror(-r));
> +        return 1;
> +    }

This can use print_error_and_exit() so it also handles the JSON error
output correctly

> +    SD_JOURNAL_FOREACH_UNIQUE(j, data, len) {
> +        g_ptr_array_add(boot_ids, g_strndup((const char *)data, len));
> +    }
> +
> +    boot_array = g_ptr_array_new_full(boot_ids->len, (GDestroyNotify)boot_info_free);
> +
> +    sd_journal_flush_matches(j);
> +
> +    regex = g_regex_new("Linux version ([a-zA-Z0-9.-]+)", G_REGEX_OPTIMIZE, G_REGEX_MATCH_DEFAULT, &error);
> +    if (error) {
> +        g_critical("regex compilation failed: %s", error->message);
> +        return 1;
> +    }

Same here, even though it's g_critical() here

> +
> +    for (size_t i = 0; i < boot_ids->len; i++) {
> +        uint64_t first_ts = 0, last_ts = 0;
> +        size_t msg_len;
> +        const char *message = NULL;
> +        const char *match = g_ptr_array_index(boot_ids, i);
> +        g_autofree char *version = NULL;
> +        g_autofree char *boot_id = NULL;
> +
> +        // Strip _BOOT_ID= from the message
> +        boot_id = g_strndup(match + prefix_len, 32);
> +
> +        sd_journal_add_match(j, match, strlen(match));
> +
> +        r = sd_journal_seek_head(j);
> +        if (r >= 0) {
> +            r = sd_journal_next(j);
> +            if (r > 0) {
> +                sd_journal_get_realtime_usec(j, &first_ts);
> +            }
> +        }
> +
> +        if (first_ts < begin || end < first_ts) {
> +            sd_journal_flush_matches(j);
> +            continue;
> +        }
> +
> +        r = sd_journal_get_data(j, "MESSAGE", (const void **)&message, &msg_len);
> +        if (r >= 0 && message != NULL && msg_len > 0) {
> +            version = regex_extract_match(regex, message);
> +        } else {
> +            version = g_strdup("unknown");
> +        }
> +
> +        r = sd_journal_seek_tail(j);
> +        if (r >= 0) {
> +            r = sd_journal_previous(j);
> +            if (r > 0) {
> +                sd_journal_get_realtime_usec(j, &last_ts);
> +            }
> +        }
> +
> +        sd_journal_flush_matches(j);
> +
> +        BootInfo *boot_info = g_new0(BootInfo, 1);
> +        boot_info->boot_id = g_steal_pointer(&boot_id);
> +        boot_info->kernel_version = g_steal_pointer(&version);
> +        boot_info->first_timestamp = first_ts;
> +        boot_info->last_timestamp = last_ts;
> +
> +        g_ptr_array_add(boot_array, boot_info);
> +    }
> +
> +    g_ptr_array_sort_values(boot_array, (GCompareFunc)boot_info_cmp);
> +
> +    if (json) {
> +        g_print("{\"data\":[");
> +    }
> +
> +    uint64_t starting_idx = (have_number && boot_array->len > number) ? boot_array->len - number : 0;
> +    for (uint64_t i = starting_idx; i < boot_array->len; i++) {
> +        BootInfo *boot_info = g_ptr_array_index(boot_array, i);
> +        print_boot(boot_info);
> +        if (json && i + 1 < boot_array->len) {
> +            g_print(",");
> +        }
> +    }
> +
> +    if (json) {
> +        g_print("],\"success\":1}");
> +    }
> +
> +    return 0;
> +}
> +
>  // helper to print errors on stderr
>  // if we're in json mode, print closing json body if possible
>  static void print_error_and_exit(const char *fmt, ...) {
> @@ -471,6 +648,7 @@ _Noreturn static void usage(char *error) {
>          "  -J\t\t\tprint as json with one object of separate fields per entry\n"
>          "  -I\t\t\twith -J, also emit a record listing the distinct syslog identifiers\n"
>          "  -U\t\t\twith -J, also emit a record listing the distinct systemd units\n"
> +        "  -B\t\t\tlist boots. The columns are: the dates of the first and last entry in the boot, the boot id, and the kernel version\n"
>          "  -h\t\t\tthis help\n"
>          "\n"
>          "Passing no range option will dump all the available journal\n"
> @@ -711,11 +889,12 @@ int main(int argc, char *argv[]) {
>      bool kernel = false;
>      bool list_identifiers = false;
>      bool list_units = false;
> +    bool list_boots = false;
>      int c;
>  
>      progname = argv[0];
>  
> -    while ((c = getopt(argc, argv, "b:e:d:n:f:t:p:i:u:jJIUkh")) != -1) {
> +    while ((c = getopt(argc, argv, "b:e:d:n:f:t:p:i:u:jJIUkhB")) != -1) {
>          switch (c) {
>          case 'b':
>              begin = arg_to_timestamp_usec(optarg);
> @@ -762,6 +941,9 @@ int main(int argc, char *argv[]) {
>          case 'U':
>              list_units = true;
>              break;
> +        case 'B':
> +            list_boots = true;
> +            break;
>          case 'h':
>              usage(NULL);
>          case '?':
> @@ -816,6 +998,12 @@ int main(int argc, char *argv[]) {
>          return 1;
>      }
>  
> +    if (list_boots) {
> +        int ret = list_boots_and_kernels(j, have_number, number, begin, end);
> +        sd_journal_close(j);
> +        return ret;
> +    }
> +
>      // restrict the traversal before seeking. Each filter is a separate group, conjoined (AND) with
>      // the others; within a group matches are OR'd (same field, or an explicit disjunction for the
>      // unit), so the groups must be split by a conjunction






  reply	other threads:[~2026-08-17  9:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-07 13:57 [PATCH manager/proxmox-mini-journalreader v4 0/2] report: list recent reboot events and their kernel version Maximiliano Sandoval
2026-08-07 13:57 ` [PATCH proxmox-mini-journalreader v4 1/2] add -B parameter to lists boots Maximiliano Sandoval
2026-08-17  9:43   ` Daniel Kral [this message]
2026-08-07 13:57 ` [PATCH manager v4 2/2] report: list recent reboot events and their kernel version Maximiliano Sandoval
2026-08-20 13:35 ` superseded: [PATCH manager/proxmox-mini-journalreader v4 0/2] " Maximiliano Sandoval

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DKR4DI5M0LK2.2ZG2GMPOB22MO@proxmox.com \
    --to=d.kral@proxmox.com \
    --cc=m.sandoval@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal