From 7538608a2587e2bfe59d9ca433f40aacd00ab1a7 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Wed, 9 Mar 2022 12:45:29 -0700 Subject: [PATCH 1/2] libeos-updater-util: Add method to detect if /boot is an automount On some hosts, systemd sets up `/boot` as an autofs automount. Unfortunately, this configuration has some bugs that make usage of `/boot` less than robust. Add a method to see if `/boot` is an automount. This uses libmount to parse `/proc/self/mountinfo` and check if the first mountpoint for the sysroot's boot directory has the `autofs` filesystem type. libmount version 2.24 was chosen as a minimum because that's when `libmnt_table` and `libmnt_cache` reference counting was added. https://phabricator.endlessm.com/T33136 --- debian/control | 1 + libeos-updater-util/meson.build | 1 + libeos-updater-util/ostree-util.c | 77 +++++++++++++++++++++++ libeos-updater-util/ostree-util.h | 3 + libeos-updater-util/tests/ostree-util.c | 81 +++++++++++++++++++++++++ 5 files changed, 163 insertions(+) diff --git a/debian/control b/debian/control index 1750c3c7d..084a5f467 100644 --- a/debian/control +++ b/debian/control @@ -22,6 +22,7 @@ Build-Depends: libgsystem-dev, libjson-glib-dev (>= 1.2.6-1endless2), libmogwai-schedule-client-0-dev, + libmount-dev (>= 2.24), libnm-dev (>= 1.2.0), libostree-dev (>= 2017.12), libsoup2.4-dev, diff --git a/libeos-updater-util/meson.build b/libeos-updater-util/meson.build index 871ac4025..bba4a4771 100644 --- a/libeos-updater-util/meson.build +++ b/libeos-updater-util/meson.build @@ -36,6 +36,7 @@ libeos_updater_util_deps = [ dependency('gobject-2.0', version: '>= 2.62'), dependency('json-glib-1.0', version: '>= 1.2.6'), dependency('libsoup-2.4'), + dependency('mount', version: '>= 2.24'), dependency('ostree-1', version: '>= 2018.6'), ] diff --git a/libeos-updater-util/ostree-util.c b/libeos-updater-util/ostree-util.c index 18d70d594..aa7d8a304 100644 --- a/libeos-updater-util/ostree-util.c +++ b/libeos-updater-util/ostree-util.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -311,3 +312,79 @@ eos_updater_get_ostree_path (OstreeRepo *repo, *ostree_path = g_steal_pointer (&path); return TRUE; } + +/** + * eos_updater_sysroot_boot_is_automount: + * @sysroot: loaded OSTree sysroot to use + * @mountinfo: (nullable): path to mountinfo file + * + * Find if the boot directory in @sysroot is an automount mount. The first + * mountpoint for the boot directory in @mountinfo is found. If the filesystem + * type for the mountpoint is autofs, then the boot directory is automounted. + * + * If @mountinfo is %NULL, /proc/self/mountinfo is used. + * + * Returns: %TRUE if boot is an automount + */ +gboolean +eos_updater_sysroot_boot_is_automount (OstreeSysroot *sysroot, + const gchar *mountinfo) +{ + GFile *sysroot_file; + g_autoptr(GFile) boot_file = NULL; + g_autofree gchar *boot_path = NULL; + gboolean ret = FALSE; + struct libmnt_table *tb = NULL; + struct libmnt_cache *cache; + struct libmnt_fs *fs; + + sysroot_file = ostree_sysroot_get_path (sysroot); + boot_file = g_file_get_child (sysroot_file, "boot"); + boot_path = g_file_get_path (boot_file); + if (boot_path == NULL) + { + g_autofree gchar *sysroot_path = g_file_get_path (sysroot_file); + g_warning ("No boot directory in sysroot %s", sysroot_path); + goto out; + } + + if (mountinfo == NULL) + mountinfo = "/proc/self/mountinfo"; + tb = mnt_new_table_from_file (mountinfo); + if (tb == NULL) + { + g_warning ("Failed to parse %s: %s", mountinfo, g_strerror (errno)); + goto out; + } + + cache = mnt_new_cache (); + if (cache == NULL) + { + g_warning ("Failed to create libmount cache: %s", g_strerror (errno)); + goto out; + } + mnt_table_set_cache (tb, cache); + mnt_unref_cache (cache); + + /* Find the /boot mountpoint iterating forward so that the autofs mount is + * found and not the automount target filesystem, as the autofs mount is + * listed first in mountinfo. + */ + fs = mnt_table_find_mountpoint (tb, boot_path, MNT_ITER_FORWARD); + if (fs == NULL) + { + g_warning ("Failed to find mountpoint for %s: %s", boot_path, + g_strerror (errno)); + goto out; + } + + ret = g_strcmp0 (mnt_fs_get_fstype (fs), "autofs") == 0; + g_debug ("Boot directory %s is%s on an autofs filesystem", + boot_path, ret ? "" : " not"); + + out: + if (tb != NULL) + mnt_unref_table (tb); + + return ret; +} diff --git a/libeos-updater-util/ostree-util.h b/libeos-updater-util/ostree-util.h index 0248d8d55..245627a7f 100644 --- a/libeos-updater-util/ostree-util.h +++ b/libeos-updater-util/ostree-util.h @@ -47,4 +47,7 @@ gboolean eos_updater_get_ostree_path (OstreeRepo *repo, gchar **ostree_path, GError **error); +gboolean eos_updater_sysroot_boot_is_automount (OstreeSysroot *sysroot, + const gchar *mountinfo); + G_END_DECLS diff --git a/libeos-updater-util/tests/ostree-util.c b/libeos-updater-util/tests/ostree-util.c index 958721a22..6d98e4d13 100644 --- a/libeos-updater-util/tests/ostree-util.c +++ b/libeos-updater-util/tests/ostree-util.c @@ -147,6 +147,85 @@ test_ostree_no_deployments (Fixture *fixture, g_assert_cmpuint (commit_timestamp, ==, 0); } +static void +test_ostree_sysroot_boot_automount (Fixture *fixture, + gconstpointer user_data G_GNUC_UNUSED) +{ + g_autoptr(GError) error = NULL; + gboolean retval; + GFile *sysroot_file = ostree_sysroot_get_path (fixture->sysroot); + g_autofree gchar *sysroot_path = g_file_get_path (sysroot_file); + g_autoptr(GFile) boot_file = g_file_get_child (sysroot_file, "boot"); + g_autofree gchar *boot_path = g_file_get_path (boot_file); + g_autoptr(GFile) mountinfo_file = g_file_get_child (fixture->tmp_dir, + "mountinfo"); + g_autofree gchar *mountinfo_path = g_file_get_path (mountinfo_file); + g_autofree gchar *mountinfo_contents = NULL; + + retval = g_file_make_directory (boot_file, NULL, &error); + g_assert_no_error (error); + g_assert_true (retval); + + /* No separate /boot mount */ + g_clear_pointer (&mountinfo_contents, g_free); + mountinfo_contents = + g_strdup_printf ("1 1 1:1 / %s rw - ext4 /dev/sda1 rw\n", sysroot_path); + g_test_message ("boot %s, mountinfo:\n%s", boot_path, mountinfo_contents); + retval = g_file_replace_contents (mountinfo_file, mountinfo_contents, + strlen (mountinfo_contents), NULL, FALSE, + G_FILE_CREATE_NONE, NULL, NULL, &error); + g_assert_no_error (error); + g_assert_true (retval); + retval = eos_updater_sysroot_boot_is_automount (fixture->sysroot, mountinfo_path); + g_assert_false (retval); + + /* Non-automount /boot */ + g_clear_pointer (&mountinfo_contents, g_free); + mountinfo_contents = + g_strdup_printf ("1 1 1:2 / %s rw - ext4 /dev/sda2 rw\n" + "2 1 1:1 / %s rw - ext4 /dev/sda1 rw\n", + sysroot_path, boot_path); + g_test_message ("boot %s, mountinfo:\n%s", boot_path, mountinfo_contents); + retval = g_file_replace_contents (mountinfo_file, mountinfo_contents, + strlen (mountinfo_contents), NULL, FALSE, + G_FILE_CREATE_NONE, NULL, NULL, &error); + g_assert_no_error (error); + g_assert_true (retval); + retval = eos_updater_sysroot_boot_is_automount (fixture->sysroot, mountinfo_path); + g_assert_false (retval); + + /* Automount /boot without target mount */ + g_clear_pointer (&mountinfo_contents, g_free); + mountinfo_contents = + g_strdup_printf ("1 1 1:2 / %s rw - ext4 /dev/sda2 rw\n" + "2 1 0:1 / %s rw - autofs systemd-1 rw\n", + sysroot_path, boot_path); + g_test_message ("boot %s, mountinfo:\n%s", boot_path, mountinfo_contents); + retval = g_file_replace_contents (mountinfo_file, mountinfo_contents, + strlen (mountinfo_contents), NULL, FALSE, + G_FILE_CREATE_NONE, NULL, NULL, &error); + g_assert_no_error (error); + g_assert_true (retval); + retval = eos_updater_sysroot_boot_is_automount (fixture->sysroot, mountinfo_path); + g_assert_true (retval); + + /* Automount /boot with target mount */ + g_clear_pointer (&mountinfo_contents, g_free); + mountinfo_contents = + g_strdup_printf ("1 1 1:2 / %s rw - ext4 /dev/sda2 rw\n" + "2 1 0:1 / %s rw - autofs systemd-1 rw\n" + "3 2 1:1 / %s rw - vfat /dev/sda1 rw\n", + sysroot_path, boot_path, boot_path); + g_test_message ("boot %s, mountinfo:\n%s", boot_path, mountinfo_contents); + retval = g_file_replace_contents (mountinfo_file, mountinfo_contents, + strlen (mountinfo_contents), NULL, FALSE, + G_FILE_CREATE_NONE, NULL, NULL, &error); + g_assert_no_error (error); + g_assert_true (retval); + retval = eos_updater_sysroot_boot_is_automount (fixture->sysroot, mountinfo_path); + g_assert_true (retval); +} + int main (int argc, char *argv[]) @@ -157,6 +236,8 @@ main (int argc, g_test_add ("/ostree/no-deployments", Fixture, NULL, setup, test_ostree_no_deployments, teardown); + g_test_add ("/ostree/sysroot-boot-automount", Fixture, NULL, setup, + test_ostree_sysroot_boot_automount, teardown); /* TODO: More */ return g_test_run (); From a19821a2b419850e188da3eee4fe3465528ca920 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Thu, 3 Mar 2022 12:26:36 -0700 Subject: [PATCH 2/2] apply: Don't use staged deployments when /boot is automounted The ostree staged deployment process works by waiting until shutdown to swap the `/boot` symlinks to make the new deployment the default. However, when `/boot` is the EFI System Partition and there's no `fstab` entry, `systemd-gpt-auto-generator` sets up an automount so that the VFAT filesystem is only exposed when needed. Unfortunately, there are 2 bugs that make this process very fragile: * Once a systemd automount unit is scheduled to be stopped, it ignores notifications from autofs that the target filesystem should be mounted. Therefore, if `/boot` isn't mounted when shutdown begins, `ostree admin finalize-staged` will fail. See https://github.com/systemd/systemd/issues/22528. * autofs is not mount namespace aware, so it will begin the expiration timer for a mount unit unless a process in the root namespace is keeping it active. Since `ostree admin finalize-staged` is run from a mount namespace (either via systemd or its own to ensure `/sysroot` and `/boot` are mounted read-write), the automount daemon (systemd) will try to unmount the filesystem if it expires during this process. See https://bugzilla.redhat.com/show_bug.cgi?id=2056090. Therefore, if `/boot` is an autofs filesystem, use a full deployment instead of a staged deployment. Since systems with an automounted `/boot` are not common, we want to retain the benefit of staged deployments for more normal systems. See https://github.com/ostreedev/ostree/issues/2543 for potential future fixes in ostree. https://phabricator.endlessm.com/T33136 --- eos-updater/apply.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/eos-updater/apply.c b/eos-updater/apply.c index 374d76e4f..a327c9c07 100644 --- a/eos-updater/apply.c +++ b/eos-updater/apply.c @@ -270,11 +270,13 @@ apply_internal (ApplyData *apply_data, origin = ostree_sysroot_origin_new_from_refspec (sysroot, update_refspec); - /* When booted into an OSTree system, stage the deployment so that the - * /etc merge happens during shutdown. Otherwise (primarily the test - * suite), deploy the finalized tree immediately. + /* When booted into an OSTree system without an automount /boot, stage the + * deployment so that the /etc merge happens during shutdown. Otherwise + * (primarily sd-boot and the test suite), deploy the finalized tree + * immediately. */ - staged_deploy = ostree_sysroot_is_booted (sysroot); + staged_deploy = ostree_sysroot_is_booted (sysroot) && + !eos_updater_sysroot_boot_is_automount (sysroot, NULL); if (staged_deploy) { g_message ("Creating staged deployment for revision %s", update_id);