From 947acbf178152f860d3d8f9896b2ce51645419f6 Mon Sep 17 00:00:00 2001
From: Jonathan Lebon <jonathan@jlebon.com>
Date: Fri, 19 Nov 2021 10:44:03 -0500
Subject: [PATCH] app: Only remount /sysroot if needed

We should only try to remount `/sysroot` if we're actually handling the
sysroot repo and the repo isn't writable. We already have public APIs to
check each of those, so let's use them.

Closes: #2485
---
 src/ostree/ot-main.c | 58 +++++++++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c
index bbaba8b01d..849a74ef74 100644
--- a/src/ostree/ot-main.c
+++ b/src/ostree/ot-main.c
@@ -238,7 +238,7 @@ ostree_run (int    argc,
   return 0;
 }
 
-/* Process a --repo arg; used below, and for the remote builtins */
+/* Process a --repo arg. */
 static OstreeRepo *
 parse_repo_option (GOptionContext *context,
                    const char     *repo_path,
@@ -248,19 +248,6 @@ parse_repo_option (GOptionContext *context,
 {
   g_autoptr(OstreeRepo) repo = NULL;
 
-  /* This is a bit of a brutal hack; we set up a mount
-   * namespace if it appears that we may need it.  It'd
-   * be better to do this more precisely in the future.
-   */
-  gboolean setup_ns = FALSE;
-  if (!maybe_setup_mount_namespace (&setup_ns, error))
-    return FALSE;
-  if (setup_ns)
-    {
-      if (mount ("/sysroot", "/sysroot", NULL, MS_REMOUNT | MS_SILENT, NULL) < 0)
-        return glnx_null_throw_errno_prefix (error, "Remounting /sysroot read-write");
-    }
-
   if (repo_path == NULL)
     {
       g_autoptr(GError) local_error = NULL;
@@ -300,6 +287,43 @@ parse_repo_option (GOptionContext *context,
   return g_steal_pointer (&repo);
 }
 
+/* Process a --repo arg, determining if we should remount /sysroot; used below, and for the remote builtins */
+static OstreeRepo *
+parse_repo_option_and_maybe_remount (GOptionContext *context,
+                                     const char     *repo_path,
+                                     gboolean        skip_repo_open,
+                                     GCancellable   *cancellable,
+                                     GError        **error)
+{
+  g_autoptr(OstreeRepo) repo = parse_repo_option (context, repo_path, skip_repo_open, cancellable, error);
+  if (!repo)
+    return NULL;
+
+  /* This is a bit of a brutal hack; we set up a mount
+   * namespace if it appears that we may need it.  It'd
+   * be better to do this more precisely in the future.
+   */
+  if (ostree_repo_is_system (repo) && !ostree_repo_is_writable (repo, NULL))
+    {
+      gboolean setup_ns = FALSE;
+      if (!maybe_setup_mount_namespace (&setup_ns, error))
+        return FALSE;
+      if (setup_ns)
+        {
+          if (mount ("/sysroot", "/sysroot", NULL, MS_REMOUNT | MS_SILENT, NULL) < 0)
+            return glnx_null_throw_errno_prefix (error, "Remounting /sysroot read-write");
+
+          /* Reload the repo so it's actually writable. */
+          g_clear_pointer (&repo, g_object_unref);
+          repo = parse_repo_option (context, repo_path, skip_repo_open, cancellable, error);
+          if (!repo)
+            return NULL;
+        }
+    }
+
+  return g_steal_pointer (&repo);
+}
+
 /* Used by the remote builtins which are special in taking --sysroot or --repo */
 gboolean
 ostree_parse_sysroot_or_repo_option (GOptionContext *context,
@@ -323,7 +347,7 @@ ostree_parse_sysroot_or_repo_option (GOptionContext *context,
     }
   else
     {
-      repo = parse_repo_option (context, repo_path, FALSE, cancellable, error);
+      repo = parse_repo_option_and_maybe_remount (context, repo_path, FALSE, cancellable, error);
       if (!repo)
         return FALSE;
     }
@@ -424,8 +448,8 @@ ostree_option_context_parse (GOptionContext *context,
 
   if (!(flags & OSTREE_BUILTIN_FLAG_NO_REPO))
     {
-      repo = parse_repo_option (context, opt_repo, (flags & OSTREE_BUILTIN_FLAG_NO_CHECK) > 0,
-                                cancellable, error);
+      repo = parse_repo_option_and_maybe_remount (context, opt_repo, (flags & OSTREE_BUILTIN_FLAG_NO_CHECK) > 0,
+                                                  cancellable, error);
       if (!repo)
         return FALSE;
     }