fixup! sysroot: Support /boot on / for syslinux and uboot

src/libostree/ostree-bootloader-grub2.c

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

src/libostree/ostree-bootloader-syslinux.c

@@ -65,7 +65,7 @@ static gboolean
 append_config_from_loader_entries (OstreeBootloaderSyslinux  *self,
                                    gboolean               regenerate_default,
                                    int                    bootversion,
-                                   gboolean               have_boot_partition,
+                                   const char            *boot_path_on_disk,
                                    GPtrArray             *new_lines,
                                    GCancellable          *cancellable,
                                    GError               **error)
@@ -79,8 +79,6 @@ append_config_from_loader_entries (OstreeBootloaderSyslinux  *self,
     {
       OstreeBootconfigParser *config = loader_configs->pdata[i];
       const char *val = ostree_bootconfig_parser_get (config, "title");
-      const char *filename_prefix = have_boot_partition ? "" : "/boot";
-
       if (!val)
         val = "(Untitled)";
 
@@ -92,15 +90,15 @@ append_config_from_loader_entries (OstreeBootloaderSyslinux  *self,
       val = ostree_bootconfig_parser_get (config, "linux");
       if (!val)
         return glnx_throw (error, "No \"linux\" key in bootloader config");
-      g_ptr_array_add (new_lines, g_strdup_printf ("\tKERNEL %s%s", filename_prefix, val));
+      g_ptr_array_add (new_lines, g_strdup_printf ("\tKERNEL %s%s", boot_path_on_disk, val));
 
       val = ostree_bootconfig_parser_get (config, "initrd");
       if (val)
-        g_ptr_array_add (new_lines, g_strdup_printf ("\tINITRD %s%s", filename_prefix, val));
+        g_ptr_array_add (new_lines, g_strdup_printf ("\tINITRD %s%s", boot_path_on_disk, val));
 
       val = ostree_bootconfig_parser_get (config, "devicetree");
       if (val)
-        g_ptr_array_add (new_lines, g_strdup_printf ("\tDEVICETREE %s", val));
+        g_ptr_array_add (new_lines, g_strdup_printf ("\tDEVICETREE %s%s", boot_path_on_disk, val));
 
       val = ostree_bootconfig_parser_get (config, "options");
       if (val)
@@ -113,7 +111,7 @@ append_config_from_loader_entries (OstreeBootloaderSyslinux  *self,
 static gboolean
 _ostree_bootloader_syslinux_write_config (OstreeBootloader          *bootloader,
                                           int                    bootversion,
-                                          gboolean               have_boot_partition,
+                                          const char            *boot_path_on_disk,
                                           GCancellable          *cancellable,
                                           GError               **error)
 {
@@ -145,7 +143,7 @@ _ostree_bootloader_syslinux_write_config (OstreeBootloader          *bootloader,
     {
       const char *line = *iter;
       gboolean skip = FALSE;
-      const char *nonostree_prefix = have_boot_partition ? "/ostree/" : "/boot/ostree/";
+      g_autofree char *nonostree_prefix = g_build_path ("/", boot_path_on_disk, "/ostree/", NULL);
 
       if (parsing_label &&
           (line == NULL || !g_str_has_prefix (line, "\t")))
@@ -214,7 +212,7 @@ _ostree_bootloader_syslinux_write_config (OstreeBootloader          *bootloader,
     regenerate_default = TRUE;
 
   if (!append_config_from_loader_entries (self, regenerate_default,
-                                          bootversion, have_boot_partition,
+                                          bootversion, boot_path_on_disk,
                                           new_lines,
                                           cancellable, error))
     return FALSE;

src/libostree/ostree-bootloader-uboot.c

@@ -106,15 +106,14 @@ append_system_uenv (OstreeBootloaderUboot   *self,
 static gboolean
 create_config_from_boot_loader_entries (OstreeBootloaderUboot     *self,
                                         int                        bootversion,
-                                        gboolean                   have_boot_partition,
+                                        const char                *boot_path_on_disk,
                                         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))
@@ -136,11 +135,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=%s%s", index_suffix, filename_prefix, val));
+      g_ptr_array_add (new_lines, g_strdup_printf ("kernel_image%s=%s%s", index_suffix, boot_path_on_disk, val));
 
       val = ostree_bootconfig_parser_get (config, "initrd");
       if (val)
-        g_ptr_array_add (new_lines, g_strdup_printf ("ramdisk_image%s=%s%s", index_suffix, filename_prefix, val));
+        g_ptr_array_add (new_lines, g_strdup_printf ("ramdisk_image%s=%s%s", index_suffix, boot_path_on_disk, val));
 
       val = ostree_bootconfig_parser_get (config, "options");
       if (val)
@@ -158,7 +157,7 @@ create_config_from_boot_loader_entries (OstreeBootloaderUboot     *self,
 static gboolean
 _ostree_bootloader_uboot_write_config (OstreeBootloader          *bootloader,
                                        int                        bootversion,
-                                       gboolean                   have_boot_partition,
+                                       const char                *boot_path_on_disk,
                                        GCancellable              *cancellable,
                                        GError                   **error)
 {
@@ -172,7 +171,7 @@ _ostree_bootloader_uboot_write_config (OstreeBootloader          *bootloader,
     return FALSE;
 
   g_autoptr(GPtrArray) new_lines = g_ptr_array_new_with_free_func (g_free);
-  if (!create_config_from_boot_loader_entries (self, bootversion, have_boot_partition, new_lines,
+  if (!create_config_from_boot_loader_entries (self, bootversion, boot_path_on_disk, new_lines,
                                                cancellable, error))
     return FALSE;
 

src/libostree/ostree-bootloader.c

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

src/libostree/ostree-bootloader.h

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

src/libostree/ostree-sysroot-deploy.c

@@ -1970,7 +1970,7 @@ cleanup_legacy_current_symlinks (OstreeSysroot         *self,
  * /sysroot.
  */
 static gboolean
-is_mount (const char *path)
+is_ro_mount (const char *path)
 {
 #ifdef HAVE_LIBMOUNT
   /* Dragging in all of this crud is apparently necessary just to determine
@@ -1983,6 +1983,7 @@ is_mount (const char *path)
   struct libmnt_fs *fs;
   struct libmnt_cache *cache;
   gboolean is_mount = FALSE;
+  struct statvfs stvfsbuf;
 
   if (!tb)
     return FALSE;
@@ -2001,27 +2002,17 @@ is_mount (const char *path)
   mnt_free_cache (cache);
 #endif
 
-  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;
+  if (!is_mount)
+    return FALSE;
 
   /* We *could* parse the options, but it seems more reliable to
    * introspect the actual mount at runtime.
    */
-  if (statvfs (path, &stvfsbuf) < 0)
-    return glnx_throw_errno_prefix (error, "statvfs");
+  if (statvfs (path, &stvfsbuf) == 0)
+    return (stvfsbuf.f_flag & ST_RDONLY) != 0;
 
-  *out_is_ro = (stvfsbuf.f_flag & ST_RDONLY) != 0;
-  return TRUE;
+#endif
+  return FALSE;
 }
 
 /**
@@ -2045,6 +2036,53 @@ ostree_sysroot_write_deployments (OstreeSysroot     *self,
                                                         cancellable, error);
 }
 
+#ifdef HAVE_LIBMOUNT
+typedef struct libmnt_table libmnt_table;
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (libmnt_table, mnt_free_table);
+typedef struct libmnt_fs libmnt_fs;
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (libmnt_fs, mnt_unref_fs);
+#endif /* HAVE_LIBMOUNT */
+
+/* Finds the path to /boot on the filesystem that /boot is actually stored on.
+ * If you have a dedicated boot partition this will probably be "".  If it's
+ * stored on the root partition it will be "/boot".
+ *
+ * The path_out won't end with a "/".  That means that "/" is represented at ""
+ */
+static gboolean
+boot_find_path_on_disk(char** path_out, GError **error)
+{
+#ifdef HAVE_LIBMOUNT
+  g_autoptr(libmnt_table) tb = mnt_new_table();
+  g_assert (tb);
+  if (mnt_table_parse_mtab(tb, getenv("OSTREE_DEBUG_MOUNTINFO_FILE")) < 0)
+    return glnx_throw (error, "Failed to parse /proc/self/mountinfo");
+
+  const char *suffix = NULL;
+  struct libmnt_fs *fs = mnt_table_find_target(tb, "/boot", MNT_ITER_BACKWARD);
+  if (fs)
+    suffix = "";
+  else
+    {
+      suffix = "boot";
+      fs = mnt_table_find_target(tb, "/", MNT_ITER_BACKWARD);
+      if (!fs)
+        return glnx_throw (error, "libmount error: Can't find /");
+    }
+  g_autofree char *path = g_build_path ("/", mnt_fs_get_root (fs), suffix, NULL);
+  if (strcmp(path, "/") == 0)
+    {
+      g_free (g_steal_pointer(&path));
+      path = g_strdup("");
+    }
+
+  *path_out = g_steal_pointer(&path);
+  return TRUE;
+#else /* !HAVE_LIBMOUNT */
+  return g_strdup("");
+#endif /* HAVE_LIBMOUNT */
+}
+
 /**
  * ostree_sysroot_write_deployments_with_options:
  * @self: Sysroot
@@ -2160,29 +2198,11 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot     *self,
       g_autofree char* new_loader_entries_dir = NULL;
       g_autoptr(OstreeRepo) repo = NULL;
       gboolean show_osname = FALSE;
-      gboolean boot_is_mount = FALSE;
 
       if (self->booted_deployment)
-        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;
-        }
+        boot_was_ro_mount = is_ro_mount ("/boot");
 
-      if (self->booted_deployment && boot_is_mount)
-        {
-          if (!query_mount_is_ro ("/boot", &boot_was_ro_mount, error))
-            goto out;
-        }
-
-      g_debug ("boot is mount: %d ro: %d", boot_is_mount, boot_was_ro_mount);
+      g_debug ("boot is ro: %s", boot_was_ro_mount ? "yes" : "no");
 
       if (boot_was_ro_mount)
         {
@@ -2260,8 +2280,11 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot     *self,
 
       if (bootloader)
         {
+          g_autofree char *boot_path_on_disk = NULL;
+          if (!boot_find_path_on_disk (&boot_path_on_disk, error))
+            goto out;
           if (!_ostree_bootloader_write_config (bootloader, new_bootversion,
-                                                boot_is_mount,
+                                                boot_path_on_disk,
                                                 cancellable, error))
             {
               g_prefix_error (error, "Bootloader write config: ");

src/libostree/ostree-sysroot-private.h

@@ -34,8 +34,6 @@ typedef enum {
   OSTREE_SYSROOT_DEBUG_NO_XATTRS = 1 << 1,
   /* https://github.com/ostreedev/ostree/pull/1049 */
   OSTREE_SYSROOT_DEBUG_TEST_FIFREEZE = 1 << 2,
-  /* If set, assume /boot is on / */
-  OSTREE_SYSROOT_DEBUG_BOOT_IS_NOT_MOUNT = 1 << 3
 } OstreeSysrootDebugFlags;
 
 /**

src/libostree/ostree-sysroot.c

@@ -192,7 +192,6 @@ ostree_sysroot_init (OstreeSysroot *self)
     { "mutable-deployments", OSTREE_SYSROOT_DEBUG_MUTABLE_DEPLOYMENTS },
     { "test-fifreeze", OSTREE_SYSROOT_DEBUG_TEST_FIFREEZE },
     { "no-xattrs", OSTREE_SYSROOT_DEBUG_NO_XATTRS },
-    { "boot-is-not-mount", OSTREE_SYSROOT_DEBUG_BOOT_IS_NOT_MOUNT }
   };
 
   self->debug_flags = g_parse_debug_string (g_getenv ("OSTREE_SYSROOT_DEBUG"),

tests/admin-test.sh

@@ -42,6 +42,8 @@ assert_ostree_deployment_refs() {
     diff -u ostree-refs{-expected,}.txt
 }
 
+export OSTREE_DEBUG_MOUNTINFO_FILE=${test_srcdir}/boot_partition.mountinfo
+
 orig_mtime=$(stat -c '%.Y' sysroot/ostree/deploy)
 ${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)

tests/boot_on_root_bind_mount.mountinfo

@@ -0,0 +1,28 @@
+13 14 179:1 / /sysroot rw,relatime shared:5 - ext4 /dev/root rw,data=ordered
+14 1 179:1 /ostree/deploy/linux/deploy/61892bc01f9afdef900d7a7904fe934ac7fc8caf9befddc1faf0f6b4f179bf35.0 / rw,relatime shared:1 - ext4 /dev/root rw,data=ordered
+15 14 179:1 /ostree/deploy/linux/var /var rw,relatime shared:2 - ext4 /dev/root rw,data=ordered
+16 14 179:1 /boot /boot rw,relatime shared:3 - ext4 /dev/root rw,data=ordered
+17 14 179:1 /ostree/deploy/linux/deploy/61892bc01f9afdef900d7a7904fe934ac7fc8caf9befddc1faf0f6b4f179bf35.0/usr /usr ro,relatime shared:4 - ext4 /dev/root rw,data=ordered
+18 14 0:12 / /sys rw,nosuid,nodev,noexec,relatime shared:6 - sysfs sysfs rw
+19 14 0:3 / /proc rw,nosuid,nodev,noexec,relatime shared:10 - proc proc rw
+20 14 0:5 / /dev rw,nosuid shared:11 - devtmpfs devtmpfs rw,size=958480k,nr_inodes=181514,mode=755
+21 20 0:13 / /dev/shm rw,nosuid,nodev shared:12 - tmpfs tmpfs rw
+22 20 0:10 / /dev/pts rw,nosuid,noexec,relatime shared:13 - devpts devpts rw,gid=5,mode=620,ptmxmode=000
+23 14 0:14 / /run rw,nosuid,nodev shared:14 - tmpfs tmpfs rw,mode=755
+24 23 0:15 / /run/lock rw,nosuid,nodev,noexec,relatime shared:15 - tmpfs tmpfs rw,size=5120k
+25 18 0:16 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:7 - tmpfs tmpfs ro,mode=755
+26 25 0:17 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime shared:8 - cgroup cgroup rw,xattr,release_agent=/lib/systemd/systemd-cgroups-agent,name=systemd
+27 18 0:18 / /sys/fs/pstore rw,nosuid,nodev,noexec,relatime shared:9 - pstore pstore rw
+28 25 0:19 / /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime shared:16 - cgroup cgroup rw,freezer
+29 25 0:20 / /sys/fs/cgroup/debug rw,nosuid,nodev,noexec,relatime shared:17 - cgroup cgroup rw,debug
+30 25 0:21 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime shared:18 - cgroup cgroup rw,cpuset
+31 25 0:22 / /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime shared:19 - cgroup cgroup rw,devices
+32 25 0:23 / /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime shared:20 - cgroup cgroup rw,memory
+33 25 0:24 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:21 - cgroup cgroup rw,cpuacct,cpu
+34 25 0:25 / /sys/fs/cgroup/perf_event rw,nosuid,nodev,noexec,relatime shared:22 - cgroup cgroup rw,perf_event
+35 19 0:26 / /proc/sys/fs/binfmt_misc rw,relatime shared:23 - autofs systemd-1 rw,fd=24,pgrp=1,timeout=0,minproto=5,maxproto=5,direct
+36 20 0:11 / /dev/mqueue rw,relatime shared:24 - mqueue mqueue rw
+37 14 0:27 / /tmp rw,nosuid,nodev shared:25 - tmpfs tmpfs rw
+38 18 0:6 / /sys/kernel/debug rw,relatime shared:26 - debugfs debugfs rw
+39 18 0:28 / /sys/fs/fuse/connections rw,relatime shared:27 - fusectl fusectl rw
+