From 3b9304b5d76968c92378b708a76239fa5f08dcf9 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Fri, 5 Jan 2018 16:02:58 -0500
Subject: [PATCH] rofiles: Fix --copyup when creating a new file

This tripped up the `docbook-dtds` `%post` in my experiments
with doing rpm-ostree for buildroots.

I cloned and built [xfstests](https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git)
but haven't yet investigated actually running it.

In the meantime let's do the obvious fix here; we need to distinguish
between "copyup enabled" and "actually did a copyup" in the open path
at least, since if we didn't do a copyup we don't need to re-open.

Closes: #1396
Approved by: jlebon
---
 src/rofiles-fuse/main.c    | 15 ++++++++---
 tests/test-rofiles-fuse.sh | 51 ++++++++++++++++++++++++++++++++------
 2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/src/rofiles-fuse/main.c b/src/rofiles-fuse/main.c
index 2e1c1346..77c2f3d7 100644
--- a/src/rofiles-fuse/main.c
+++ b/src/rofiles-fuse/main.c
@@ -247,10 +247,14 @@ gioerror_to_errno (GIOErrorEnum e)
 }
 
 static int
-verify_write_or_copyup (const char *path, const struct stat *stbuf)
+verify_write_or_copyup (const char *path, const struct stat *stbuf,
+                        gboolean *out_did_copyup)
 {
   struct stat stbuf_local;
 
+  if (out_did_copyup)
+    *out_did_copyup = FALSE;
+
   /* If a stbuf wasn't provided, gather it now */
   if (!stbuf)
     {
@@ -272,6 +276,8 @@ verify_write_or_copyup (const char *path, const struct stat *stbuf)
           g_autoptr(GError) tmp_error = NULL;
           if (!ostree_break_hardlink (basefd, path, FALSE, NULL, &tmp_error))
             return -gioerror_to_errno ((GIOErrorEnum)tmp_error->code);
+          if (out_did_copyup)
+            *out_did_copyup = TRUE;
         }
       else
         return -EROFS;
@@ -286,7 +292,7 @@ verify_write_or_copyup (const char *path, const struct stat *stbuf)
  */
 #define PATH_WRITE_ENTRYPOINT(path) do {                     \
     path = ENSURE_RELPATH (path);                            \
-    int r = verify_write_or_copyup (path, NULL);             \
+    int r = verify_write_or_copyup (path, NULL, NULL);       \
     if (r != 0)                                              \
       return r;                                              \
   } while (0)
@@ -374,7 +380,8 @@ do_open (const char *path, mode_t mode, struct fuse_file_info *finfo)
           return -errno;
         }
 
-      int r = verify_write_or_copyup (path, &stbuf);
+      gboolean did_copyup;
+      int r = verify_write_or_copyup (path, &stbuf, &did_copyup);
       if (r != 0)
         {
           (void) close (fd);
@@ -382,7 +389,7 @@ do_open (const char *path, mode_t mode, struct fuse_file_info *finfo)
         }
 
       /* In the copyup case, we need to re-open */
-      if (opt_copyup)
+      if (did_copyup)
         {
           (void) close (fd);
           /* Note that unlike the initial open, we will pass through
diff --git a/tests/test-rofiles-fuse.sh b/tests/test-rofiles-fuse.sh
index 805a7188..1c91a5cd 100755
--- a/tests/test-rofiles-fuse.sh
+++ b/tests/test-rofiles-fuse.sh
@@ -121,19 +121,54 @@ echo "ok flock"
 
 # And now with --copyup enabled
 
-fusermount -u ${test_tmpdir}/mnt
-assert_not_has_file mnt/firstfile
-rofiles-fuse --copyup checkout-test2 mnt
+copyup_reset() {
+    cd ${test_tmpdir}
+    fusermount -u mnt
+    rm checkout-test2 -rf
+    $OSTREE checkout -H test2 checkout-test2
+    rofiles-fuse --copyup checkout-test2 mnt
+}
+
+assert_test_file() {
+    t=$1
+    f=$2
+    if ! test ${t} "${f}"; then
+        ls -al "${f}"
+        fatal "Failed test ${t} ${f}"
+    fi
+}
+
+copyup_reset
 assert_file_has_content mnt/firstfile first
 echo "ok copyup mount"
 
+# Test O_TRUNC directly
 firstfile_orig_inode=$(stat -c %i checkout-test2/firstfile)
-for path in firstfile{,-link}; do
-    echo truncating > mnt/${path}
-    assert_file_has_content mnt/${path} truncating
-    assert_not_file_has_content mnt/${path} first
-done
+echo -n truncating > mnt/firstfile
+assert_streq "$(cat mnt/firstfile)" truncating
 firstfile_new_inode=$(stat -c %i checkout-test2/firstfile)
 assert_not_streq "${firstfile_orig_inode}" "${firstfile_new_inode}"
+assert_test_file -f checkout-test2/firstfile
+
+copyup_reset
+firstfile_link_orig_inode=$(stat -c %i checkout-test2/firstfile-link)
+firstfile_orig_inode=$(stat -c %i checkout-test2/firstfile)
+# Now write via the symlink
+echo -n truncating > mnt/firstfile-link
+assert_streq "$(cat mnt/firstfile)" truncating
+firstfile_new_inode=$(stat -c %i checkout-test2/firstfile)
+firstfile_link_new_inode=$(stat -c %i checkout-test2/firstfile-link)
+assert_not_streq "${firstfile_orig_inode}" "${firstfile_new_inode}"
+assert_streq "${firstfile_link_orig_inode}" "${firstfile_link_new_inode}"
+assert_test_file -f checkout-test2/firstfile
+# Verify we didn't replace the link with a regfile somehow
+assert_test_file -L checkout-test2/firstfile-link
+
+# These both end up creating new files; in the sed case we'll then do a rename()
+copyup_reset
+echo "hello new file" > mnt/a-new-non-copyup-file
+assert_file_has_content_literal mnt/a-new-non-copyup-file "hello new file"
+sed -i -e s,first,second, mnt/firstfile
+assert_file_has_content_literal mnt/firstfile "second"
 
 echo "ok copyup"