diff options
author | Patrick Steinhardt <ps@pks.im> | 2022-03-11 12:58:59 +0300 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2022-03-15 23:30:58 +0300 |
commit | bc22d845c4328f5bd896d019b3729f776ad4be4c (patch) | |
tree | a7f0f27f3b40768a35e7b91825245d3d9512b1c5 /refs | |
parent | 00997924001f3d8a07510613ab44d16c0a9b2883 (diff) |
core.fsync: new option to harden references
When writing both loose and packed references to disk we first create a
lockfile, write the updated values into that lockfile, and on commit we
rename the file into place. According to filesystem developers, this
behaviour is broken because applications should always sync data to disk
before doing the final rename to ensure data consistency [1][2][3]. If
applications fail to do this correctly, a hard crash of the machine can
easily result in corrupted on-disk data.
This kind of corruption can in fact be easily observed with Git when the
machine hard-resets shortly after writing references to disk. On
machines with ext4, this will likely lead to the "empty files" problem:
the file has been renamed, but its data has not been synced to disk. The
result is that the reference is corrupt, and in the worst case this can
lead to data loss.
Implement a new option to harden references so that users and admins can
avoid this scenario by syncing locked loose and packed references to
disk before we rename them into place.
[1]: https://thunk.org/tytso/blog/2009/03/15/dont-fear-the-fsync/
[2]: https://btrfs.wiki.kernel.org/index.php/FAQ (What are the crash guarantees of overwrite-by-rename)
[3]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/admin-guide/ext4.rst (see auto_da_alloc)
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'refs')
-rw-r--r-- | refs/files-backend.c | 1 | ||||
-rw-r--r-- | refs/packed-backend.c | 3 |
2 files changed, 3 insertions, 1 deletions
diff --git a/refs/files-backend.c b/refs/files-backend.c index f59589d6cc..6521ee8af5 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1787,6 +1787,7 @@ static int write_ref_to_lockfile(struct ref_lock *lock, fd = get_lock_file_fd(&lock->lk); if (write_in_full(fd, oid_to_hex(oid), the_hash_algo->hexsz) < 0 || write_in_full(fd, &term, 1) < 0 || + fsync_component(FSYNC_COMPONENT_REFERENCE, get_lock_file_fd(&lock->lk)) < 0 || close_ref_gently(lock) < 0) { strbuf_addf(err, "couldn't write '%s'", get_lock_file_path(&lock->lk)); diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 27dd8c3922..9d704ccd3e 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1262,7 +1262,8 @@ static int write_with_updates(struct packed_ref_store *refs, goto error; } - if (close_tempfile_gently(refs->tempfile)) { + if (fsync_component(FSYNC_COMPONENT_REFERENCE, get_tempfile_fd(refs->tempfile)) || + close_tempfile_gently(refs->tempfile)) { strbuf_addf(err, "error closing file %s: %s", get_tempfile_path(refs->tempfile), strerror(errno)); |