From: Zbigniew Jędrzejewski-Szmek Date: Fri, 1 Apr 2022 10:08:17 +0000 (+0200) Subject: bootctl: unify boot entry loading for "status" and "list" X-Git-Tag: v251-rc2~201^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8214758bd5d3bda7c1a4b4b79467266f833cc3ac;p=thirdparty%2Fsystemd.git bootctl: unify boot entry loading for "status" and "list" We must be consistent in the two listings, so let's split out the loading code and call it from both verb_status() and verb_list(). This effectively makes verb_status() also call efi_loader_get_entries(). There is still some code duplicated, but that's hard to avoid. Error messages are made identical for the same errors in various places. Fixes #22580. --- diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index c4cb33c7056..a393370f8d4 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -583,6 +583,35 @@ static const char* const boot_entry_type_table[_BOOT_ENTRY_TYPE_MAX] = { DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(boot_entry_type, BootEntryType); +static int boot_config_load_and_select( + BootConfig *config, + const char *esp_path, + dev_t esp_devid, + const char *xbootldr_path, + dev_t xbootldr_devid) { + + _cleanup_strv_free_ char **efi_entries = NULL; + int r; + + /* If XBOOTLDR and ESP actually refer to the same block device, suppress XBOOTLDR, since it would + * find the same entries twice. */ + bool same = esp_path && xbootldr_path && devid_set_and_equal(esp_devid, xbootldr_devid); + + r = boot_config_load(config, esp_path, same ? NULL : xbootldr_path); + if (r < 0) + return r; + + r = efi_loader_get_entries(&efi_entries); + if (r == -ENOENT || ERRNO_IS_NOT_SUPPORTED(r)) + log_debug_errno(r, "Boot loader reported no entries."); + else if (r < 0) + log_warning_errno(r, "Failed to determine entries reported by boot loader, ignoring: %m"); + else + (void) boot_config_augment_from_loader(config, efi_entries, /* only_auto= */ false); + + return boot_config_select_special_entries(config); +} + static int boot_entry_show( const BootEntry *e, bool show_as_default, @@ -682,16 +711,17 @@ static int boot_entry_show( } static int status_entries( + const BootConfig *config, const char *esp_path, sd_id128_t esp_partition_uuid, const char *xbootldr_path, sd_id128_t xbootldr_partition_uuid) { - _cleanup_(boot_config_free) BootConfig config = BOOT_CONFIG_NULL; sd_id128_t dollar_boot_partition_uuid; const char *dollar_boot_path; int r; + assert(config); assert(esp_path || xbootldr_path); if (xbootldr_path) { @@ -709,21 +739,13 @@ static int status_entries( SD_ID128_FORMAT_VAL(dollar_boot_partition_uuid)); printf("\n\n"); - r = boot_config_load(&config, esp_path, xbootldr_path); - if (r < 0) - return r; - - r = boot_config_select_special_entries(&config); - if (r < 0) - return r; - - if (config.default_entry < 0) - printf("%zu entries, no entry could be determined as default.\n", config.n_entries); + if (config->default_entry < 0) + printf("%zu entries, no entry could be determined as default.\n", config->n_entries); else { printf("Default Boot Loader Entry:\n"); r = boot_entry_show( - boot_config_default_entry(&config), + boot_config_default_entry(config), /* show_as_default= */ false, /* show_as_selected= */ false, /* show_discovered= */ false); @@ -1641,7 +1663,7 @@ static int verb_status(int argc, char *argv[], void *userdata) { r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, &xbootldr_uuid, &xbootldr_devid); if (arg_print_dollar_boot_path) { if (r == -EACCES) - return log_error_errno(r, "Failed to determine XBOOTLDR location: %m"); + return log_error_errno(r, "Failed to determine XBOOTLDR partition: %m"); if (r < 0) return r; @@ -1772,16 +1794,20 @@ static int verb_status(int argc, char *argv[], void *userdata) { } if (arg_esp_path || arg_xbootldr_path) { - /* If XBOOTLDR and ESP actually refer to the same block device, suppress XBOOTLDR, since it would find the same entries twice */ - bool same = arg_esp_path && arg_xbootldr_path && devid_set_and_equal(esp_devid, xbootldr_devid); - - k = status_entries( - arg_esp_path, - esp_uuid, - same ? NULL : arg_xbootldr_path, - same ? SD_ID128_NULL : xbootldr_uuid); + _cleanup_(boot_config_free) BootConfig config = BOOT_CONFIG_NULL; + + k = boot_config_load_and_select(&config, + arg_esp_path, esp_devid, + arg_xbootldr_path, xbootldr_devid); if (k < 0) r = k; + else { + k = status_entries(&config, + arg_esp_path, esp_uuid, + arg_xbootldr_path, xbootldr_uuid); + if (k < 0) + r = k; + } } return r; @@ -1789,17 +1815,17 @@ static int verb_status(int argc, char *argv[], void *userdata) { static int verb_list(int argc, char *argv[], void *userdata) { _cleanup_(boot_config_free) BootConfig config = BOOT_CONFIG_NULL; - _cleanup_strv_free_ char **efi_entries = NULL; dev_t esp_devid = 0, xbootldr_devid = 0; int r; - /* If we lack privileges we invoke find_esp_and_warn() in "unprivileged mode" here, which does two things: turn - * off logging about access errors and turn off potentially privileged device probing. Here we're interested in - * the latter but not the former, hence request the mode, and log about EACCES. */ + /* If we lack privileges we invoke find_esp_and_warn() in "unprivileged mode" here, which does two + * things: turn off logging about access errors and turn off potentially privileged device probing. + * Here we're interested in the latter but not the former, hence request the mode, and log about + * EACCES. */ r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, NULL, &esp_devid); if (r == -EACCES) /* We really need the ESP path for this call, hence also log about access errors */ - return log_error_errno(r, "Failed to determine ESP: %m"); + return log_error_errno(r, "Failed to determine ESP location: %m"); if (r < 0) return r; @@ -1809,22 +1835,7 @@ static int verb_list(int argc, char *argv[], void *userdata) { if (r < 0) return r; - /* If XBOOTLDR and ESP actually refer to the same block device, suppress XBOOTLDR, since it would find the same entries twice */ - bool same = arg_esp_path && arg_xbootldr_path && devid_set_and_equal(esp_devid, xbootldr_devid); - - r = boot_config_load(&config, arg_esp_path, same ? NULL : arg_xbootldr_path); - if (r < 0) - return r; - - r = efi_loader_get_entries(&efi_entries); - if (r == -ENOENT || ERRNO_IS_NOT_SUPPORTED(r)) - log_debug_errno(r, "Boot loader reported no entries."); - else if (r < 0) - log_warning_errno(r, "Failed to determine entries reported by boot loader, ignoring: %m"); - else - (void) boot_config_augment_from_loader(&config, efi_entries, /* only_auto= */ false); - - r = boot_config_select_special_entries(&config); + r = boot_config_load_and_select(&config, arg_esp_path, esp_devid, arg_xbootldr_path, xbootldr_devid); if (r < 0) return r; diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h index 99bc5f69a49..93fb22e9d40 100644 --- a/src/shared/bootspec.h +++ b/src/shared/bootspec.h @@ -71,7 +71,7 @@ typedef struct BootConfig { BootEntry* boot_config_find_entry(BootConfig *config, const char *id); -static inline BootEntry* boot_config_default_entry(BootConfig *config) { +static inline const BootEntry* boot_config_default_entry(const BootConfig *config) { assert(config); if (config->default_entry < 0)