sysroot: Handle ro /boot but rw /sysroot

The recent change in coreos/fedora-coreos-config#659
broke some of our tests that do `mount -o remount,rw /sysroot` but
leave `/boot` read-only.

We had code for having `/boot` read-only before `/sysroot` but
in practice we had a file descriptor for `/sysroot` that we opened
before the remount that would happen later on.

Clean things up here so that in the library, we also remount
`/boot` at the same time we remount `/sysroot` if either are readonly.

Second, adapt the client side code to check for either being
readonly to enable the mount namespace.  Now honestly we could
almost certainly just set this unconditionally - the original
client code here is just being excessively conservative.  But
I'd like to make that cleanup independently of this fix.

src/libostree/ostree-sysroot-deploy.c

@@ -2583,7 +2583,7 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot     *self,
       if (self->booted_deployment)
         boot_was_ro_mount = is_ro_mount ("/boot");
 
-      g_debug ("boot is ro: %s", boot_was_ro_mount ? "yes" : "no");
+      g_debug ("boot-ro:%s mountns:%s", ot_booltostr (boot_was_ro_mount), ot_booltostr (self->mount_namespace_in_use));
 
       if (boot_was_ro_mount)
         {

src/libostree/ostree-sysroot.c

@@ -281,6 +281,30 @@ ensure_sysroot_fd (OstreeSysroot          *self,
   return TRUE;
 }
 
+static gboolean
+remount_writable (const char *path, gboolean *did_remount, GError **error)
+{
+  *did_remount = FALSE;
+  struct statvfs stvfsbuf;
+  if (statvfs (path, &stvfsbuf) < 0)
+    {
+      if (errno != ENOENT)
+        return glnx_throw_errno_prefix (error, "statvfs(%s)", path);
+      else
+        return TRUE;
+    }
+
+  if ((stvfsbuf.f_flag & ST_RDONLY) != 0)
+    {
+      /* OK, let's remount writable. */
+      if (mount (path, path, NULL, MS_REMOUNT | MS_RELATIME, "") < 0)
+        return glnx_throw_errno_prefix (error, "Remounting %s read-write", path);
+      *did_remount = TRUE;
+    }
+
+  return TRUE;
+}
+
 /* Remount /sysroot read-write if necessary */
 gboolean
 _ostree_sysroot_ensure_writable (OstreeSysroot      *self,
@@ -300,21 +324,27 @@ _ostree_sysroot_ensure_writable (OstreeSysroot      *self,
   if (!self->root_is_ostree_booted)
     return TRUE;
 
-  /* Check if /sysroot is a read-only mountpoint */
-  struct statvfs stvfsbuf;
-  if (statvfs ("/sysroot", &stvfsbuf) < 0)
-    return glnx_throw_errno_prefix (error, "fstatvfs(/sysroot)");
-  if ((stvfsbuf.f_flag & ST_RDONLY) == 0)
-    return TRUE;
-
-  /* OK, let's remount writable. */
-  if (mount ("/sysroot", "/sysroot", NULL, MS_REMOUNT | MS_RELATIME, "") < 0)
-    return glnx_throw_errno_prefix (error, "Remounting /sysroot read-write");
+  gboolean did_remount_sysroot = FALSE;
+  /* Handle /boot */
+  if (!remount_writable ("/boot", &did_remount_sysroot, error))
+    return FALSE;
+  else if (did_remount_sysroot)
+    {
+      g_debug ("remounted /boot writable");
+    }
 
-  /* Reopen our fd */
-  glnx_close_fd (&self->sysroot_fd);
-  if (!ensure_sysroot_fd (self, error))
+  /* And /sysroot; also reopen our fd if it changed */
+  gboolean did_remount_boot = FALSE;
+  if (!remount_writable ("/sysroot", &did_remount_boot, error))
     return FALSE;
+  else if (did_remount_boot || did_remount_sysroot)
+    {
+      /* Reopen our fd */
+      glnx_close_fd (&self->sysroot_fd);
+      if (!ensure_sysroot_fd (self, error))
+        return FALSE;
+      g_debug ("remounted /sysroot writable");
+    }
 
   return TRUE;
 }

src/libotutil/otutil.h

@@ -34,6 +34,12 @@
 #define OT_VARIANT_BUILDER_INITIALIZER {{{0,}}}
 #endif
 
+static inline const char *
+ot_booltostr (int b)
+{
+  return b ? "true" : "false";
+}
+
 #define ot_gobject_refz(o) (o ? g_object_ref (o) : o)
 
 #define ot_transfer_out_value(outp, srcp) G_STMT_START {   \

src/ostree/ot-main.c

@@ -122,23 +122,32 @@ maybe_setup_mount_namespace (gboolean    *out_ns,
   if (errno == ENOENT)
     return TRUE;
 
-  glnx_autofd int sysroot_subdir_fd = glnx_opendirat_with_errno (AT_FDCWD, "/sysroot", TRUE);
-  if (sysroot_subdir_fd < 0)
+  gboolean have_ro_mount = FALSE;
+  static const char *ropaths[] = {"/sysroot", "/boot"};
+  for (guint i = 0; i < G_N_ELEMENTS (ropaths); i++)
     {
-      if (errno != ENOENT)
-        return glnx_throw_errno_prefix (error, "opendirat");
-      /* No /sysroot - nothing to do */
-      return TRUE;
+      const char *path = ropaths[i];
+      struct statvfs stvfs;
+      if (statvfs (path, &stvfs) < 0)
+        {
+          if (errno != ENOENT)
+            return glnx_throw_errno_prefix (error, "statvfs(%s)", path);
+          else
+            continue;
+        }
+      if (stvfs.f_flag & ST_RDONLY)
+        {
+          have_ro_mount = TRUE;
+          break;
+        }
     }
 
-  struct statvfs stvfs;
-  if (fstatvfs (sysroot_subdir_fd, &stvfs) < 0)
-    return glnx_throw_errno_prefix (error, "fstatvfs");
-  if (stvfs.f_flag & ST_RDONLY)
+  if (have_ro_mount)
     {
       if (unshare (CLONE_NEWNS) < 0)
         return glnx_throw_errno_prefix (error, "preparing writable sysroot: unshare (CLONE_NEWNS)");
 
+      g_debug ("setup mount namespace");
       *out_ns = TRUE;
     }