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: ostreedev#2154
p5 · Sep 24, 2020 · aa2a278 · aa2a278
1 parent 6950a98
commit aa2a278
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 31 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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"