fs-util: add conservative_rename() that suppresses unnecessary renames

if the source and destination file match in contents and basic file
attributes, don#t rename, but just remove source.

This is a simple way to suppress inotify events + mtime changes when
atomically updating files.

diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index 6924f5dfb12bec577ae452d97c7d8d6b8d0471cb..f240f84322476c8a00f03a02236135e395f51bf2 100644 (file)
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -1613,3 +1613,80 @@ int path_is_encrypted(const char *path) {
 
         return blockdev_is_encrypted(p, 10 /* safety net: maximum recursion depth */);
 }
+
+int conservative_rename(
+                int olddirfd, const char *oldpath,
+                int newdirfd, const char *newpath) {
+
+        _cleanup_close_ int old_fd = -1, new_fd = -1;
+        struct stat old_stat, new_stat;
+
+        /* Renames the old path to thew new path, much like renameat() — except if both are regular files and
+         * have the exact same contents and basic file attributes already. In that case remove the new file
+         * instead. This call is useful for reducing inotify wakeups on files that are updated but don't
+         * actually change. This function is written in a style that we rather rename too often than suppress
+         * too much. i.e. whenever we are in doubt we rather rename than fail. After all reducing inotify
+         * events is an optimization only, not more. */
+
+        old_fd = openat(olddirfd, oldpath, O_CLOEXEC|O_RDONLY|O_NOCTTY|O_NOFOLLOW);
+        if (old_fd < 0)
+                goto do_rename;
+
+        new_fd = openat(newdirfd, newpath, O_CLOEXEC|O_RDONLY|O_NOCTTY|O_NOFOLLOW);
+        if (new_fd < 0)
+                goto do_rename;
+
+        if (fstat(old_fd, &old_stat) < 0)
+                goto do_rename;
+
+        if (!S_ISREG(old_stat.st_mode))
+                goto do_rename;
+
+        if (fstat(new_fd, &new_stat) < 0)
+                goto do_rename;
+
+        if (new_stat.st_ino == old_stat.st_ino &&
+            new_stat.st_dev == old_stat.st_dev)
+                goto is_same;
+
+        if (old_stat.st_mode != new_stat.st_mode ||
+            old_stat.st_size != new_stat.st_size ||
+            old_stat.st_uid != new_stat.st_uid ||
+            old_stat.st_gid != new_stat.st_gid)
+                goto do_rename;
+
+        for (;;) {
+                char buf1[16*1024];
+                char buf2[sizeof(buf1) + 1];
+                ssize_t l1, l2;
+
+                l1 = read(old_fd, buf1, sizeof(buf1));
+                if (l1 < 0)
+                        goto do_rename;
+
+                l2 = read(new_fd, buf2, l1 + 1);
+                if (l1 != l2)
+                        goto do_rename;
+
+                if (l1 == 0) /* EOF on both! And everything's the same so far, yay! */
+                        break;
+
+                if (memcmp(buf1, buf2, l1) != 0)
+                        goto do_rename;
+        }
+
+is_same:
+        /* Everything matches? Then don't rename, instead remove the source file, and leave the existing
+         * destination in place */
+
+        if (unlinkat(olddirfd, oldpath, 0) < 0)
+                goto do_rename;
+
+        return 0;
+
+do_rename:
+        if (renameat(olddirfd, oldpath, newdirfd, newpath) < 0)
+                return -errno;
+
+        return 1;
+}

diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h
index 5dc8853eac82aae3b94603aa0051bd952a88fd82..9a39473567f88b938d2e07892f2d9ee8ac030155 100644 (file)
--- a/src/basic/fs-util.h
+++ b/src/basic/fs-util.h
@@ -132,3 +132,5 @@ int syncfs_path(int atfd, const char *path);
 int open_parent(const char *path, int flags, mode_t mode);
 
 int path_is_encrypted(const char *path);
+
+int conservative_rename(int olddirfd, const char *oldpath, int newdirfd, const char *newpath);

diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c
index d1f9252521a4da6436ad33fbf036458d9b8f8000..e0ef8257bdd1c2f8904249226fae34d1f58d21c8 100644 (file)
--- a/src/test/test-fs-util.c
+++ b/src/test/test-fs-util.c
@@ -3,7 +3,9 @@
 #include <unistd.h>
 
 #include "alloc-util.h"
+#include "copy.h"
 #include "fd-util.h"
+#include "fileio.h"
 #include "fs-util.h"
 #include "id128-util.h"
 #include "macro.h"
@@ -834,6 +836,51 @@ static void test_path_is_encrypted(void) {
         test_path_is_encrypted_one("/dev", booted > 0 ? false : -1);
 }
 
+static void test_conservative_rename(void) {
+        _cleanup_(unlink_and_freep) char *p = NULL;
+        _cleanup_free_ char *q = NULL;
+
+        assert_se(tempfn_random_child(NULL, NULL, &p) >= 0);
+        assert_se(write_string_file(p, "this is a test", WRITE_STRING_FILE_CREATE) >= 0);
+
+        assert_se(tempfn_random_child(NULL, NULL, &q) >= 0);
+
+        /* Check that the hardlinked "copy" is detected */
+        assert_se(link(p, q) >= 0);
+        assert_se(conservative_rename(AT_FDCWD, q, AT_FDCWD, p) == 0);
+        assert_se(access(q, F_OK) < 0 && errno == ENOENT);
+
+        /* Check that a manual copy is detected */
+        assert_se(copy_file(p, q, 0, (mode_t) -1, 0, 0, COPY_REFLINK) >= 0);
+        assert_se(conservative_rename(AT_FDCWD, q, AT_FDCWD, p) == 0);
+        assert_se(access(q, F_OK) < 0 && errno == ENOENT);
+
+        /* Check that a manual new writeout is also detected */
+        assert_se(write_string_file(q, "this is a test", WRITE_STRING_FILE_CREATE) >= 0);
+        assert_se(conservative_rename(AT_FDCWD, q, AT_FDCWD, p) == 0);
+        assert_se(access(q, F_OK) < 0 && errno == ENOENT);
+
+        /* Check that a minimally changed version is detected */
+        assert_se(write_string_file(q, "this is a_test", WRITE_STRING_FILE_CREATE) >= 0);
+        assert_se(conservative_rename(AT_FDCWD, q, AT_FDCWD, p) > 0);
+        assert_se(access(q, F_OK) < 0 && errno == ENOENT);
+
+        /* Check that this really is new updated version */
+        assert_se(write_string_file(q, "this is a_test", WRITE_STRING_FILE_CREATE) >= 0);
+        assert_se(conservative_rename(AT_FDCWD, q, AT_FDCWD, p) == 0);
+        assert_se(access(q, F_OK) < 0 && errno == ENOENT);
+
+        /* Make sure we detect extended files */
+        assert_se(write_string_file(q, "this is a_testx", WRITE_STRING_FILE_CREATE) >= 0);
+        assert_se(conservative_rename(AT_FDCWD, q, AT_FDCWD, p) > 0);
+        assert_se(access(q, F_OK) < 0 && errno == ENOENT);
+
+        /* Make sure we detect truncated files */
+        assert_se(write_string_file(q, "this is a_test", WRITE_STRING_FILE_CREATE) >= 0);
+        assert_se(conservative_rename(AT_FDCWD, q, AT_FDCWD, p) > 0);
+        assert_se(access(q, F_OK) < 0 && errno == ENOENT);
+}
+
 int main(int argc, char *argv[]) {
         test_setup_logging(LOG_INFO);
 
@@ -852,6 +899,7 @@ int main(int argc, char *argv[]) {
         test_rename_noreplace();
         test_chmod_and_chown();
         test_path_is_encrypted();
+        test_conservative_rename();
 
         return 0;
 }