sysroot: Support /boot on / for syslinux and uboot

People making smaller operating systems, particularly things like
cloud images, don't need to have /boot as a separate partition.

Build on the code that recently landed to detect whether /boot
is a mountpoint at runtime (for read-only purposes) here in order
to influence bootloader config generation.

Basically if /boot is not a partition, prepend `/boot` to generated
syslinux/uboot config.

NOTE: *not* tested in a real world scenario for either syslinux or
uboot, but I did extend the test suite to cover this.

https://bugzilla.gnome.org/show_bug.cgi?id=751666

Makefile-tests.am

@@ -54,6 +54,7 @@ test_scripts = \
 	tests/test-gpg-signed-commit.sh \
 	tests/test-admin-upgrade-unconfigured.sh \
 	tests/test-admin-deploy-syslinux.sh \
+	tests/test-admin-deploy-syslinux-bootonslash.sh \
 	tests/test-admin-deploy-2.sh \
 	tests/test-admin-deploy-karg.sh \
 	tests/test-admin-deploy-switch.sh \

src/libostree/ostree-bootloader-grub2.c

@@ -285,6 +285,7 @@ grub2_child_setup (gpointer user_data)
 static gboolean
 _ostree_bootloader_grub2_write_config (OstreeBootloader      *bootloader,
                                        int                    bootversion,
+                                       gboolean               have_boot_partition,
                                        GCancellable          *cancellable,
                                        GError               **error)
 {

src/libostree/ostree-bootloader-syslinux.c

@@ -60,11 +60,12 @@ _ostree_bootloader_syslinux_get_name (OstreeBootloader *bootloader)
 
 static gboolean
 append_config_from_boostree_loader_entries (OstreeBootloaderSyslinux  *self,
-                                        gboolean               regenerate_default,
-                                        int                    bootversion,
-                                        GPtrArray             *new_lines,
-                                        GCancellable          *cancellable,
-                                        GError               **error)
+                                            gboolean                   regenerate_default,
+                                            int                        bootversion,
+                                            gboolean                   have_boot_partition,
+                                            GPtrArray                 *new_lines,
+                                            GCancellable              *cancellable,
+                                            GError                   **error)
 {
   gboolean ret = FALSE;
   g_autoptr(GPtrArray) boostree_loader_configs = NULL;
@@ -78,6 +79,7 @@ append_config_from_boostree_loader_entries (OstreeBootloaderSyslinux  *self,
     {
       OstreeBootconfigParser *config = boostree_loader_configs->pdata[i];
       const char *val;
+      const char *filename_prefix = have_boot_partition ? "" : "/boot";
 
       val = ostree_bootconfig_parser_get (config, "title");
       if (!val)
@@ -97,11 +99,11 @@ append_config_from_boostree_loader_entries (OstreeBootloaderSyslinux  *self,
                        "No \"linux\" key in bootloader config");
           goto out;
         }
-      g_ptr_array_add (new_lines, g_strdup_printf ("\tKERNEL %s", val));
+      g_ptr_array_add (new_lines, g_strdup_printf ("\tKERNEL %s%s", filename_prefix, val));
 
       val = ostree_bootconfig_parser_get (config, "initrd");
       if (val)
-        g_ptr_array_add (new_lines, g_strdup_printf ("\tINITRD %s", val));
+        g_ptr_array_add (new_lines, g_strdup_printf ("\tINITRD %s%s", filename_prefix, val));
 
       val = ostree_bootconfig_parser_get (config, "options");
       if (val)
@@ -115,9 +117,10 @@ append_config_from_boostree_loader_entries (OstreeBootloaderSyslinux  *self,
 
 static gboolean
 _ostree_bootloader_syslinux_write_config (OstreeBootloader          *bootloader,
-                                     int                    bootversion,
-                                     GCancellable          *cancellable,
-                                     GError               **error)
+                                          int                    bootversion,
+                                          gboolean               have_boot_partition,
+                                          GCancellable          *cancellable,
+                                          GError               **error)
 {
   gboolean ret = FALSE;
   OstreeBootloaderSyslinux *self = OSTREE_BOOTLOADER_SYSLINUX (bootloader);
@@ -155,6 +158,7 @@ _ostree_bootloader_syslinux_write_config (OstreeBootloader          *bootloader,
     {
       char *line = *iter;
       gboolean skip = FALSE;
+      const char *nonostree_prefix = have_boot_partition ? "/ostree/" : "/boot/ostree/";
 
       if (parsing_label && 
           (line == NULL || !g_str_has_prefix (line, "\t")))
@@ -170,7 +174,7 @@ _ostree_bootloader_syslinux_write_config (OstreeBootloader          *bootloader,
           /* If this is a non-ostree kernel, just emit the lines
            * we saw.
            */
-          if (!g_str_has_prefix (kernel_arg, "/ostree/"))
+          if (!g_str_has_prefix (kernel_arg, nonostree_prefix))
             {
               for (i = 0; i < tmp_lines->len; i++)
                 {
@@ -240,8 +244,9 @@ _ostree_bootloader_syslinux_write_config (OstreeBootloader          *bootloader,
     regenerate_default = TRUE;
 
   if (!append_config_from_boostree_loader_entries (self, regenerate_default,
-                                               bootversion, new_lines,
-                                               cancellable, error))
+                                                   bootversion, have_boot_partition,
+                                                   new_lines,
+                                                   cancellable, error))
     goto out;
 
   new_config_contents = _ostree_sysroot_join_lines (new_lines);

src/libostree/ostree-bootloader-uboot.c

@@ -64,14 +64,16 @@ _ostree_bootloader_uboot_get_name (OstreeBootloader *bootloader)
 
 static gboolean
 create_config_from_boot_loader_entries (OstreeBootloaderUboot     *self,
-                                        int                    bootversion,
-                                        GPtrArray             *new_lines,
-                                        GCancellable          *cancellable,
-                                        GError               **error)
+                                        int                        bootversion,
+                                        gboolean                   have_boot_partition,
+                                        GPtrArray                 *new_lines,
+                                        GCancellable              *cancellable,
+                                        GError                   **error)
 {
   g_autoptr(GPtrArray) boot_loader_configs = NULL;
   OstreeBootconfigParser *config;
   const char *val;
+  const char *filename_prefix = have_boot_partition ? "" : "/boot";
 
   if (!_ostree_sysroot_read_boot_loader_configs (self->sysroot, bootversion, &boot_loader_configs,
                                                  cancellable, error))
@@ -87,11 +89,11 @@ create_config_from_boot_loader_entries (OstreeBootloaderUboot     *self,
                    "No \"linux\" key in bootloader config");
       return FALSE;
     }
-  g_ptr_array_add (new_lines, g_strdup_printf ("kernel_image=%s", val));
+  g_ptr_array_add (new_lines, g_strdup_printf ("kernel_image=%s%s", filename_prefix, val));
 
   val = ostree_bootconfig_parser_get (config, "initrd");
   if (val)
-    g_ptr_array_add (new_lines, g_strdup_printf ("ramdisk_image=%s", val));
+    g_ptr_array_add (new_lines, g_strdup_printf ("ramdisk_image=%s%s", filename_prefix, val));
 
   val = ostree_bootconfig_parser_get (config, "options");
   if (val)
@@ -102,9 +104,10 @@ create_config_from_boot_loader_entries (OstreeBootloaderUboot     *self,
 
 static gboolean
 _ostree_bootloader_uboot_write_config (OstreeBootloader          *bootloader,
-                                  int                    bootversion,
-                                  GCancellable          *cancellable,
-                                  GError               **error)
+                                       int                        bootversion,
+                                       gboolean                   have_boot_partition,
+                                       GCancellable              *cancellable,
+                                       GError                   **error)
 {
   OstreeBootloaderUboot *self = OSTREE_BOOTLOADER_UBOOT (bootloader);
   g_autoptr(GFile) new_config_path = NULL;
@@ -122,7 +125,9 @@ _ostree_bootloader_uboot_write_config (OstreeBootloader          *bootloader,
 
   new_lines = g_ptr_array_new_with_free_func (g_free);
 
-  if (!create_config_from_boot_loader_entries (self, bootversion, new_lines,
+  if (!create_config_from_boot_loader_entries (self, bootversion,
+                                               have_boot_partition,
+                                               new_lines,
                                                cancellable, error))
     return FALSE;
 

src/libostree/ostree-bootloader.c

@@ -54,14 +54,16 @@ _ostree_bootloader_get_name (OstreeBootloader  *self)
 
 gboolean
 _ostree_bootloader_write_config (OstreeBootloader  *self,
-                            int            bootversion,
-                            GCancellable  *cancellable,
-                            GError       **error)
+                                 int            bootversion,
+                                 gboolean have_boot_partition,
+                                 GCancellable  *cancellable,
+                                 GError       **error)
 {
   g_return_val_if_fail (OSTREE_IS_BOOTLOADER (self), FALSE);
 
   return OSTREE_BOOTLOADER_GET_IFACE (self)->write_config (self, bootversion, 
-                                                       cancellable, error);
+                                                           have_boot_partition,
+                                                           cancellable, error);
 }
 
 gboolean

src/libostree/ostree-bootloader.h

@@ -44,6 +44,7 @@ struct _OstreeBootloaderInterface
   const char *         (* get_name)               (OstreeBootloader  *self);
   gboolean             (* write_config)           (OstreeBootloader  *self,
                                                    int            bootversion,
+                                                   gboolean       have_boot_partition,
                                                    GCancellable  *cancellable,
                                                    GError       **error);
   gboolean             (* is_atomic)              (OstreeBootloader  *self);
@@ -60,6 +61,7 @@ const char *_ostree_bootloader_get_name (OstreeBootloader  *self);
 
 gboolean _ostree_bootloader_write_config (OstreeBootloader  *self,
                                           int            bootversion,
+                                          gboolean       have_boot_partition,
                                           GCancellable  *cancellable,
                                           GError       **error);
 

src/libostree/ostree-sysroot-deploy.c

@@ -1656,7 +1656,7 @@ cleanup_legacy_current_symlinks (OstreeSysroot         *self,
 }
 
 static gboolean
-is_ro_mount (const char *path)
+is_mount (const char *path)
 {
 #ifdef HAVE_LIBMOUNT
   /* Dragging in all of this crud is apparently necessary just to determine
@@ -1669,7 +1669,6 @@ is_ro_mount (const char *path)
   struct libmnt_fs *fs;
   struct libmnt_cache *cache;
   gboolean is_mount = FALSE;
-  struct statvfs stvfsbuf;
 
   if (!tb)
     return FALSE;
@@ -1683,17 +1682,29 @@ is_ro_mount (const char *path)
   mnt_free_cache (cache);
   mnt_free_table (tb);
 
-  if (!is_mount)
-    return FALSE;
+  return is_mount;
+#else
+  return FALSE;
+#endif
+}
+
+static gboolean
+query_mount_is_ro (const char *path,
+                   gboolean *out_is_ro,
+                   GError **error)
+{
+  struct statvfs stvfsbuf;
 
   /* We *could* parse the options, but it seems more reliable to
    * introspect the actual mount at runtime.
    */
-  if (statvfs (path, &stvfsbuf) == 0)
-    return (stvfsbuf.f_flag & ST_RDONLY) != 0;
-
-#endif
-  return FALSE;
+  if (statvfs (path, &stvfsbuf) < 0)
+    {
+      glnx_set_error_from_errno (error);
+      return FALSE;
+    }
+  *out_is_ro = (stvfsbuf.f_flag & ST_RDONLY) != 0;
+  return TRUE;
 }
 
 /**
@@ -1804,11 +1815,29 @@ ostree_sysroot_write_deployments (OstreeSysroot     *self,
       g_autoptr(GFile) new_loader_entries_dir = NULL;
       glnx_unref_object OstreeRepo *repo = NULL;
       gboolean show_osname = FALSE;
+      gboolean boot_is_mount = FALSE;
 
       if (self->booted_deployment)
-        boot_was_ro_mount = is_ro_mount ("/boot");
+        boot_is_mount = is_mount ("/boot");
+      else
+        {
+          /* For legacy reasons; see
+           * https://bugzilla.gnome.org/show_bug.cgi?id=751666 But the
+           * test suite can override this.  Do NOT use this debug flag
+           * in production, instead we could consider a compile-time
+           * default if you don't want a dependency on libmount or
+           * something.
+           */
+          boot_is_mount = (self->debug_flags & OSTREE_SYSROOT_DEBUG_BOOT_IS_NOT_MOUNT) == 0;
+        }
+
+      if (self->booted_deployment && boot_is_mount)
+        {
+          if (!query_mount_is_ro ("/boot", &boot_was_ro_mount, error))
+            goto out;
+        }
 
-      g_debug ("boot is ro: %s", boot_was_ro_mount ? "yes" : "no");
+      g_debug ("boot is mount: %d ro: %d", boot_is_mount, boot_was_ro_mount);
 
       if (boot_was_ro_mount)
         {
@@ -1887,6 +1916,7 @@ ostree_sysroot_write_deployments (OstreeSysroot     *self,
       if (bootloader)
         {
           if (!_ostree_bootloader_write_config (bootloader, new_bootversion,
+                                                boot_is_mount,
                                                 cancellable, error))
             {
               g_prefix_error (error, "Bootloader write config: ");

src/libostree/ostree-sysroot-private.h

@@ -30,7 +30,9 @@ G_BEGIN_DECLS
 typedef enum {
 
   /* Don't flag deployments as immutable. */
-  OSTREE_SYSROOT_DEBUG_MUTABLE_DEPLOYMENTS = 1 << 0
+  OSTREE_SYSROOT_DEBUG_MUTABLE_DEPLOYMENTS = 1 << 0,
+  /* If set, assume /boot is on / */
+  OSTREE_SYSROOT_DEBUG_BOOT_IS_NOT_MOUNT = 1 << 1
 
 } OstreeSysrootDebugFlags;
 

src/libostree/ostree-sysroot.c

@@ -156,9 +156,10 @@ ostree_sysroot_init (OstreeSysroot *self)
 {
   const GDebugKey keys[] = {
     { "mutable-deployments", OSTREE_SYSROOT_DEBUG_MUTABLE_DEPLOYMENTS },
+    { "boot-is-not-mount", OSTREE_SYSROOT_DEBUG_BOOT_IS_NOT_MOUNT }
   };
 
-  self->debug_flags = g_parse_debug_string (g_getenv("OSTREE_SYSROOT_DEBUG"),
+  self->debug_flags = g_parse_debug_string (g_getenv ("OSTREE_SYSROOT_DEBUG"),
                                             keys, G_N_ELEMENTS (keys));
 
   self->sysroot_fd = -1;

tests/test-admin-deploy-syslinux-bootonslash.sh

@@ -0,0 +1,46 @@
+#!/bin/bash
+#
+# Copyright (C) 2016 Colin Walters <[email protected]>
+#
+# 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
+
+# Exports OSTREE_SYSROOT so --sysroot not needed.
+setup_os_repository "archive-z2" "syslinux"
+
+echo "1..2"
+
+${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmaster/x86_64-runtime
+# Override the default and say that /boot is not a partition
+env OSTREE_SYSROOT_DEBUG=${OSTREE_SYSROOT_DEBUG},boot-is-not-mount \
+  ${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/syslinux/syslinux.cfg "KERNEL /boot/ostree/testos.*vmlinuz"
+assert_file_has_content sysroot/boot/syslinux/syslinux.cfg "INITRD /boot/ostree/testos.*initramfs"
+
+echo "ok bootonslash"
+
+${CMD_PREFIX} ostree admin undeploy 0
+
+# Legacy default of /boot is a partition
+${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/syslinux/syslinux.cfg "KERNEL /ostree/testos.*vmlinuz"
+assert_file_has_content sysroot/boot/syslinux/syslinux.cfg "INITRD /ostree/testos.*initramfs"
+
+echo "ok bootpartition"