lib/sysroot-deploy: Refactor kernel layout parsing

I'd like to move the new canonical kernel directory to `/usr/lib/modules/$kver`,
as Fedora has done. The `get_kernel_from_tree()` function now abstracts over
parsing the data (src vs destination filenames, as well as checksum) in
preparation for adding the new case.

In preparation for this, let's change the current test suite to use the
*current* directory of `/usr/lib/ostree-boot`, and also add coverage of `/boot`.

Closes: #1053
Approved by: jlebon

diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index 3207850586fb82d7da493a3b1025371152ed93f8..7e72a3ef03fb40b10a7b3fbef4be098c4b72ff84 100644 (file)
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -113,7 +113,7 @@ hardlink_or_copy_at (int         src_dfd,
                                   sysroot_flags_to_copy_flags (0, flags),
                                   cancellable, error);
       else
-        return glnx_throw_errno_prefix (error, "linkat");
+        return glnx_throw_errno_prefix (error, "linkat(%s)", dest_subpath);
     }
 
   return TRUE;
@@ -868,16 +868,29 @@ ostree_sysroot_write_origin_file (OstreeSysroot         *sysroot,
                                      cancellable, error);
 }
 
+/* Originally OSTree defined kernels to be found underneath /boot
+ * in the tree.  But that means when mounting /boot at runtime
+ * we end up masking the content underneath, triggering a warning.
+ *
+ * For that reason, and also consistency with the "/usr defines the OS" model we
+ * later switched to defining the in-tree kernels to be found under
+ * /usr/lib/ostree-boot.
+ */
 static gboolean
 get_kernel_from_tree (int             deployment_dfd,
                       int            *out_boot_dfd,
-                      char          **out_kernel_name,
-                      char          **out_initramfs_name,
+                      char          **out_kernel_srcpath,
+                      char          **out_kernel_namever,
+                      char          **out_initramfs_srcpath,
+                      char          **out_initramfs_namever,
+                      char          **out_bootcsum,
                       GCancellable   *cancellable,
                       GError        **error)
 {
-  g_autofree char *ret_kernel_name = NULL;
-  g_autofree char *ret_initramfs_name = NULL;
+  g_autofree char *ret_kernel_srcpath = NULL;
+  g_autofree char *ret_kernel_namever = NULL;
+  g_autofree char *ret_initramfs_srcpath = NULL;
+  g_autofree char *ret_initramfs_namever = NULL;
   g_autofree char *kernel_checksum = NULL;
   g_autofree char *initramfs_checksum = NULL;
 
@@ -907,67 +920,57 @@ get_kernel_from_tree (int             deployment_dfd,
       if (dent == NULL)
         break;
 
-      if (ret_kernel_name == NULL && g_str_has_prefix (dent->d_name, "vmlinuz-"))
+      const char *name = dent->d_name;
+      if (ret_kernel_srcpath == NULL && g_str_has_prefix (name, "vmlinuz-"))
         {
-          const char *dash = strrchr (dent->d_name, '-');
+          const char *dash = strrchr (name, '-');
           g_assert (dash);
           if (ostree_validate_structureof_checksum_string (dash + 1, NULL))
             {
               kernel_checksum = g_strdup (dash + 1);
-              ret_kernel_name = g_strdup (dent->d_name);
+              ret_kernel_srcpath = g_strdup (name);
+              ret_kernel_namever = g_strndup (name, dash - name);
             }
         }
-      else if (ret_initramfs_name == NULL && g_str_has_prefix (dent->d_name, "initramfs-"))
+      else if (ret_initramfs_srcpath == NULL && g_str_has_prefix (name, "initramfs-"))
         {
-          const char *dash = strrchr (dent->d_name, '-');
+          const char *dash = strrchr (name, '-');
           g_assert (dash);
           if (ostree_validate_structureof_checksum_string (dash + 1, NULL))
             {
               initramfs_checksum = g_strdup (dash + 1);
-              ret_initramfs_name = g_strdup (dent->d_name);
+              ret_initramfs_srcpath = g_strdup (name);
+              ret_initramfs_namever = g_strndup (name, dash - name);
             }
         }
 
-      if (ret_kernel_name != NULL && ret_initramfs_name != NULL)
+      if (ret_kernel_srcpath != NULL && ret_initramfs_srcpath != NULL)
         break;
     }
 
-  if (ret_kernel_name == NULL)
+  if (ret_kernel_srcpath == NULL)
     {
       g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
-                   "Failed to find boot/vmlinuz-<CHECKSUM> in tree");
+                   "Failed to find kernel in /usr/lib/ostree-boot or /boot");
       return FALSE;
     }
 
