mirror of
https://github.com/ostreedev/ostree.git
synced 2024-10-26 17:25:36 +03:00
lib/repo: Don't syncfs or fsync() dirs if fsync opt is disabled
There are use cases for not syncing at all; think build cache repos, etc. Let's be consistent here and make sure if fsync is disabled we do no sync at all. I chose this opportunity to add tests using the shiny new strace fault injection. I can forsee using this for a lot more things, so I made the support for detecting things generic. Related: https://github.com/ostreedev/ostree/issues/1184 Closes: #1186 Approved by: jlebon
This commit is contained in:
parent
7a8511e0ca
commit
75150fe04a
@ -10,7 +10,7 @@ pkg_upgrade
|
||||
pkg_install_builddeps ostree
|
||||
# Until this propagates farther
|
||||
pkg_install 'pkgconfig(libcurl)' 'pkgconfig(openssl)'
|
||||
pkg_install sudo which attr fuse \
|
||||
pkg_install sudo which attr fuse strace \
|
||||
libubsan libasan libtsan PyYAML redhat-rpm-config \
|
||||
elfutils
|
||||
if test -n "${CI_PKGS:-}"; then
|
||||
|
@ -1187,7 +1187,7 @@ rename_pending_loose_objects (OstreeRepo *self,
|
||||
renamed_some_object = TRUE;
|
||||
}
|
||||
|
||||
if (renamed_some_object)
|
||||
if (renamed_some_object && !self->disable_fsync)
|
||||
{
|
||||
/* Ensure that in the case of a power cut all the directory metadata that
|
||||
we want has reached the disk. In particular, we want this before we
|
||||
@ -1208,8 +1208,11 @@ rename_pending_loose_objects (OstreeRepo *self,
|
||||
}
|
||||
|
||||
/* In case we created any loose object subdirs, make sure they are on disk */
|
||||
if (fsync (self->objects_dir_fd) == -1)
|
||||
return glnx_throw_errno_prefix (error, "fsync");
|
||||
if (!self->disable_fsync)
|
||||
{
|
||||
if (fsync (self->objects_dir_fd) == -1)
|
||||
return glnx_throw_errno_prefix (error, "fsync");
|
||||
}
|
||||
|
||||
if (!glnx_tmpdir_delete (&self->commit_stagedir, cancellable, error))
|
||||
return FALSE;
|
||||
@ -1517,10 +1520,11 @@ ostree_repo_commit_transaction (OstreeRepo *self,
|
||||
if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_PRE_COMMIT) > 0)
|
||||
return glnx_throw (error, "OSTREE_REPO_TEST_ERROR_PRE_COMMIT specified");
|
||||
|
||||
/* FIXME: Added since valgrind in el7 doesn't know about
|
||||
* `syncfs`...we should delete this later.
|
||||
/* FIXME: Added OSTREE_SUPPRESS_SYNCFS since valgrind in el7 doesn't know
|
||||
* about `syncfs`...we should delete this later.
|
||||
*/
|
||||
if (g_getenv ("OSTREE_SUPPRESS_SYNCFS") == NULL)
|
||||
if (!self->disable_fsync &&
|
||||
g_getenv ("OSTREE_SUPPRESS_SYNCFS") == NULL)
|
||||
{
|
||||
if (syncfs (self->tmp_dir_fd) < 0)
|
||||
return glnx_throw_errno_prefix (error, "syncfs");
|
||||
|
@ -19,7 +19,7 @@
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
echo "1..$((72 + ${extra_basic_tests:-0}))"
|
||||
echo "1..$((73 + ${extra_basic_tests:-0}))"
|
||||
|
||||
$CMD_PREFIX ostree --version > version.yaml
|
||||
python -c 'import yaml; yaml.safe_load(open("version.yaml"))'
|
||||
@ -809,8 +809,18 @@ cd ${test_tmpdir}
|
||||
rm -rf test2-checkout
|
||||
mkdir -p test2-checkout
|
||||
cd test2-checkout
|
||||
touch should-not-be-fsynced
|
||||
$OSTREE commit ${COMMIT_ARGS} -b test2 -s "Unfsynced commit" --fsync=false
|
||||
echo 'should not be fsynced' > should-not-be-fsynced
|
||||
if ! skip_one_without_strace_fault_injection; then
|
||||
# Test that --fsync=false doesn't fsync
|
||||
fsync_inject_error_ostree="strace -o /dev/null -f -e inject=syncfs,fsync,sync:error=EPERM ostree"
|
||||
${fsync_inject_error_ostree} --repo=${test_tmpdir}/repo commit ${COMMIT_ARGS} -b test2-no-fsync --fsync=false
|
||||
# And test that we get EPERM if we inject an error
|
||||
if ${fsync_inject_error_ostree} --repo=${test_tmpdir}/repo commit ${COMMIT_ARGS} -b test2-no-fsync 2>err.txt; then
|
||||
fatal "fsync error injection failed"
|
||||
fi
|
||||
assert_file_has_content err.txt 'sync.*Operation not permitted'
|
||||
echo "ok fsync disabled"
|
||||
fi
|
||||
|
||||
# Run this test only as non-root user. When run as root, the chmod
|
||||
# won't have any effect.
|
||||
|
@ -543,6 +543,29 @@ skip_without_user_xattrs () {
|
||||
fi
|
||||
}
|
||||
|
||||
# https://brokenpi.pe/tools/strace-fault-injection
|
||||
_have_strace_fault_injection=''
|
||||
have_strace_fault_injection() {
|
||||
if test "${_have_strace_fault_injection}" = ''; then
|
||||
if strace -P ${test_srcdir}/libtest-core.sh -e inject=read:retval=0 cat ${test_srcdir}/libtest-core.sh >out.txt &&
|
||||
test '!' -s out.txt; then
|
||||
_have_strace_fault_injection=yes
|
||||
else
|
||||
_have_strace_fault_injection=no
|
||||
fi
|
||||
rm -f out.txt
|
||||
fi
|
||||
test ${_have_strace_fault_injection} = yes
|
||||
}
|
||||
|
||||
skip_one_without_strace_fault_injection() {
|
||||
if ! have_strace_fault_injection; then
|
||||
echo "ok # SKIP this test requires strace fault injection"
|
||||
return 0
|
||||
fi
|
||||
return 1
|
||||
}
|
||||
|
||||
skip_without_fuse () {
|
||||
fusermount --version >/dev/null 2>&1 || skip "no fusermount"
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user