From 5aa285b437d14c809649beaa2b47341b0de4a67d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 26 May 2022 20:56:12 +0200 Subject: [PATCH 1/3] kernel-install: if a plugin fails, return error immediately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since the first version in 81516adcb71a47837544340f72eb8ee810274119, kernel-install would "gather" a return value by summing the exit codes of the plugins… This makes no sense, because those are not additive values. Let's just break off immediately. We now implement cleanup via trap, so if we break, we should leave no garbage behind. --- src/kernel-install/kernel-install.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kernel-install/kernel-install.in b/src/kernel-install/kernel-install.in index 5f88ac2490..9296bfcd8e 100755 --- a/src/kernel-install/kernel-install.in +++ b/src/kernel-install/kernel-install.in @@ -336,7 +336,7 @@ case "$COMMAND" in err=$? [ $err -eq $skip_remaining ] && break - ret=$(( ret + err )) + [ $err -ne 0 ] && exit $err done ;; @@ -346,7 +346,7 @@ case "$COMMAND" in "$f" remove "$KERNEL_VERSION" "$ENTRY_DIR_ABS" err=$? [ $err -eq $skip_remaining ] && break - ret=$(( ret + err )) + [ $err -ne 0 ] && exit $err done if [ "$MAKE_ENTRY_DIR_ABS" -eq 0 ]; then From 4e124425547e243482a1bae43cd20853de13172f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 26 May 2022 21:36:33 +0200 Subject: [PATCH 2/3] shared/find-esp: fix inverted check for XBOOTLDR type --- src/shared/find-esp.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/shared/find-esp.c b/src/shared/find-esp.c index 75e639dd99..3b0c6d3fa9 100644 --- a/src/shared/find-esp.c +++ b/src/shared/find-esp.c @@ -571,7 +571,11 @@ static int verify_xbootldr_udev( r = sd_device_get_property_value(d, "ID_PART_ENTRY_TYPE", &v); if (r < 0) return log_error_errno(r, "Failed to get device property: %m"); - if (id128_equal_string(v, GPT_XBOOTLDR)) + + r = id128_equal_string(v, GPT_XBOOTLDR); + if (r < 0) + return log_error_errno(r, "Failed to parse ID_PART_ENTRY_TYPE=%s: %m", v); + if (r == 0) return log_full_errno(searching ? LOG_DEBUG : LOG_ERR, searching ? SYNTHETIC_ERRNO(EADDRNOTAVAIL) : SYNTHETIC_ERRNO(ENODEV), "File system \"%s\" has wrong type for extended boot loader partition.", node); From 13d7c841a24f784995a000230706f259c4baec96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 26 May 2022 21:36:59 +0200 Subject: [PATCH 3/3] shared/find-esp: enhance logging If something doesn't match, let's print the non-matching value. If we can't query something, say what. And make the messages in the udev and blkid paths different, so we tell which approach failed from a log. --- src/shared/find-esp.c | 87 ++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 38 deletions(-) diff --git a/src/shared/find-esp.c b/src/shared/find-esp.c index 3b0c6d3fa9..09c09c8e7e 100644 --- a/src/shared/find-esp.c +++ b/src/shared/find-esp.c @@ -7,6 +7,7 @@ #include "alloc-util.h" #include "blkid-util.h" +#include "device-util.h" #include "devnum-util.h" #include "env-util.h" #include "errno-util.h" @@ -474,16 +475,18 @@ static int verify_xbootldr_blkid( #if HAVE_BLKID _cleanup_(blkid_free_probep) blkid_probe b = NULL; _cleanup_free_ char *node = NULL; - const char *v; + const char *type, *v; int r; r = device_path_make_major_minor(S_IFBLK, devid, &node); if (r < 0) - return log_error_errno(r, "Failed to format major/minor device path: %m"); + return log_error_errno(r, "Failed to format block device path for %u:%u: %m", + major(devid), minor(devid)); + errno = 0; b = blkid_new_probe_from_filename(node); if (!b) - return log_error_errno(errno ?: SYNTHETIC_ERRNO(ENOMEM), "Failed to open file system \"%s\": %m", node); + return log_error_errno(errno ?: SYNTHETIC_ERRNO(ENOMEM), "%s: Failed to create blkid probe: %m", node); blkid_probe_enable_partitions(b, 1); blkid_probe_set_partitions_flags(b, BLKID_PARTS_ENTRY_DETAILS); @@ -491,50 +494,50 @@ static int verify_xbootldr_blkid( errno = 0; r = blkid_do_safeprobe(b); if (r == -2) - return log_error_errno(SYNTHETIC_ERRNO(ENODEV), "File system \"%s\" is ambiguous.", node); + return log_error_errno(SYNTHETIC_ERRNO(ENODEV), "%s: File system is ambiguous.", node); else if (r == 1) - return log_error_errno(SYNTHETIC_ERRNO(ENODEV), "File system \"%s\" does not contain a label.", node); + return log_error_errno(SYNTHETIC_ERRNO(ENODEV), "%s: File system does not contain a label.", node); else if (r != 0) - return log_error_errno(errno ?: SYNTHETIC_ERRNO(EIO), "Failed to probe file system \"%s\": %m", node); + return log_error_errno(errno ?: SYNTHETIC_ERRNO(EIO), "%s: Failed to probe file system: %m", node); errno = 0; - r = blkid_probe_lookup_value(b, "PART_ENTRY_SCHEME", &v, NULL); + r = blkid_probe_lookup_value(b, "PART_ENTRY_SCHEME", &type, NULL); if (r != 0) - return log_error_errno(errno ?: SYNTHETIC_ERRNO(EIO), "Failed to probe partition scheme of \"%s\": %m", node); - if (streq(v, "gpt")) { + return log_error_errno(errno ?: SYNTHETIC_ERRNO(EIO), "%s: Failed to probe PART_ENTRY_SCHEME: %m", node); + if (streq(type, "gpt")) { errno = 0; r = blkid_probe_lookup_value(b, "PART_ENTRY_TYPE", &v, NULL); if (r != 0) - return log_error_errno(errno ?: SYNTHETIC_ERRNO(EIO), "Failed to probe partition type UUID of \"%s\": %m", node); + return log_error_errno(errno ?: SYNTHETIC_ERRNO(EIO), "%s: Failed to probe PART_ENTRY_TYPE: %m", node); if (id128_equal_string(v, GPT_XBOOTLDR) <= 0) return log_full_errno(searching ? LOG_DEBUG : LOG_ERR, searching ? SYNTHETIC_ERRNO(EADDRNOTAVAIL) : SYNTHETIC_ERRNO(ENODEV), - "File system \"%s\" has wrong type for extended boot loader partition.", node); + "%s: Partitition has wrong PART_ENTRY_TYPE=%s for XBOOTLDR partition.", node, v); errno = 0; r = blkid_probe_lookup_value(b, "PART_ENTRY_UUID", &v, NULL); if (r != 0) - return log_error_errno(errno ?: SYNTHETIC_ERRNO(EIO), "Failed to probe partition entry UUID of \"%s\": %m", node); + return log_error_errno(errno ?: SYNTHETIC_ERRNO(EIO), "%s: Failed to probe PART_ENTRY_UUID: %m", node); r = sd_id128_from_string(v, &uuid); if (r < 0) - return log_error_errno(r, "Partition \"%s\" has invalid UUID \"%s\".", node, v); + return log_error_errno(r, "%s: Partition has invalid UUID PART_ENTRY_TYPE=%s: %m", node, v); - } else if (streq(v, "dos")) { + } else if (streq(type, "dos")) { errno = 0; r = blkid_probe_lookup_value(b, "PART_ENTRY_TYPE", &v, NULL); if (r != 0) - return log_error_errno(errno ?: SYNTHETIC_ERRNO(EIO), "Failed to probe partition type UUID of \"%s\": %m", node); + return log_error_errno(errno ?: SYNTHETIC_ERRNO(EIO), "%s: Failed to probe PART_ENTRY_TYPE: %m", node); if (!streq(v, "0xea")) return log_full_errno(searching ? LOG_DEBUG : LOG_ERR, searching ? SYNTHETIC_ERRNO(EADDRNOTAVAIL) : SYNTHETIC_ERRNO(ENODEV), - "File system \"%s\" has wrong type for extended boot loader partition.", node); + "%s: Wrong PART_ENTRY_TYPE=%s for XBOOTLDR partition.", node, v); } else return log_full_errno(searching ? LOG_DEBUG : LOG_ERR, searching ? SYNTHETIC_ERRNO(EADDRNOTAVAIL) : SYNTHETIC_ERRNO(ENODEV), - "File system \"%s\" is not on a GPT or DOS partition table.", node); + "%s: Not on a GPT or DOS partition table (PART_ENTRY_SCHEME=%s).", node, type); #endif if (ret_uuid) @@ -551,55 +554,63 @@ static int verify_xbootldr_udev( _cleanup_(sd_device_unrefp) sd_device *d = NULL; _cleanup_free_ char *node = NULL; sd_id128_t uuid = SD_ID128_NULL; - const char *v; + const char *type, *v; int r; r = device_path_make_major_minor(S_IFBLK, devid, &node); if (r < 0) - return log_error_errno(r, "Failed to format major/minor device path: %m"); + return log_error_errno(r, "Failed to format block device path for %u:%u: %m", + major(devid), minor(devid)); r = sd_device_new_from_devnum(&d, 'b', devid); if (r < 0) - return log_error_errno(r, "Failed to get device from device number: %m"); + return log_error_errno(r, "%s: Failed to get block device: %m", node); - r = sd_device_get_property_value(d, "ID_PART_ENTRY_SCHEME", &v); + r = sd_device_get_property_value(d, "ID_PART_ENTRY_SCHEME", &type); if (r < 0) - return log_error_errno(r, "Failed to get device property: %m"); + return log_device_error_errno(d, r, "Failed to query ID_PART_ENTRY_SCHEME: %m"); - if (streq(v, "gpt")) { + if (streq(type, "gpt")) { r = sd_device_get_property_value(d, "ID_PART_ENTRY_TYPE", &v); if (r < 0) - return log_error_errno(r, "Failed to get device property: %m"); + return log_device_error_errno(d, r, "Failed to query ID_PART_ENTRY_TYPE: %m"); r = id128_equal_string(v, GPT_XBOOTLDR); if (r < 0) - return log_error_errno(r, "Failed to parse ID_PART_ENTRY_TYPE=%s: %m", v); + return log_device_error_errno(d, r, "Failed to parse ID_PART_ENTRY_TYPE=%s: %m", v); if (r == 0) - return log_full_errno(searching ? LOG_DEBUG : LOG_ERR, - searching ? SYNTHETIC_ERRNO(EADDRNOTAVAIL) : SYNTHETIC_ERRNO(ENODEV), - "File system \"%s\" has wrong type for extended boot loader partition.", node); + return log_device_full_errno( + d, + searching ? LOG_DEBUG : LOG_ERR, + searching ? SYNTHETIC_ERRNO(EADDRNOTAVAIL) : SYNTHETIC_ERRNO(ENODEV), + "Parition has wrong ID_PART_ENTRY_TYPE=%s for XBOOTLDR partition.", v); r = sd_device_get_property_value(d, "ID_PART_ENTRY_UUID", &v); if (r < 0) - return log_error_errno(r, "Failed to get device property: %m"); + return log_device_error_errno(d, r, "Failed to query ID_PART_ENTRY_UUID: %m"); r = sd_id128_from_string(v, &uuid); if (r < 0) - return log_error_errno(r, "Partition \"%s\" has invalid UUID \"%s\".", node, v); + return log_device_error_errno(d, r, "Partition has invalid UUID ID_PART_ENTRY_TYPE=%s: %m", v); - } else if (streq(v, "dos")) { + } else if (streq(type, "dos")) { r = sd_device_get_property_value(d, "ID_PART_ENTRY_TYPE", &v); if (r < 0) - return log_error_errno(r, "Failed to get device property: %m"); + return log_device_error_errno(d, r, "Failed to query ID_PART_ENTRY_TYPE: %m"); if (!streq(v, "0xea")) - return log_full_errno(searching ? LOG_DEBUG : LOG_ERR, - searching ? SYNTHETIC_ERRNO(EADDRNOTAVAIL) : SYNTHETIC_ERRNO(ENODEV), - "File system \"%s\" has wrong type for extended boot loader partition.", node); + return log_device_full_errno( + d, + searching ? LOG_DEBUG : LOG_ERR, + searching ? SYNTHETIC_ERRNO(EADDRNOTAVAIL) : SYNTHETIC_ERRNO(ENODEV), + "Wrong ID_PART_ENTRY_TYPE=%s for XBOOTLDR partition.", v); + } else - return log_full_errno(searching ? LOG_DEBUG : LOG_ERR, - searching ? SYNTHETIC_ERRNO(EADDRNOTAVAIL) : SYNTHETIC_ERRNO(ENODEV), - "File system \"%s\" is not on a GPT or DOS partition table.", node); + return log_device_full_errno( + d, + searching ? LOG_DEBUG : LOG_ERR, + searching ? SYNTHETIC_ERRNO(EADDRNOTAVAIL) : SYNTHETIC_ERRNO(ENODEV), + "Not on a GPT or DOS partition table (ID_PART_ENTRY_SCHEME=%s).", type); if (ret_uuid) *ret_uuid = uuid;