Commit 1229e1c4d7ea1eb4c5bcd5c0d89bc576053175db
2017-02-17T16:36:53
fsync parent directories when fsyncing When fsync'ing files, fsync the parent directory in the case where we rename a file into place, or create a new file, to ensure that the directory entry is flushed correctly.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142
diff --git a/src/filebuf.c b/src/filebuf.c
index c4a13ff..f37cf02 100644
--- a/src/filebuf.c
+++ b/src/filebuf.c
@@ -445,6 +445,9 @@ int git_filebuf_commit(git_filebuf *file)
goto on_error;
}
+ if (file->do_fsync && git_futils_fsync_parent(file->path_original) < 0)
+ goto on_error;
+
file->did_rename = true;
git_filebuf_cleanup(file);
diff --git a/src/fileops.c b/src/fileops.c
index 13b1fc2..881d1be 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -265,8 +265,13 @@ int git_futils_writebuffer(
return error;
}
- if ((error = p_close(fd)) < 0)
+ if ((error = p_close(fd)) < 0) {
giterr_set(GITERR_OS, "error while closing '%s'", path);
+ return error;
+ }
+
+ if (do_fsync && (flags & O_CREAT))
+ error = git_futils_fsync_parent(path);
return error;
}
@@ -1119,3 +1124,28 @@ void git_futils_filestamp_set_from_stat(
memset(stamp, 0, sizeof(*stamp));
}
}
+
+int git_futils_fsync_dir(const char *path)
+{
+ int fd, error = -1;
+
+ if ((fd = p_open(path, O_RDONLY)) < 0) {
+ giterr_set(GITERR_OS, "failed to open directory '%s' for fsync", path);
+ return -1;
+ }
+
+ if ((error = p_fsync(fd)) < 0)
+ giterr_set(GITERR_OS, "failed to fsync directory '%s'", path);
+
+ p_close(fd);
+ return error;
+}
+
+int git_futils_fsync_parent(const char *path)
+{
+ char *parent = git_path_dirname(path);
+ int error = git_futils_fsync_dir(parent);
+
+ git__free(parent);
+ return error;
+}
diff --git a/src/fileops.h b/src/fileops.h
index 9a6fc3f..46886b0 100644
--- a/src/fileops.h
+++ b/src/fileops.h
@@ -363,4 +363,22 @@ extern void git_futils_filestamp_set(
extern void git_futils_filestamp_set_from_stat(
git_futils_filestamp *stamp, struct stat *st);
+/**
+ * `fsync` the parent directory of the given path, if `fsync` is
+ * supported for directories on this platform.
+ *
+ * @param path Path of the directory to sync.
+ * @return 0 on success, -1 on error
+ */
+extern int git_futils_fsync_dir(const char *path);
+
+/**
+ * `fsync` the parent directory of the given path, if `fsync` is
+ * supported for directories on this platform.
+ *
+ * @param path Path of the file whose parent directory should be synced.
+ * @return 0 on success, -1 on error
+ */
+extern int git_futils_fsync_parent(const char *path);
+
#endif /* INCLUDE_fileops_h__ */
diff --git a/src/indexer.c b/src/indexer.c
index 30c3655..9510253 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -1086,7 +1086,14 @@ int git_indexer_commit(git_indexer *idx, git_transfer_progress *stats)
goto on_error;
/* And don't forget to rename the packfile to its new place. */
- p_rename(idx->pack->pack_name, git_buf_cstr(&filename));
+ if (p_rename(idx->pack->pack_name, git_buf_cstr(&filename)) < 0)
+ goto on_error;
+
+ /* And fsync the parent directory if we're asked to. */
+ if (git_object__synchronized_writing &&
+ git_futils_fsync_parent(git_buf_cstr(&filename)) < 0)
+ goto on_error;
+
idx->pack_committed = 1;
git_buf_free(&filename);
diff --git a/tests/pack/packbuilder.c b/tests/pack/packbuilder.c
index 9afc178..77f61f7 100644
--- a/tests/pack/packbuilder.c
+++ b/tests/pack/packbuilder.c
@@ -204,7 +204,7 @@ void test_pack_packbuilder__fsync_when_asked(void)
p_fsync__cnt = 0;
seed_packbuilder();
git_packbuilder_write(_packbuilder, ".", 0666, NULL, NULL);
- cl_assert_equal_sz(2, p_fsync__cnt);
+ cl_assert_equal_sz(4, p_fsync__cnt);
}
static int foreach_cb(void *buf, size_t len, void *payload)
diff --git a/tests/refs/create.c b/tests/refs/create.c
index 5c46fb3..e9630e8 100644
--- a/tests/refs/create.c
+++ b/tests/refs/create.c
@@ -329,7 +329,7 @@ void test_refs_create__fsyncs_when_requested(void)
git_oid_fromstr(&id, current_master_tip);
cl_git_pass(git_reference_create(&ref, g_repo, "refs/heads/fsync_test", &id, 0, "log message"));
git_reference_free(ref);
- cl_assert_equal_i(2, p_fsync__cnt);
+ cl_assert_equal_i(4, p_fsync__cnt);
p_fsync__cnt = 0;
@@ -337,5 +337,5 @@ void test_refs_create__fsyncs_when_requested(void)
cl_git_pass(git_refdb_compress(refdb));
git_refdb_free(refdb);
- cl_assert_equal_i(1, p_fsync__cnt);
+ cl_assert_equal_i(2, p_fsync__cnt);
}