From 8fba904b16022ff6003bb15966c9f7f6b4a894ef Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 20 Mar 2015 01:40:51 +0100 Subject: [PATCH 1/2] fix noatime mode, fixes #243 added "nonlocal euid" - without this, euid just gets redefined in inner scope instead of assigned to outer scope added check for euid 0 - if we run as root, we always have permissions (not just if we are file owner) note: due to caching and OS behaviour on linux, the bug was a bit tricky to reproduce and also the fix was a bit tricky to test. one needs strictatime mount option to enfore traditional atime updating. for repeated tests, always change file contents (e.g. from /dev/urandom) or attic's caching will prevent that the file gets read ("accessed") again. check atimes with ls -lu i could reproduce code was broken and is fixed with this changeset. and root now doesn't touch any atimes. --- attic/archive.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/attic/archive.py b/attic/archive.py index d78ce4b8..0b62a105 100644 --- a/attic/archive.py +++ b/attic/archive.py @@ -429,7 +429,8 @@ def open_simple(p, s): return open(p, 'rb') def open_noatime_if_owner(p, s): - if s.st_uid == euid: + if euid == 0 or s.st_uid == euid: + # we are root or owner of file return os.fdopen(os.open(p, flags_noatime), 'rb') else: return open(p, 'rb') @@ -442,6 +443,7 @@ def open_noatime(p, s): fo = open(p, 'rb') # Yes, it was -- otherwise the above line would have thrown # another exception. + nonlocal euid euid = os.geteuid() # So in future, let's check whether the file is owned by us # before attempting to use O_NOATIME. From d43cb4bac89393f39e90d34eaadf7d7ecad1a110 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 24 Mar 2015 22:08:06 +0100 Subject: [PATCH 2/2] refactor _open_rb code a bit, so it is more consistent / regular --- attic/archive.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/attic/archive.py b/attic/archive.py index 0b62a105..10eaa5c5 100644 --- a/attic/archive.py +++ b/attic/archive.py @@ -422,25 +422,31 @@ def list_archives(repository, key, manifest, cache=None): @staticmethod def _open_rb(path, st): - flags_noatime = None + flags_normal = os.O_RDONLY | getattr(os, 'O_BINARY', 0) + flags_noatime = flags_normal | getattr(os, 'NO_ATIME', 0) euid = None def open_simple(p, s): - return open(p, 'rb') + fd = os.open(p, flags_normal) + return os.fdopen(fd, 'rb') + + def open_noatime(p, s): + fd = os.open(p, flags_noatime) + return os.fdopen(fd, 'rb') def open_noatime_if_owner(p, s): if euid == 0 or s.st_uid == euid: # we are root or owner of file - return os.fdopen(os.open(p, flags_noatime), 'rb') + return open_noatime(p, s) else: - return open(p, 'rb') + return open_simple(p, s) - def open_noatime(p, s): + def open_noatime_with_fallback(p, s): try: fd = os.open(p, flags_noatime) except PermissionError: # Was this EPERM due to the O_NOATIME flag? - fo = open(p, 'rb') + fd = os.open(p, flags_normal) # Yes, it was -- otherwise the above line would have thrown # another exception. nonlocal euid @@ -448,14 +454,11 @@ def open_noatime(p, s): # So in future, let's check whether the file is owned by us # before attempting to use O_NOATIME. Archive._open_rb = open_noatime_if_owner - return fo return os.fdopen(fd, 'rb') - o_noatime = getattr(os, 'O_NOATIME', None) - if o_noatime is not None: - flags_noatime = os.O_RDONLY | getattr(os, 'O_BINARY', 0) | o_noatime + if flags_noatime != flags_normal: # Always use O_NOATIME version. - Archive._open_rb = open_noatime + Archive._open_rb = open_noatime_with_fallback else: # Always use non-O_NOATIME version. Archive._open_rb = open_simple