From e6b77ac7df7fef142149fd2528dc942657a22cb1 Mon Sep 17 00:00:00 2001 From: David Rheinsberg Date: Mon, 19 Dec 2022 10:18:08 +0100 Subject: [PATCH] util/fscache: avoid RENAME_NOREPLACE in _atomic_file() The `RENAME_NOREPLACE` option is not available on NFS. Avoid using it in _atomic_file() to allow NFS backed storage. If the caller allows replacing the destination entry, we simply use the original `os.rename()` system call. This will unconditionally replace the destination on all file-systems. If the caller requests `no-replace`, we cannot use `os.rename()`. Instead, we use `os.link()` to create a new hard-link on the destination. This will always fail if the destination already exists. We then rely on the cleanup-path to unlink the original temporary entry. This will require adjustments in future maintenance tasks on the cache, since they need to be aware that entries can be hardlinked temporarily. However, we already consider `uuid-*` entries in the object-store to be temporary and unaccounted for similar reasons, so this doesn't even break our cache-maintenance ideas. Signed-off-by: David Rheinsberg --- osbuild/util/fscache.py | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/osbuild/util/fscache.py b/osbuild/util/fscache.py index ac4bfd70..ed901d87 100644 --- a/osbuild/util/fscache.py +++ b/osbuild/util/fscache.py @@ -438,28 +438,35 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike): with os.fdopen(fd, "r+", closefd=False, encoding="utf8") as file: yield file - if replace: - flags = ctypes.c_uint(0) - else: - flags = self._libc.RENAME_NOREPLACE - suppress = [] if ignore_exist: suppress.append(errno.EEXIST) - # As a last step, move the file to the desired location. - with ctx.suppress_oserror(*suppress): - self._libc.renameat2( - oldpath=self._path(rpath_tmp).encode(), - newpath=self._path(rpath).encode(), - flags=flags, + if replace: + # Move the file into the desired location, possibly + # replacing any existing entry. + os.rename( + src=self._path(rpath_tmp), + dst=self._path(rpath), ) + else: + # Preferably, we used `RENAME_NOREPLACE`, but this is not + # supported on NFS. Instead, we create a hard-link, which + # will fail if the target already exists. We rely on the + # cleanup-path to drop the original link. + with ctx.suppress_oserror(*suppress): + os.link( + src=self._path(rpath_tmp), + dst=self._path(rpath), + follow_symlinks=False, + ) finally: if rpath_tmp is not None: - # If the temporary file exists, we delete it on error. If we - # haven't created it, or if we already moved it, this will be a - # no-op. Due to the unique name, we will never delete a file we - # do not own. + # If the temporary file exists, we delete it. If we haven't + # created it, or if we already moved it, this will be a no-op. + # Due to the unique name, we will never delete a file we do not + # own. If we hard-linked the file, this merely deletes the + # original temporary link. # On fatal errors, we leak the file into the object store. Due # to the released lock and UUID name, cache management will # clean it up.