From aa2a2783ea71d956db41f0f874d596752e47449f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 24 Sep 2020 19:28:31 +0000 Subject: [PATCH] deploy: Remove deployment bootcsum assertion When support for devicetree was added, it created a problem because old and new ostree versions would compute different checksums for the "boot data". The scenario here is: - Have system with ostree < 2020.4 - Reboot into system with ostree 2020.5 - Try to perform an operation that would retain that previous booted deployment (common) Currently ostree iterates over all the deployments that will be retained and calls `install_deployment_kernel()`, even for the booted one (which is a bit silly), but just to verify that all boot data for the targeted deployments are installed. This then re-computes the checksum and we'd trip this assertion. In practice though, we don't strictly require them to match; the only thing that will happen if they don't is that we'll end up with another copy of the kernel/initramfs - and that only temporarily until the previous deployment gets GC'd. Longer term, I think what we really want to do anyways is probably closer to like a little ostree repo for `/boot` so that we can e.g. still hardlink kernels there even if the initramfs changes, or hardlink both kernel/initramfs if just the devicetree changes, etc. Closes: https://github.com/ostreedev/ostree/issues/2154 --- Makefile-tests.am | 1 + src/libostree/ostree-sysroot-deploy.c | 67 ++++++++++++++------------ src/libostree/ostree-sysroot-private.h | 1 + src/libostree/ostree-sysroot.c | 1 + tests/test-osupdate-dtb.sh | 62 ++++++++++++++++++++++++ 5 files changed, 101 insertions(+), 31 deletions(-) create mode 100755 tests/test-osupdate-dtb.sh diff --git a/Makefile-tests.am b/Makefile-tests.am index a4179377..eb110dad 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -105,6 +105,7 @@ _installed_or_uninstalled_test_scripts = \ tests/test-admin-deploy-nomerge.sh \ tests/test-admin-deploy-none.sh \ tests/test-admin-deploy-bootid-gc.sh \ + tests/test-osupdate-dtb.sh \ tests/test-admin-instutil-set-kargs.sh \ tests/test-admin-upgrade-not-backwards.sh \ tests/test-admin-pull-deploy-commit.sh \ diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 3f337a50..d97d96a8 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1028,7 +1028,8 @@ _ostree_kernel_layout_new (void) /* See get_kernel_from_tree() below */ static gboolean -get_kernel_from_tree_usrlib_modules (int deployment_dfd, +get_kernel_from_tree_usrlib_modules (OstreeSysroot *sysroot, + int deployment_dfd, OstreeKernelLayout **out_layout, GCancellable *cancellable, GError **error) @@ -1137,37 +1138,41 @@ get_kernel_from_tree_usrlib_modules (int deployment_dfd, g_clear_object (&in); glnx_close_fd (&fd); - /* Check for /usr/lib/modules/$kver/devicetree first, if it does not - * exist check for /usr/lib/modules/$kver/dtb/ directory. - */ - if (!ot_openat_ignore_enoent (ret_layout->boot_dfd, "devicetree", &fd, error)) - return FALSE; - if (fd != -1) - { - ret_layout->devicetree_srcpath = g_strdup ("devicetree"); - ret_layout->devicetree_namever = g_strdup_printf ("devicetree-%s", kver); - in = g_unix_input_stream_new (fd, FALSE); - if (!ot_gio_splice_update_checksum (NULL, in, &checksum, cancellable, error)) - return FALSE; - } - else + /* Testing aid for https://github.com/ostreedev/ostree/issues/2154 */ + const gboolean no_dtb = (sysroot->debug_flags & OSTREE_SYSROOT_DEBUG_TEST_NO_DTB) > 0; + if (!no_dtb) { - struct stat stbuf; - /* Check for dtb directory */ - if (!glnx_fstatat_allow_noent (ret_layout->boot_dfd, "dtb", &stbuf, 0, error)) + /* Check for /usr/lib/modules/$kver/devicetree first, if it does not + * exist check for /usr/lib/modules/$kver/dtb/ directory. + */ + if (!ot_openat_ignore_enoent (ret_layout->boot_dfd, "devicetree", &fd, error)) return FALSE; - - if (errno == 0 && S_ISDIR (stbuf.st_mode)) + if (fd != -1) { - /* devicetree_namever set to NULL indicates a complete directory */ - ret_layout->devicetree_srcpath = g_strdup ("dtb"); - ret_layout->devicetree_namever = NULL; - - if (!checksum_dir_recurse(ret_layout->boot_dfd, "dtb", &checksum, cancellable, error)) + ret_layout->devicetree_srcpath = g_strdup ("devicetree"); + ret_layout->devicetree_namever = g_strdup_printf ("devicetree-%s", kver); + in = g_unix_input_stream_new (fd, FALSE); + if (!ot_gio_splice_update_checksum (NULL, in, &checksum, cancellable, error)) return FALSE; } - } + else + { + struct stat stbuf; + /* Check for dtb directory */ + if (!glnx_fstatat_allow_noent (ret_layout->boot_dfd, "dtb", &stbuf, 0, error)) + return FALSE; + if (errno == 0 && S_ISDIR (stbuf.st_mode)) + { + /* devicetree_namever set to NULL indicates a complete directory */ + ret_layout->devicetree_srcpath = g_strdup ("dtb"); + ret_layout->devicetree_namever = NULL; + + if (!checksum_dir_recurse(ret_layout->boot_dfd, "dtb", &checksum, cancellable, error)) + return FALSE; + } + } + } g_clear_object (&in); glnx_close_fd (&fd); @@ -1336,7 +1341,8 @@ get_kernel_from_tree_legacy_layouts (int deployment_dfd, * initramfs there, so we need to look in /usr/lib/ostree-boot first. */ static gboolean -get_kernel_from_tree (int deployment_dfd, +get_kernel_from_tree (OstreeSysroot *sysroot, + int deployment_dfd, OstreeKernelLayout **out_layout, GCancellable *cancellable, GError **error) @@ -1345,7 +1351,7 @@ get_kernel_from_tree (int deployment_dfd, g_autoptr(OstreeKernelLayout) legacy_layout = NULL; /* First, gather from usr/lib/modules/$kver if it exists */ - if (!get_kernel_from_tree_usrlib_modules (deployment_dfd, &usrlib_modules_layout, cancellable, error)) + if (!get_kernel_from_tree_usrlib_modules (sysroot, deployment_dfd, &usrlib_modules_layout, cancellable, error)) return FALSE; /* Gather the legacy layout */ @@ -1761,7 +1767,7 @@ install_deployment_kernel (OstreeSysroot *sysroot, /* Find the kernel/initramfs/devicetree in the tree */ g_autoptr(OstreeKernelLayout) kernel_layout = NULL; - if (!get_kernel_from_tree (deployment_dfd, &kernel_layout, + if (!get_kernel_from_tree (sysroot, deployment_dfd, &kernel_layout, cancellable, error)) return FALSE; @@ -1771,7 +1777,6 @@ install_deployment_kernel (OstreeSysroot *sysroot, const char *osname = ostree_deployment_get_osname (deployment); const char *bootcsum = ostree_deployment_get_bootcsum (deployment); - g_assert_cmpstr (kernel_layout->bootcsum, ==, bootcsum); g_autofree char *bootcsumdir = g_strdup_printf ("ostree/%s-%s", osname, bootcsum); g_autofree char *bootconfdir = g_strdup_printf ("loader.%d/entries", new_bootversion); g_autofree char *bootconf_name = g_strdup_printf ("ostree-%d-%s.conf", @@ -2711,7 +2716,7 @@ sysroot_initialize_deployment (OstreeSysroot *self, return FALSE; g_autoptr(OstreeKernelLayout) kernel_layout = NULL; - if (!get_kernel_from_tree (deployment_dfd, &kernel_layout, + if (!get_kernel_from_tree (self, deployment_dfd, &kernel_layout, cancellable, error)) return FALSE; diff --git a/src/libostree/ostree-sysroot-private.h b/src/libostree/ostree-sysroot-private.h index fa1e5336..1af2fd27 100644 --- a/src/libostree/ostree-sysroot-private.h +++ b/src/libostree/ostree-sysroot-private.h @@ -38,6 +38,7 @@ typedef enum { /* This is a temporary flag until we fully drop the explicit `systemctl start * ostree-finalize-staged.service` so that tests can exercise the new path unit. */ OSTREE_SYSROOT_DEBUG_TEST_STAGED_PATH = 1 << 3, + OSTREE_SYSROOT_DEBUG_TEST_NO_DTB = 1 << 4, /* https://github.com/ostreedev/ostree/issues/2154 */ } OstreeSysrootDebugFlags; typedef enum { diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index b211fea7..e412ea4d 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -190,6 +190,7 @@ ostree_sysroot_init (OstreeSysroot *self) { "test-fifreeze", OSTREE_SYSROOT_DEBUG_TEST_FIFREEZE }, { "no-xattrs", OSTREE_SYSROOT_DEBUG_NO_XATTRS }, { "test-staged-path", OSTREE_SYSROOT_DEBUG_TEST_STAGED_PATH }, + { "no-dtb", OSTREE_SYSROOT_DEBUG_TEST_NO_DTB }, }; self->debug_flags = g_parse_debug_string (g_getenv ("OSTREE_SYSROOT_DEBUG"), diff --git a/tests/test-osupdate-dtb.sh b/tests/test-osupdate-dtb.sh new file mode 100755 index 00000000..9e0c4686 --- /dev/null +++ b/tests/test-osupdate-dtb.sh @@ -0,0 +1,62 @@ +#!/bin/bash +# +# Copyright (C) 2020 Red Hat, Inc. +# +# SPDX-License-Identifier: LGPL-2.0+ +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the +# Free Software Foundation, Inc., 59 Temple Place - Suite 330, +# Boston, MA 02111-1307, USA. + +set -euo pipefail + +. $(dirname $0)/libtest.sh + +echo "1..1" + +# Exports OSTREE_SYSROOT so --sysroot not needed. +kver="3.6.0" +modulesdir="usr/lib/modules/${kver}" +setup_os_repository "archive" "syslinux" ${modulesdir} + +cd ${test_tmpdir} +os_repository_new_commit "test" "test with device tree directory" + +devicetree_path=osdata/${modulesdir}/dtb/asoc-board.dtb +devicetree_overlay_path=osdata/${modulesdir}/dtb/overlays/overlay.dtbo + +mkdir -p osdata/${modulesdir}/dtb +echo "a device tree" > ${devicetree_path} +mkdir -p osdata/${modulesdir}/dtb/overlays +echo "a device tree overlay" > ${devicetree_overlay_path} + +${CMD_PREFIX} ostree --repo=testos-repo commit --tree=dir=osdata/ -b testos/buildmaster/x86_64-runtime +${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmaster/x86_64-runtime +${CMD_PREFIX} ostree --repo=sysroot/ostree/repo remote add --set=gpg-verify=false testos file://$(pwd)/testos-repo testos/buildmaster/x86_64-runtime +${CMD_PREFIX} env OSTREE_SYSROOT_DEBUG=${OSTREE_SYSROOT_DEBUG},no-dtb ostree admin deploy --os=testos testos:testos/buildmaster/x86_64-runtime +assert_has_file sysroot/boot/ostree/testos-${bootcsum}/vmlinuz-3.6.0 +assert_not_has_file sysroot/boot/ostree/testos-${bootcsum}/dtb/asoc-board.dtb 'a device tree' +assert_streq $(ls sysroot/boot/ostree | wc -l) 1 +assert_streq $(find sysroot/boot/ostree -name '*.dtb' | wc -l) 0 +${CMD_PREFIX} ostree --repo=testos-repo commit --tree=dir=osdata/ -b testos/buildmaster/x86_64-runtime +env OSTREE_SYSROOT_DEBUG=${OSTREE_SYSROOT_DEBUG},no-dtb ${CMD_PREFIX} ostree admin upgrade --os=testos +${CMD_PREFIX} ostree --repo=testos-repo commit --tree=dir=osdata/ -b testos/buildmaster/x86_64-runtime +${CMD_PREFIX} ostree admin upgrade --os=testos +assert_streq $(ls sysroot/boot/ostree | wc -l) 2 +# Note that the bootcsum computed by the test suite doesn't include devicetree +# And currently we end up installing the dtb for the *previous* deployment +# too which is a bug - in the future this should be fixed to assert 1. +assert_streq $(find sysroot/boot/ostree -name '*.dtb' | wc -l) 2 + +echo "ok update with no dtb to dtb" -- 2.26.2