diff --git a/src/borg/archive.py b/src/borg/archive.py index db7de5cd03..19dd9056a1 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -287,6 +287,28 @@ def backup_io_iter(iterator): yield item +class StatOrigAtime: + """ + An os.stat_result look-alike: all attributes come from *st*, only the + access time comes from *st_orig*. + + Rationale: borg needs to open() a fs item before it can fstat() it. If the + platform does not support O_NOATIME (or we are not allowed to use it), that + open() might have already updated the atime - we do not want to archive such + an atime, but the one the item had before borg touched it. + """ + + __slots__ = ("_st", "st_atime", "st_atime_ns") + + def __init__(self, st, st_orig): + self._st = st + self.st_atime = st_orig.st_atime + self.st_atime_ns = st_orig.st_atime_ns + + def __getattr__(self, name): + return getattr(self._st, name) + + def stat_update_check(st_old, st_curr): """ this checks for some race conditions between the first filename-based stat() @@ -309,6 +331,12 @@ def stat_update_check(st_old, st_curr): if st_old.st_ino != st_curr.st_ino: # in this case, the hard-links-related code in create_helper has the wrong inode - abort! raise BackupRaceConditionError("file inode changed (race condition), skipping file") + if st_old.st_atime_ns != st_curr.st_atime_ns: + # the atime was updated in between the 2 stat calls - most likely by us, because we + # had to open the item and O_NOATIME was not available / not usable, see #6194. + # in the (rare) case that somebody else accessed the item at just that moment, we + # lose that atime update, but that is much less of an issue than archiving our own. + return StatOrigAtime(st_curr, st_old) # looks ok, we are still dealing with the same thing - return current stat: return st_curr diff --git a/src/borg/testsuite/archive_test.py b/src/borg/testsuite/archive_test.py index cb2aee520e..9f12665dc3 100644 --- a/src/borg/testsuite/archive_test.py +++ b/src/borg/testsuite/archive_test.py @@ -7,13 +7,14 @@ import pytest -from . import rejected_dotdot_paths +from . import rejected_dotdot_paths, is_utime_fully_supported from ..cache import ChunkListEntry from ..constants import ROBJ_FILE_STREAM, zeros from ..crypto.key import ChecksumKey from ..archive import Archive, CacheChunkBuffer, DownloadPipeline, RobustUnpacker, valid_msgpacked_dict from ..archive import ITEM_KEYS, Statistics -from ..archive import BackupOSError, backup_io, backup_io_iter, get_item_uid_gid +from ..archive import BackupOSError, BackupRaceConditionError, backup_io, backup_io_iter, get_item_uid_gid +from ..archive import stat_update_check from ..helpers import msgpack from ..repoobj import RepoObj from ..item import Item, ArchiveItem @@ -467,6 +468,59 @@ def __next__(self): assert False, "StopIteration handled incorrectly" +def _stat_with_atime(path, atime_ns, mtime_ns=234567890000000000): + os.utime(path, ns=(atime_ns, mtime_ns)) + return os.stat(path) + + +@pytest.mark.skipif(not is_utime_fully_supported(), reason="cannot properly setup and execute test without utime") +def test_stat_update_check_atime_updated(tmpdir): + path = str(tmpdir.join("file")) + with open(path, "wb") as f: + f.write(b"12345") + st_old = _stat_with_atime(path, 123456789000000000) + st_curr = _stat_with_atime(path, 987654321000000000) + st = stat_update_check(st_old, st_curr) + # the atime is the one from before we (usually: by opening the file) touched it, see #6194: + assert st.st_atime_ns == st_old.st_atime_ns + assert st.st_atime == st_old.st_atime + # everything else comes from the current stat: + assert st.st_mode == st_curr.st_mode + assert st.st_ino == st_curr.st_ino + assert st.st_size == st_curr.st_size + assert st.st_mtime_ns == st_curr.st_mtime_ns + # optional attributes must be present (or absent) just like on a real stat result: + assert hasattr(st, "st_birthtime_ns") == hasattr(st_curr, "st_birthtime_ns") + with pytest.raises(AttributeError): + st.st_does_not_exist + + +@pytest.mark.skipif(not is_utime_fully_supported(), reason="cannot properly setup and execute test without utime") +def test_stat_update_check_atime_unchanged(tmpdir): + path = str(tmpdir.join("file")) + with open(path, "wb") as f: + f.write(b"12345") + st_old = _stat_with_atime(path, 123456789000000000) + st_curr = os.stat(path) + assert st_old.st_atime_ns == st_curr.st_atime_ns + # nothing to fix up, so we get the current stat result as is: + assert stat_update_check(st_old, st_curr) is st_curr + + +def test_stat_update_check_race_conditions(tmpdir): + file_path = str(tmpdir.join("file")) + with open(file_path, "wb"): + pass + other_path = str(tmpdir.join("other_file")) + with open(other_path, "wb"): + pass + st_file, st_other, st_dir = os.stat(file_path), os.stat(other_path), os.stat(str(tmpdir)) + with pytest.raises(BackupRaceConditionError): # file type changed + stat_update_check(st_file, st_dir) + with pytest.raises(BackupRaceConditionError): # inode changed + stat_update_check(st_file, st_other) + + def test_get_item_uid_gid(): # test requires that: # - a user/group name for the current process' real uid/gid exists. diff --git a/src/borg/testsuite/archiver/extract_cmd_test.py b/src/borg/testsuite/archiver/extract_cmd_test.py index d2a3e302c9..e405b7336f 100644 --- a/src/borg/testsuite/archiver/extract_cmd_test.py +++ b/src/borg/testsuite/archiver/extract_cmd_test.py @@ -9,6 +9,7 @@ from ... import xattr from ... import platform +from ... import archive as archive_module from ...archive import Archive from ...cache import Cache from ...chunkers import has_seek_hole @@ -306,6 +307,47 @@ def has_noatime(some_file): assert same_ts_ns(sto.st_atime_ns, atime * 10**9) +@pytest.mark.skipif(not is_utime_fully_supported(), reason="cannot properly setup and execute test without utime") +def test_atime_open_updates_atime(archiver): + # simulate a platform where already the open() updates the atime (no O_NOATIME support, + # e.g. cygwin) - borg must archive the atime the item had before borg opened it, see #6194. + atime, mtime = 123456780, 234567890 + + def updates_atime(some_file): + os.utime(some_file, (atime, mtime)) + with open(some_file, "rb") as f: + f.read(1) + return os.stat(some_file).st_atime_ns != atime * 10**9 + + real_os_open = archive_module.os_open + + def os_open_updating_atime(**kwargs): + # open without O_NOATIME (as on a platform not having it), so that reading + # updates the atime - and read right away, as if the open() had done that. + fd = real_os_open(**dict(kwargs, noatime=False)) + if fd is not None: + try: + os.pread(fd, 1, 0) # a read updates the atime (but not the ctime) + except OSError: + pass # not seekable / not readable (fifo, directory, ...) + return fd + + create_test_files(archiver.input_path) + if not updates_atime("input/file1"): + pytest.skip("filesystem does not update the atime when reading") + os.utime("input/file1", (atime, mtime)) + cmd(archiver, "repo-create", RK_ENCRYPTION) + with patch.object(archive_module, "os_open", os_open_updating_atime): + cmd(archiver, "create", "--atime", "test", "input") + # make sure the simulation worked: the open() did update the input file's atime. + assert os.stat("input/file1").st_atime_ns != atime * 10**9 + with changedir("output"): + cmd(archiver, "extract", "test") + sto = os.stat("output/input/file1") + assert same_ts_ns(sto.st_mtime_ns, mtime * 10**9) + assert same_ts_ns(sto.st_atime_ns, atime * 10**9) + + @pytest.mark.skipif(not is_utime_fully_supported(), reason="cannot setup and execute test without utime") @pytest.mark.skipif(not is_birthtime_fully_supported(), reason="cannot setup and execute test without birthtime") def test_birthtime(archivers, request):