-  if (ret_initramfs_name != NULL)
+  if (ret_initramfs_srcpath != NULL)
     {
       if (strcmp (kernel_checksum, initramfs_checksum) != 0)
         {
           g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
-                       "Mismatched kernel checksum vs initrd in tree");
+                       "Mismatched kernel checksum vs initrd");
           return FALSE;
         }
     }
 
   *out_boot_dfd = glnx_steal_fd (&ret_boot_dfd);
-  *out_kernel_name = g_steal_pointer (&ret_kernel_name);
-  *out_initramfs_name = g_steal_pointer (&ret_initramfs_name);
-  return TRUE;
-}
-
-static gboolean
-checksum_from_kernel_src (const char   *name,
-                          char        **out_checksum,
-                          GError     **error)
-{
-  const char *last_dash = strrchr (name, '-');
-  if (!last_dash)
-    {
-      return glnx_throw (error,
-                        "Malformed kernel/initramfs name '%s', missing '-'",
-                        name);
-    }
-  *out_checksum = g_strdup (last_dash + 1);
+  *out_kernel_srcpath = g_steal_pointer (&ret_kernel_srcpath);
+  *out_kernel_namever = g_steal_pointer (&ret_kernel_namever);
+  *out_initramfs_srcpath = g_steal_pointer (&ret_initramfs_srcpath);
+  *out_initramfs_namever = g_steal_pointer (&ret_initramfs_namever);
+  *out_bootcsum = g_steal_pointer (&kernel_checksum);
   return TRUE;
 }
 
@@ -1231,15 +1234,6 @@ swap_bootlinks (OstreeSysroot *self,
   return TRUE;
 }
 
-static char *
-remove_checksum_from_kernel_name (const char *name,
-                                  const char *csum)
-{
-  const char *p = strrchr (name, '-');
-  g_assert_cmpstr (p+1, ==, csum);
-  return g_strndup (name, p-name);
-}
-
 static GHashTable *
 parse_os_release (const char *contents,
                   const char *split)
@@ -1293,11 +1287,19 @@ install_deployment_kernel (OstreeSysroot   *sysroot,
                        &deployment_dfd, error))
     return FALSE;
 
+  /* Find the kernel/initramfs in the tree */
   glnx_fd_close int tree_boot_dfd = -1;
-  g_autofree char *tree_kernel_name = NULL;
-  g_autofree char *tree_initramfs_name = NULL;
+  g_autofree char *tree_kernel_srcpath = NULL;
+  g_autofree char *tree_kernel_namever = NULL;
+  g_autofree char *tree_initramfs_srcpath = NULL;
+  g_autofree char *tree_initramfs_namever = NULL;
+  g_autofree char *tree_bootcsum = NULL;
   if (!get_kernel_from_tree (deployment_dfd, &tree_boot_dfd,
-                             &tree_kernel_name, &tree_initramfs_name,
+                             &tree_kernel_srcpath,
+                             &tree_kernel_namever,
+                             &tree_initramfs_srcpath,
+                             &tree_initramfs_namever,
+                             &tree_bootcsum,
                              cancellable, error))
     return FALSE;
 
