From 20ec8f534f90c94669ac8f9a50869f22f94fd4c8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 15 Feb 2022 14:24:53 +0100 Subject: [PATCH] sd-boot: make use of new "sort-key" boot loader spec field --- src/boot/bootctl.c | 2 + src/boot/efi/boot.c | 77 +++++++++++++----- src/fundamental/bootspec-fundamental.c | 20 +++-- src/fundamental/bootspec-fundamental.h | 5 +- src/shared/bootspec.c | 105 +++++++++++++++++-------- src/shared/bootspec.h | 1 + 6 files changed, 150 insertions(+), 60 deletions(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index e2900291bf..557b66b1e5 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -590,6 +590,8 @@ static int boot_entry_show( printf(" source: %s\n", link ?: e->path); } + if (e->sort_key) + printf(" sort-key: %s\n", e->sort_key); if (e->version) printf(" version: %s\n", e->version); if (e->machine_id) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index c2b799f007..1f254198ee 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -53,6 +53,7 @@ typedef struct { CHAR16 *id; /* The unique identifier for this entry (typically the filename of the file defining the entry) */ CHAR16 *title_show; /* The string to actually display (this is made unique before showing) */ CHAR16 *title; /* The raw (human readable) title string of the entry (not necessarily unique) */ + CHAR16 *sort_key; /* The string to use as primary sory key, usually ID= from os-release, possibly suffixed */ CHAR16 *version; /* The raw (human readable) version string of the entry */ CHAR16 *machine_id; EFI_HANDLE *device; @@ -540,6 +541,7 @@ static void print_status(Config *config, CHAR16 *loaded_image_path) { ps_string(L" id: %s\n", entry->id); ps_string(L" title: %s\n", entry->title); ps_string(L" title show: %s\n", streq_ptr(entry->title, entry->title_show) ? NULL : entry->title_show); + ps_string(L" sort key: %s\n", entry->sort_key); ps_string(L" version: %s\n", entry->version); ps_string(L" machine-id: %s\n", entry->machine_id); if (entry->device) @@ -1026,6 +1028,7 @@ static void config_entry_free(ConfigEntry *entry) { FreePool(entry->id); FreePool(entry->title_show); FreePool(entry->title); + FreePool(entry->sort_key); FreePool(entry->version); FreePool(entry->machine_id); FreePool(entry->loader); @@ -1427,6 +1430,12 @@ static void config_entry_add_from_file( continue; } + if (strcmpa((CHAR8 *)"sort-key", key) == 0) { + FreePool(entry->sort_key); + entry->sort_key = xstra_to_str(value); + continue; + } + if (strcmpa((CHAR8 *)"version", key) == 0) { FreePool(entry->version); entry->version = xstra_to_str(value); @@ -1643,12 +1652,39 @@ static INTN config_entry_compare(const ConfigEntry *a, const ConfigEntry *b) { assert(a); assert(b); - /* Order entries that have no tries left to the beginning of the list */ - if (a->tries_left != 0 && b->tries_left == 0) - return 1; + /* Order entries that have no tries left to the end of the list */ if (a->tries_left == 0 && b->tries_left != 0) + return 1; + if (a->tries_left != 0 && b->tries_left == 0) return -1; + /* If there's a sort key defined for *both* entries, then we do new-style ordering, i.e. by + * sort-key/machine-id/version, with a final fallback to id. If there's no sort key for either, we do + * old-style ordering, i.e. by id only. If one has sort key and the other does not, we put new-style + * before old-style. */ + r = CMP(!a->sort_key, !b->sort_key); + if (r != 0) /* one is old-style, one new-style */ + return r; + if (a->sort_key && b->sort_key) { + + r = strcmp(a->sort_key, b->sort_key); + if (r != 0) + return r; + + /* If multiple installations of the same OS are around, group by machine ID */ + r = strcmp_ptr(a->machine_id, b->machine_id); + if (r != 0) + return r; + + /* If the sort key was defined, then order by version now (downwards, putting the newest first) */ + r = -strverscmp_improved(a->version, b->version); + if (r != 0) + return r; + } + + /* Now order by ID (the version is likely part of the ID, thus note that this might put the oldest + * version last, not first, i.e. specifying a sort key explicitly is thus generally preferable, to + * take benefit of the explicit sorting above.) */ r = strverscmp_improved(a->id, b->id); if (r != 0) return r; @@ -1657,16 +1693,16 @@ static INTN config_entry_compare(const ConfigEntry *a, const ConfigEntry *b) { b->tries_left == UINTN_MAX) return 0; - /* If both items have boot counting, and otherwise are identical, put the entry with more tries left last */ - if (a->tries_left > b->tries_left) - return 1; + /* If both items have boot counting, and otherwise are identical, put the entry with more tries left first */ if (a->tries_left < b->tries_left) + return 1; + if (a->tries_left > b->tries_left) return -1; /* If they have the same number of tries left, then let the one win which was tried fewer times so far */ - if (a->tries_done < b->tries_done) - return 1; if (a->tries_done > b->tries_done) + return 1; + if (a->tries_done < b->tries_done) return -1; return 0; @@ -1678,7 +1714,7 @@ static UINTN config_entry_find(Config *config, const CHAR16 *needle) { if (!needle) return IDX_INVALID; - for (INTN i = config->entry_count - 1; i >= 0; i--) + for (UINTN i = 0; i < config->entry_count; i++) if (MetaiMatch(config->entries[i]->id, (CHAR16*) needle)) return i; @@ -1713,9 +1749,8 @@ static void config_default_entry_select(Config *config) { return; } - /* select the last suitable entry */ - i = config->entry_count; - while (i--) { + /* select the first suitable entry */ + for (i = 0; i < config->entry_count; i++) { if (config->entries[i]->type == LOADER_AUTO || config->entries[i]->call) continue; config->idx_default = i; @@ -1843,6 +1878,7 @@ static ConfigEntry *config_entry_add_loader( CHAR16 key, const CHAR16 *title, const CHAR16 *loader, + const CHAR16 *sort_key, const CHAR16 *version) { ConfigEntry *entry; @@ -1861,6 +1897,7 @@ static ConfigEntry *config_entry_add_loader( .device = device, .loader = xstrdup(loader), .id = xstrdup(id), + .sort_key = xstrdup(sort_key), .key = key, .tries_done = UINTN_MAX, .tries_left = UINTN_MAX, @@ -1935,7 +1972,7 @@ static ConfigEntry *config_entry_add_loader_auto( if (EFI_ERROR(err)) return NULL; - return config_entry_add_loader(config, device, LOADER_AUTO, id, key, title, loader, NULL); + return config_entry_add_loader(config, device, LOADER_AUTO, id, key, title, loader, NULL, NULL); } static void config_entry_add_osx(Config *config) { @@ -2113,7 +2150,7 @@ static void config_entry_add_linux( _cleanup_freepool_ CHAR16 *os_pretty_name = NULL, *os_image_id = NULL, *os_name = NULL, *os_id = NULL, *os_image_version = NULL, *os_version = NULL, *os_version_id = NULL, *os_build_id = NULL, *path = NULL; - const CHAR16 *good_name, *good_version; + const CHAR16 *good_name, *good_version, *good_sort_key; _cleanup_freepool_ CHAR8 *content = NULL; UINTN offs[_SECTION_MAX] = {}; UINTN szs[_SECTION_MAX] = {}; @@ -2194,7 +2231,7 @@ static void config_entry_add_linux( } } - if (!bootspec_pick_name_version( + if (!bootspec_pick_name_version_sort_key( os_pretty_name, os_image_id, os_name, @@ -2204,7 +2241,8 @@ static void config_entry_add_linux( os_version_id, os_build_id, &good_name, - &good_version)) + &good_version, + &good_sort_key)) continue; path = xpool_print(L"\\EFI\\Linux\\%s", f->FileName); @@ -2212,10 +2250,11 @@ static void config_entry_add_linux( config, device, LOADER_UNIFIED_LINUX, - f->FileName, + /* id= */ f->FileName, /* key= */ 'l', - good_name, - path, + /* title= */ good_name, + /* loader= */ path, + /* sort_key= */ good_sort_key, good_version); config_entry_parse_tries(entry, L"\\EFI\\Linux", f->FileName, L".efi"); diff --git a/src/fundamental/bootspec-fundamental.c b/src/fundamental/bootspec-fundamental.c index 9c4aee9744..89e29f5982 100644 --- a/src/fundamental/bootspec-fundamental.c +++ b/src/fundamental/bootspec-fundamental.c @@ -2,7 +2,7 @@ #include "bootspec-fundamental.h" -sd_bool bootspec_pick_name_version( +sd_bool bootspec_pick_name_version_sort_key( const sd_char *os_pretty_name, const sd_char *os_image_id, const sd_char *os_name, @@ -12,12 +12,14 @@ sd_bool bootspec_pick_name_version( const sd_char *os_version_id, const sd_char *os_build_id, const sd_char **ret_name, - const sd_char **ret_version) { + const sd_char **ret_version, + const sd_char **ret_sort_key) { - const sd_char *good_name, *good_version; + const sd_char *good_name, *good_version, *good_sort_key; - /* Find the best human readable title and version string for a boot entry (using the os-release(5) - * fields). Precise is preferred over vague, and human readable over machine readable. Thus: + /* Find the best human readable title, version string and sort key for a boot entry (using the + * os-release(5) fields). Precise is preferred over vague, and human readable over machine + * readable. Thus: * * 1. First priority gets the PRETTY_NAME field, which is the primary string intended for display, * and should already contain both a nice description and a version indication (if that concept @@ -37,11 +39,12 @@ sd_bool bootspec_pick_name_version( * which case the version is shown too. * * Note that name/version determined here are used only for display purposes. Boot entry preference - * sorting (i.e. algorithmic ordering of boot entries) is done based on the order of the entry "id" - * string (i.e. not on os-release(5) data). */ + * sorting (i.e. algorithmic ordering of boot entries) is done based on the order of the sort key (if + * defined) or entry "id" string (i.e. entry file name) otherwise. */ good_name = os_pretty_name ?: (os_image_id ?: (os_name ?: os_id)); good_version = os_image_version ?: (os_version ?: (os_version_id ? : os_build_id)); + good_sort_key = os_image_id ?: os_id; if (!good_name || !good_version) return sd_false; @@ -52,5 +55,8 @@ sd_bool bootspec_pick_name_version( if (ret_version) *ret_version = good_version; + if (ret_sort_key) + *ret_sort_key = good_sort_key; + return sd_true; } diff --git a/src/fundamental/bootspec-fundamental.h b/src/fundamental/bootspec-fundamental.h index 2cb6d7daa1..ff88f8bc3f 100644 --- a/src/fundamental/bootspec-fundamental.h +++ b/src/fundamental/bootspec-fundamental.h @@ -3,7 +3,7 @@ #include "types-fundamental.h" -sd_bool bootspec_pick_name_version( +sd_bool bootspec_pick_name_version_sort_key( const sd_char *os_pretty_name, const sd_char *os_image_id, const sd_char *os_name, @@ -13,4 +13,5 @@ sd_bool bootspec_pick_name_version( const sd_char *os_version_id, const sd_char *os_build_id, const sd_char **ret_name, - const sd_char **ret_version); + const sd_char **ret_version, + const sd_char **ret_sort_key); diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 524f0ccfcd..99abeac34b 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -45,6 +45,7 @@ static void boot_entry_free(BootEntry *entry) { free(entry->root); free(entry->title); free(entry->show_title); + free(entry->sort_key); free(entry->version); free(entry->machine_id); free(entry->architecture); @@ -131,6 +132,8 @@ static int boot_entry_load( if (streq(field, "title")) r = free_and_strdup(&tmp.title, p); + else if (streq(field, "sort-key")) + r = free_and_strdup(&tmp.sort_key, p); else if (streq(field, "version")) r = free_and_strdup(&tmp.version, p); else if (streq(field, "machine-id")) @@ -263,6 +266,28 @@ static int boot_loader_read_conf(const char *path, BootConfig *config) { } static int boot_entry_compare(const BootEntry *a, const BootEntry *b) { + int r; + + assert(a); + assert(b); + + r = CMP(!a->sort_key, !b->sort_key); + if (r != 0) + return r; + if (a->sort_key && b->sort_key) { + r = strcmp(a->sort_key, b->sort_key); + if (r != 0) + return r; + + r = strcmp_ptr(a->machine_id, b->machine_id); + if (r != 0) + return r; + + r = -strverscmp_improved(a->version, b->version); + if (r != 0) + return r; + } + return strverscmp_improved(a->id, b->id); } @@ -311,7 +336,7 @@ static int boot_entry_load_unified( _cleanup_(boot_entry_free) BootEntry tmp = { .type = BOOT_ENTRY_UNIFIED, }; - const char *k, *good_name, *good_version; + const char *k, *good_name, *good_version, *good_sort_key; _cleanup_fclose_ FILE *f = NULL; int r; @@ -339,7 +364,7 @@ static int boot_entry_load_unified( if (r < 0) return log_error_errno(r, "Failed to parse os-release data from unified kernel image %s: %m", path); - if (!bootspec_pick_name_version( + if (!bootspec_pick_name_version_sort_key( os_pretty_name, os_image_id, os_name, @@ -349,7 +374,8 @@ static int boot_entry_load_unified( os_version_id, os_build_id, &good_name, - &good_version)) + &good_version, + &good_sort_key)) return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "Missing fields in os-release data from unified kernel image %s, refusing.", path); r = path_extract_filename(path, &tmp.id); @@ -387,6 +413,10 @@ static int boot_entry_load_unified( if (!tmp.title) return log_oom(); + tmp.sort_key = strdup(good_sort_key); + if (!tmp.sort_key) + return log_oom(); + tmp.version = strdup(good_version); if (!tmp.version) return log_oom(); @@ -634,6 +664,19 @@ static int boot_entries_uniquify(BootEntry *entries, size_t n_entries) { return 0; } +static int boot_config_find(const BootConfig *config, const char *id) { + assert(config); + + if (!id) + return -1; + + for (size_t i = 0; i < config->n_entries; i++) + if (fnmatch(id, config->entries[i].id, FNM_CASEFOLD) == 0) + return i; + + return -1; +} + static int boot_entries_select_default(const BootConfig *config) { int i; @@ -645,47 +688,45 @@ static int boot_entries_select_default(const BootConfig *config) { return -1; /* -1 means "no default" */ } - if (config->entry_oneshot) - for (i = config->n_entries - 1; i >= 0; i--) - if (streq(config->entry_oneshot, config->entries[i].id)) { - log_debug("Found default: id \"%s\" is matched by LoaderEntryOneShot", - config->entries[i].id); - return i; - } + if (config->entry_oneshot) { + i = boot_config_find(config, config->entry_oneshot); + if (i >= 0) { + log_debug("Found default: id \"%s\" is matched by LoaderEntryOneShot", + config->entries[i].id); + return i; + } + } - if (config->entry_default) - for (i = config->n_entries - 1; i >= 0; i--) - if (streq(config->entry_default, config->entries[i].id)) { - log_debug("Found default: id \"%s\" is matched by LoaderEntryDefault", - config->entries[i].id); - return i; - } + if (config->entry_default) { + i = boot_config_find(config, config->entry_default); + if (i >= 0) { + log_debug("Found default: id \"%s\" is matched by LoaderEntryDefault", + config->entries[i].id); + return i; + } + } - if (config->default_pattern) - for (i = config->n_entries - 1; i >= 0; i--) - if (fnmatch(config->default_pattern, config->entries[i].id, FNM_CASEFOLD) == 0) { - log_debug("Found default: id \"%s\" is matched by pattern \"%s\"", - config->entries[i].id, config->default_pattern); - return i; - } + if (config->default_pattern) { + i = boot_config_find(config, config->default_pattern); + if (i >= 0) { + log_debug("Found default: id \"%s\" is matched by pattern \"%s\"", + config->entries[i].id, config->default_pattern); + return i; + } + } - log_debug("Found default: last entry \"%s\"", config->entries[config->n_entries - 1].id); - return config->n_entries - 1; + log_debug("Found default: first entry \"%s\"", config->entries[0].id); + return 0; } static int boot_entries_select_selected(const BootConfig *config) { - assert(config); assert(config->entries || config->n_entries == 0); if (!config->entry_selected || config->n_entries == 0) return -1; - for (int i = config->n_entries - 1; i >= 0; i--) - if (streq(config->entry_selected, config->entries[i].id)) - return i; - - return -1; + return boot_config_find(config, config->entry_selected); } static int boot_load_efi_entry_pointers(BootConfig *config) { diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h index 16353746ae..64052af593 100644 --- a/src/shared/bootspec.h +++ b/src/shared/bootspec.h @@ -28,6 +28,7 @@ typedef struct BootEntry { char *root; /* The root path in which the drop-in was found, i.e. to which 'kernel', 'efi' and 'initrd' are relative */ char *title; char *show_title; + char *sort_key; char *version; char *machine_id; char *architecture; -- 2.25.1