From: Zbigniew Jędrzejewski-Szmek Date: Fri, 8 Sep 2023 16:19:36 +0000 (+0200) Subject: hibernate-resume: remove kernel/image version comparison when resuming X-Git-Tag: v255-rc1~262 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=fed0a899b22b261664a22ef61730f993f9b4cb82;p=thirdparty%2Fsystemd.git hibernate-resume: remove kernel/image version comparison when resuming We already had a similar check that was removed, see 8340b762e4f597e98a72de1385e74b9be04e521d (*). The kernel supports loading of a resume image from a different kernel version. This makes sense, because the goal of "resume" is to replace the running system by a saved memory image, so it doesn't really matter that the short-lived kernel is different. By removing the check, we make the process more reliable: for example, the user may select a different kernel from a list, or not have the previously running kernel in /boot at all, etc. Requiring the exact same kernel version makes the process more fragile for no benefit. Similar reasoning holds for the image version: the image may be updated, and for example an older kernel+initrd might be used, with an embedded VERSION_ID that is not the latest. This is fine, and the check is not useful. I left the check for ID/IMAGE_ID: we probably don't want to use the resume image if the hibernation was done from a different installation. (Note: why not check VERSION_ID/IMAGE_VERSION? Because of the following scenario: a user has an installation of Fedora 35, and they upgrade to Fedora 36, which means that the os-release file on disk gets replaced and now specifies VERSION_ID=36. But the running kernel is not replaced, and its package is not removed because the running kernel version is never removed, so we still have a boot entry that in initrd-release says VERSION_ID=35. Without rebooting, the user does hibernation. When resuming, we want to resume, no matter if one of the new entries with VERSION_ID=36 or one of the old entries with VERSION_ID=35 is picked in the boot loader menu. If the installation is image-based, i.e. it has IMAGE_ID+IMAGE_VERSION, the situation is similar: after an upgrade, we may still have an boot entry from before the upgrade. Using an older kernel+initrd to boot and switch-root into a newer installation is supported and is rather common. In fact, it is a rather common situation that the version reported by the boot entry (or stored internally in the initrd-release in the initrd) does not match the actual system on disk. Generally, this metadata is saved when the boot menu entry is written and does not reflect subsequent upgrades. Various distributions generally keep at least 3 kernels after a upgrade, and during an upgrade only install one new, which means that after a major upgrade, generally there will be at least two kernels which have mismatched version information.) OTOH, I think it is useful to *write* all the details to the EFI var. As discussed in https://github.com/systemd/systemd/issues/29037, we may want to show this information in the boot loader. It is also useful for debugging. (*) Also again discussed and verified in https://github.com/systemd/systemd/pull/27330#discussion_r1234332080. ", ignored" is dropped, since this failure is likely to cause the following check to fail. Better not to say anything then to say the misleading thing. --- diff --git a/src/hibernate-resume/hibernate-resume-config.c b/src/hibernate-resume/hibernate-resume-config.c index 4aac6234d0f..6d0987eedd1 100644 --- a/src/hibernate-resume/hibernate-resume-config.c +++ b/src/hibernate-resume/hibernate-resume-config.c @@ -1,7 +1,5 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include - #include "alloc-util.h" #include "device-nodes.h" #include "fstab-util.h" @@ -110,33 +108,29 @@ static int get_kernel_hibernate_location(KernelHibernateLocation **ret) { #if ENABLE_EFI static bool validate_efi_hibernate_location(EFIHibernateLocation *e) { - _cleanup_free_ char *id = NULL, *image_id = NULL, *version_id = NULL, *image_version = NULL; - struct utsname uts = {}; + _cleanup_free_ char *id = NULL, *image_id = NULL; int r; assert(e); - if (uname(&uts) < 0) - log_warning_errno(errno, "Failed to get kernel info, ignoring: %m"); - r = parse_os_release(NULL, "ID", &id, - "IMAGE_ID", &image_id, - "VERSION_ID", &version_id, - "IMAGE_VERSION", &image_version); + "IMAGE_ID", &image_id); if (r < 0) - log_warning_errno(r, "Failed to parse os-release, ignoring: %m"); - - if (!streq(uts.release, strempty(e->kernel_version)) || - !streq_ptr(id, e->id) || - !streq_ptr(image_id, e->image_id) || - !streq_ptr(version_id, e->version_id) || - !streq_ptr(image_version, e->image_version)) { + log_warning_errno(r, "Failed to parse os-release: %m"); - log_notice("HibernateLocation system info doesn't match with current running system, not resuming from it."); + if (!streq_ptr(id, e->id) || + !streq_ptr(image_id, e->image_id)) { + log_notice("HibernateLocation system identifier doesn't match currently running system, not resuming from it."); return false; } + /* + * Note that we accept kernel version mismatches. Linux writes the old kernel to disk as part of the + * hibernation image, and thus resuming means the short-lived kernel that reads the image from the + * disk will be replaced by the original kernel and effectively removed from memory as part of that. + */ + return true; } @@ -183,6 +177,15 @@ static int get_efi_hibernate_location(EFIHibernateLocation **ret) { if (r < 0) return r; + log_info("Reported hibernation image:%s%s%s%s%s%s%s%s%s%s UUID="SD_ID128_UUID_FORMAT_STR" offset=%"PRIu64, + e->id ? " ID=" : "", strempty(e->id), + e->image_id ? " IMAGE_ID=" : "", strempty(e->image_id), + e->version_id ? " VERSION_ID=" : "", strempty(e->version_id), + e->image_version ? " IMAGE_VERSION=" : "", strempty(e->image_version), + e->kernel_version ? " kernel=" : "", strempty(e->kernel_version), + SD_ID128_FORMAT_VAL(e->uuid), + e->offset); + if (!validate_efi_hibernate_location(e)) goto skip;