prepare-root: unmount temporary var mount after /var is mounted

src/libostree/ostree-impl-system-generator.c

@@ -126,6 +126,35 @@ require_internal_units (const char *normal_dir, const char *early_dir, const cha
 #endif
 }
 
+static gboolean
+atomic_write_unit (int dir_fd, const char *path, GCancellable *cancellable, GError **error, const char *fmt, ...)
+{
+  g_auto (GLnxTmpfile) tmpf = {
+    0,
+  };
+  if (!glnx_open_tmpfile_linkable_at (dir_fd, ".", O_WRONLY | O_CLOEXEC, &tmpf, error))
+    return FALSE;
+  g_autoptr (GOutputStream) outstream = g_unix_output_stream_new (tmpf.fd, FALSE);
+  gsize bytes_written;
+  va_list args;
+  va_start (args, fmt);
+  const gboolean r = g_output_stream_vprintf (outstream, &bytes_written, cancellable, error, fmt, args);
+  va_end (args);
+  if (!r)
+    return FALSE;
+  if (!g_output_stream_flush (outstream, cancellable, error))
+    return FALSE;
+  g_clear_object (&outstream);
+  /* It should be readable */
+  if (!glnx_fchmod (tmpf.fd, 0644, error))
+    return FALSE;
+  /* Error out if somehow it already exists, that'll help us debug conflicts */
+  if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_NOREPLACE, dir_fd, path,
+                             error))
+    return FALSE;
+  return TRUE;
+}
+
 /* Generate var.mount */
 static gboolean
 fstab_generator (const char *ostree_target, const bool is_aboot, const char *normal_dir,
@@ -135,8 +164,37 @@ fstab_generator (const char *ostree_target, const bool is_aboot, const char *nor
   /* Not currently cancellable, but define a var in case we care later */
   GCancellable *cancellable = NULL;
   /* Some path constants to avoid typos */
-  static const char fstab_path[] = "/etc/fstab";
-  static const char var_path[] = "/var";
+  const char *fstab_path = "/etc/fstab";
+  const char *var_dst = "/var";
+  const char *var_src = OTCORE_RUN_OSTREE_PRIVATE "/var";
+
+  /* Prepare to write to the output unit dir; we use the "normal" dir
+   * that overrides /usr, but not /etc.
+   */
+  glnx_autofd int normal_dir_dfd = -1;
+  if (!glnx_opendirat (AT_FDCWD, normal_dir, TRUE, &normal_dir_dfd, error))
+    return FALSE;
+
+  /* Generate a unit to unmount var_src */
+  if (!atomic_write_unit (normal_dir_dfd, "ostree-unmount-temp-var.service", cancellable, error,
+                          "##\n# Automatically generated by ostree-system-generator\n##\n\n"
+                          "[Unit]\n"
+                          "Documentation=man:ostree(1)\n"
+                          "ConditionPathIsMountPoint=%s\n"
+                          "After=var.mount\n"
+                          "\n"
+                          "[Service]\n"
+                          "Type=oneshot\n"
+                          "ExecStart=/usr/bin/umount --lazy %s\n",
+                          var_src, var_src))
+    return FALSE;
+
+  if (!glnx_shutil_mkdir_p_at (normal_dir_dfd, "local-fs.target.wants", 0755, cancellable,
+                               error))
+    return FALSE;
+  if (symlinkat ("../ostree-unmount-temp-var.service", normal_dir_dfd,
+             "local-fs.target.wants/ostree-unmount-temp-var.service") < 0)
+    return glnx_throw_errno_prefix (error, "symlinkat");
 
   /* Load /etc/fstab if it exists, and look for a /var mount */
   g_autoptr (OtLibMountFile) fstab = setmntent (fstab_path, "re");
@@ -157,7 +215,7 @@ fstab_generator (const char *ostree_target, const bool is_aboot, const char *nor
             path_kill_slashes (where);
 
           /* We're only looking for /var here */
-          if (strcmp (where, var_path) != 0)
+          if (strcmp (where, var_dst) != 0)
             continue;
 
           found_var_mnt = TRUE;
@@ -169,59 +227,19 @@ fstab_generator (const char *ostree_target, const bool is_aboot, const char *nor
   if (found_var_mnt)
     return TRUE;
 
-  /* Prepare to write to the output unit dir; we use the "normal" dir
-   * that overrides /usr, but not /etc.
-   */
-  glnx_autofd int normal_dir_dfd = -1;
-  if (!glnx_opendirat (AT_FDCWD, normal_dir, TRUE, &normal_dir_dfd, error))
-    return FALSE;
-
   /* Generate our bind mount unit */
-  const char *var_dir = OTCORE_RUN_OSTREE_PRIVATE "/var";
-
-  g_auto (GLnxTmpfile) tmpf = {
-    0,
-  };
-  if (!glnx_open_tmpfile_linkable_at (normal_dir_dfd, ".", O_WRONLY | O_CLOEXEC, &tmpf, error))
-    return FALSE;
-  g_autoptr (GOutputStream) outstream = g_unix_output_stream_new (tmpf.fd, FALSE);
-  gsize bytes_written;
-  /* This code is inspired by systemd's fstab-generator.c.
-   *
-   * Note that our unit doesn't run if systemd.volatile is enabled;
-   * see https://github.com/ostreedev/ostree/pull/856
-   *
-   * To avoid having submounts of /var propagate into $stateroot/var, the mount
-   * is made with slave+shared propagation. This means that /var will receive
-   * mount events from the parent /sysroot mount, but not vice versa. Adding a
-   * shared peer group below the slave group means that submounts of /var will
-   * inherit normal shared propagation. See mount_namespaces(7), Linux
-   * Documentation/filesystems/sharedsubtree.txt and
-   * https://github.com/ostreedev/ostree/issues/2086. This also happens in
-   * ostree-prepare-root.c for the INITRAMFS_MOUNT_VAR case.
-   */
-  if (!g_output_stream_printf (outstream, &bytes_written, cancellable, error,
-                               "##\n# Automatically generated by ostree-system-generator\n##\n\n"
-                               "[Unit]\n"
-                               "Documentation=man:ostree(1)\n"
-                               "ConditionKernelCommandLine=!systemd.volatile\n"
-                               "Before=local-fs.target\n"
-                               "\n"
-                               "[Mount]\n"
-                               "Where=%s\n"
-                               "What=%s\n"
-                               "Options=bind,slave,shared\n",
-                               var_path, var_dir))
-    return FALSE;
-  if (!g_output_stream_flush (outstream, cancellable, error))
-    return FALSE;
-  g_clear_object (&outstream);
-  /* It should be readable */
-  if (!glnx_fchmod (tmpf.fd, 0644, error))
-    return FALSE;
-  /* Error out if somehow it already exists, that'll help us debug conflicts */
-  if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_NOREPLACE, normal_dir_dfd, "var.mount",
-                             error))
+  if (!atomic_write_unit (normal_dir_dfd, "var.mount", cancellable, error,
+                          "##\n# Automatically generated by ostree-system-generator\n##\n\n"
+                          "[Unit]\n"
+                          "Documentation=man:ostree(1)\n"
+                          "ConditionKernelCommandLine=!systemd.volatile\n"
+                          "Before=local-fs.target\n"
+                          "\n"
+                          "[Mount]\n"
+                          "Where=%s\n"
+                          "What=%s\n"
+                          "Options=bind,slave,shared\n",
+                          var_dst, var_src))
     return FALSE;
 
   /* And ensure it's required; newer systemd will auto-inject fs dependencies

src/switchroot/ostree-prepare-root.c

@@ -693,7 +693,10 @@ main (int argc, char *argv[])
   if (mount_var)
     {
       if (mount (var_dir, TMP_SYSROOT "/var", NULL, MS_BIND | MS_SILENT, NULL) < 0)
-        err (EXIT_FAILURE, "failed to bind mount /var");
+        err (EXIT_FAILURE, "failed to bind mount %s to /var", var_dir);
+
+      if (umount2 (var_dir, MNT_DETACH) < 0)
+        err (EXIT_FAILURE, "failed to umount %s", var_dir);
 
       /* To avoid having submounts of /var propagate into $stateroot/var, the
        * mount is made with slave+shared propagation. See the comment in