@@ -1307,6 +1309,7 @@ install_deployment_kernel (OstreeSysroot   *sysroot,
 
   const char *osname = ostree_deployment_get_osname (deployment);
   const char *bootcsum = ostree_deployment_get_bootcsum (deployment);
+  g_assert_cmpstr (bootcsum, ==, tree_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-%s-%d.conf", osname,
@@ -1321,30 +1324,33 @@ install_deployment_kernel (OstreeSysroot   *sysroot,
   if (!glnx_shutil_mkdir_p_at (boot_dfd, bootconfdir, 0775, cancellable, error))
     return FALSE;
 
-  g_autofree char *dest_kernel_name = remove_checksum_from_kernel_name (tree_kernel_name, bootcsum);
+  /* Install (hardlink/copy) the kernel into /boot/ostree/osname-${bootcsum} if
+   * it doesn't exist already.
+   */
   struct stat stbuf;
-  if (fstatat (bootcsum_dfd, dest_kernel_name, &stbuf, 0) != 0)
+  if (fstatat (bootcsum_dfd, tree_kernel_namever, &stbuf, 0) != 0)
     {
       if (errno != ENOENT)
-        return glnx_throw_errno_prefix (error, "fstat %s", dest_kernel_name);
-      if (!hardlink_or_copy_at (tree_boot_dfd, tree_kernel_name,
-                                bootcsum_dfd, dest_kernel_name,
+        return glnx_throw_errno_prefix (error, "fstat %s", tree_kernel_namever);
+      if (!hardlink_or_copy_at (tree_boot_dfd, tree_kernel_srcpath,
+                                bootcsum_dfd, tree_kernel_namever,
                                 sysroot->debug_flags,
                                 cancellable, error))
         return FALSE;
     }
 
-  g_autofree char *dest_initramfs_name = NULL;
-  if (tree_initramfs_name)
+  /* If we have an initramfs, then install it into
+   * /boot/ostree/osname-${bootcsum} if it doesn't exist already.
+   */
+  if (tree_initramfs_srcpath)
     {
-      dest_initramfs_name = remove_checksum_from_kernel_name (tree_initramfs_name, bootcsum);
-
-      if (fstatat (bootcsum_dfd, dest_initramfs_name, &stbuf, 0) != 0)
+      g_assert (tree_initramfs_namever);
+      if (fstatat (bootcsum_dfd, tree_initramfs_namever, &stbuf, 0) != 0)
         {
           if (errno != ENOENT)
-            return glnx_throw_errno_prefix (error, "fstat %s", dest_initramfs_name);
-          if (!hardlink_or_copy_at (tree_boot_dfd, tree_initramfs_name,
-                                    bootcsum_dfd, dest_initramfs_name,
+            return glnx_throw_errno_prefix (error, "fstat %s", tree_initramfs_namever);
+          if (!hardlink_or_copy_at (tree_boot_dfd, tree_initramfs_srcpath,
+                                    bootcsum_dfd, tree_initramfs_namever,
                                     sysroot->debug_flags,
                                     cancellable, error))
             return FALSE;
@@ -1425,12 +1431,12 @@ install_deployment_kernel (OstreeSysroot   *sysroot,
 
   g_autofree char *version_key = g_strdup_printf ("%d", n_deployments - ostree_deployment_get_index (deployment));
   ostree_bootconfig_parser_set (bootconfig, OSTREE_COMMIT_META_KEY_VERSION, version_key);
-  g_autofree char * boot_relpath = g_strconcat ("/", bootcsumdir, "/", dest_kernel_name, NULL);
+  g_autofree char * boot_relpath = g_strconcat ("/", bootcsumdir, "/", tree_kernel_namever, NULL);
   ostree_bootconfig_parser_set (bootconfig, "linux", boot_relpath);
 
-  if (dest_initramfs_name)
+  if (tree_initramfs_namever)
     {
-      g_autofree char * boot_relpath = g_strconcat ("/", bootcsumdir, "/", dest_initramfs_name, NULL);
+      g_autofree char * boot_relpath = g_strconcat ("/", bootcsumdir, "/", tree_initramfs_namever, NULL);
       ostree_bootconfig_parser_set (bootconfig, "initrd", boot_relpath);
     }
 
@@ -2053,24 +2059,20 @@ ostree_sysroot_deploy_tree (OstreeSysroot     *self,
     }
 
   glnx_fd_close int tree_boot_dfd = -1;
-  g_autofree char *tree_kernel_path = NULL;
-  g_autofree char *tree_initramfs_path = NULL;
+  g_autofree char *tree_kernel_srcpath = NULL;
+  g_autofree char *tree_kernel_namever = NULL;
+  g_autofree char *tree_initramfs_srcpath = NULL;
+  g_autofree char *tree_initramfs_namever = NULL;
+  g_autofree char *tree_bootcsum = NULL;
   if (!get_kernel_from_tree (deployment_dfd, &tree_boot_dfd,
-                             &tree_kernel_path, &tree_initramfs_path,
+                             &tree_kernel_srcpath,
+                             &tree_kernel_namever,
+                             &tree_initramfs_srcpath,
+                             &tree_initramfs_namever,
+                             &new_bootcsum,
                              cancellable, error))
     return FALSE;
 
-  if (tree_initramfs_path != NULL)
-    {
-      if (!checksum_from_kernel_src (tree_initramfs_path, &new_bootcsum, error))
-        return FALSE;
-    }
-  else
-    {
-      if (!checksum_from_kernel_src (tree_kernel_path, &new_bootcsum, error))
-        return FALSE;
-    }
-
   _ostree_deployment_set_bootcsum (new_deployment, new_bootcsum);
 
   /* Create an empty boot configuration; we will merge things into

diff --git a/tests/libtest.sh b/tests/libtest.sh
index 6f33c5b54c4571e5901b98676b330d4e268237c4..ff096505328dccc8837a1b69154f2fc70c297f5b 100755 (executable)
--- a/tests/libtest.sh
+++ b/tests/libtest.sh
@@ -363,8 +363,10 @@ setup_os_boot_grub2() {
 
 setup_os_repository () {
     mode=$1
-    bootmode=$2
     shift
+    bootmode=$1
+    shift
+    bootdir=${1:-usr/lib/ostree-boot}
 
     oldpwd=`pwd`
 
@@ -379,14 +381,19 @@ setup_os_repository () {
     cd ${test_tmpdir}
     mkdir osdata
     cd osdata
-    mkdir -p boot usr/bin usr/lib/modules/3.6.0 usr/share usr/etc
-    echo "a kernel" > boot/vmlinuz-3.6.0
-    echo "an initramfs" > boot/initramfs-3.6.0
-    bootcsum=$(cat boot/vmlinuz-3.6.0 boot/initramfs-3.6.0 | sha256sum | cut -f 1 -d ' ')
+    mkdir -p usr/bin usr/lib/modules/3.6.0 usr/share usr/etc
+    mkdir -p ${bootdir}
+    echo "a kernel" > ${bootdir}/vmlinuz-3.6.0
+    echo "an initramfs" > ${bootdir}/initramfs-3.6.0
+    bootcsum=$(cat ${bootdir}/vmlinuz-3.6.0 ${bootdir}/initramfs-3.6.0 | sha256sum | cut -f 1 -d ' ')
     export bootcsum
-    mv boot/vmlinuz-3.6.0 boot/vmlinuz-3.6.0-${bootcsum}
-    mv boot/initramfs-3.6.0 boot/initramfs-3.6.0-${bootcsum}
-    
+    # Add the checksum for legacy dirs (/boot, /usr/lib/ostree-boot), but not
+    # /usr/lib/modules.
+    if [[ $bootdir != usr/lib/modules ]]; then
+        mv ${bootdir}/vmlinuz-3.6.0{,-${bootcsum}}
+        mv ${bootdir}/initramfs-3.6.0{,-${bootcsum}}
+    fi
+
     echo "an executable" > usr/bin/sh
     echo "some shared data" > usr/share/langs.txt
     echo "a library" > usr/lib/libfoo.so.0
@@ -458,13 +465,17 @@ os_repository_new_commit ()
     branch=${3:-testos/buildmaster/x86_64-runtime}
     echo "BOOT ITERATION: $boot_checksum_iteration"
     cd ${test_tmpdir}/osdata
-    rm boot/*
-    echo "new: a kernel ${boot_checksum_iteration}" > boot/vmlinuz-3.6.0
-    echo "new: an initramfs ${boot_checksum_iteration}" > boot/initramfs-3.6.0
-    bootcsum=$(cat boot/vmlinuz-3.6.0 boot/initramfs-3.6.0 | sha256sum | cut -f 1 -d ' ')
+    bootdir=usr/lib/ostree-boot
+    if ! test -d ${bootdir}; then
+        bootdir=boot
+    fi
+    rm ${bootdir}/*
+    echo "new: a kernel ${boot_checksum_iteration}" > ${bootdir}/vmlinuz-3.6.0
+    echo "new: an initramfs ${boot_checksum_iteration}" > ${bootdir}/initramfs-3.6.0
+    bootcsum=$(cat ${bootdir}/vmlinuz-3.6.0 ${bootdir}/initramfs-3.6.0 | sha256sum | cut -f 1 -d ' ')
     export bootcsum
-    mv boot/vmlinuz-3.6.0 boot/vmlinuz-3.6.0-${bootcsum}
-    mv boot/initramfs-3.6.0 boot/initramfs-3.6.0-${bootcsum}
+    mv ${bootdir}/vmlinuz-3.6.0 ${bootdir}/vmlinuz-3.6.0-${bootcsum}
+    mv ${bootdir}/initramfs-3.6.0 ${bootdir}/initramfs-3.6.0-${bootcsum}
 
     echo "a new default config file" > usr/etc/a-new-default-config-file
     mkdir -p usr/etc/new-default-dir

diff --git a/tests/test-admin-deploy-syslinux.sh b/tests/test-admin-deploy-syslinux.sh
index e03e211bccddfa0149b7f537e3c0aee186c2bea9..b19a74f07dbe85947ac9edae846ffc98702a84ea 100755 (executable)
--- a/tests/test-admin-deploy-syslinux.sh
+++ b/tests/test-admin-deploy-syslinux.sh
@@ -19,7 +19,7 @@
 
 set -euo pipefail
 
-echo "1..19"
+echo "1..20"
 
 . $(dirname $0)/libtest.sh
 
@@ -27,3 +27,23 @@ echo "1..19"
 setup_os_repository "archive-z2" "syslinux"
 
 . $(dirname $0)/admin-test.sh
+
+cd ${test_tmpdir}
+rm httpd osdata testos-repo sysroot -rf
+setup_os_repository "archive-z2" "syslinux" "boot"
+
+${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmaster/x86_64-runtime
+rev=$(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime)
+${CMD_PREFIX} ostree admin deploy --karg=root=LABEL=MOO --karg=quiet --os=testos testos:testos/buildmaster/x86_64-runtime
+assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.* root=LABEL=MOO'
+assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'options.* quiet'
+assert_file_has_content sysroot/boot/ostree/testos-${bootcsum}/vmlinuz-3.6.0 'a kernel'
+assert_file_has_content sysroot/boot/ostree/testos-${bootcsum}/initramfs-3.6.0 'an initramfs'
+# kernel/initrams should also be in the tree's /boot with the checksum
+assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.0/boot/vmlinuz-3.6.0-${bootcsum} 'a kernel'
+assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.0/boot/initramfs-3.6.0-${bootcsum} 'an initramfs'
+assert_file_has_content sysroot/ostree/deploy/testos/deploy/${rev}.0/etc/os-release 'NAME=TestOS'
+assert_file_has_content sysroot/ostree/boot.1/testos/${bootcsum}/0/etc/os-release 'NAME=TestOS'
+${CMD_PREFIX} ostree admin status
+validate_bootloader
+echo "ok kernel in tree's /boot"

diff --git a/tests/test-admin-deploy-uboot.sh b/tests/test-admin-deploy-uboot.sh
index 5262b48ac5da0ad91fc3e649859d1d9a657d5855..3685e31ef21181eb3bd7e7bec5f9c27f0413caa9 100755 (executable)
--- a/tests/test-admin-deploy-uboot.sh
+++ b/tests/test-admin-deploy-uboot.sh
@@ -30,8 +30,7 @@ setup_os_repository "archive-z2" "uboot"
 . $(dirname $0)/admin-test.sh
 
 cd ${test_tmpdir}
-ln -s ../../boot/ osdata/usr/lib/ostree-boot
-cat << 'EOF' > osdata/boot/uEnv.txt
+cat << 'EOF' > osdata/usr/lib/ostree-boot/uEnv.txt
 loaduimage=load mmc ${bootpart} ${loadaddr} ${kernel_image}
 loadfdt=load mmc ${bootpart} ${fdtaddr} ${bootdir}${fdtfile}
 loadramdisk=load mmc ${bootpart} ${rdaddr} ${ramdisk_image}