Merge pull request #17804 from poettering/write-resolve-conf-less

write resolv.conf less often
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2020-12-02 16:56:52 +01:00 committed by GitHub
commit 4cbd28af60
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 149 additions and 6 deletions

View File

@ -1047,18 +1047,29 @@ int copy_file_full(
copy_progress_bytes_t progress_bytes,
void *userdata) {
_cleanup_close_ int fdf = -1;
struct stat st;
int fdt = -1, r;
assert(from);
assert(to);
fdf = open(from, O_RDONLY|O_CLOEXEC|O_NOCTTY);
if (fdf < 0)
return -errno;
if (mode == (mode_t) -1)
if (fstat(fdf, &st) < 0)
return -errno;
RUN_WITH_UMASK(0000) {
if (copy_flags & COPY_MAC_CREATE) {
r = mac_selinux_create_file_prepare(to, S_IFREG);
if (r < 0)
return r;
}
fdt = open(to, flags|O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY, mode);
fdt = open(to, flags|O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY,
mode != (mode_t) -1 ? mode : st.st_mode);
if (copy_flags & COPY_MAC_CREATE)
mac_selinux_create_file_clear();
if (fdt < 0)
@ -1068,13 +1079,16 @@ int copy_file_full(
if (chattr_mask != 0)
(void) chattr_fd(fdt, chattr_flags, chattr_mask & CHATTR_EARLY_FL, NULL);
r = copy_file_fd_full(from, fdt, copy_flags, progress_bytes, userdata);
r = copy_bytes_full(fdf, fdt, (uint64_t) -1, copy_flags, NULL, NULL, progress_bytes, userdata);
if (r < 0) {
close(fdt);
(void) unlink(to);
return r;
}
(void) copy_times(fdf, fdt, copy_flags);
(void) copy_xattr(fdf, fdt);
if (chattr_mask != 0)
(void) chattr_fd(fdt, chattr_flags, chattr_mask & ~CHATTR_EARLY_FL, NULL);

View File

@ -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;
}

View File

@ -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);

View File

@ -359,8 +359,9 @@ int manager_write_resolv_conf(Manager *m) {
goto fail;
}
if (rename(temp_path_stub, PRIVATE_STUB_RESOLV_CONF) < 0)
r = log_error_errno(errno, "Failed to move new %s into place: %m", PRIVATE_STUB_RESOLV_CONF);
r = conservative_rename(AT_FDCWD, temp_path_stub, AT_FDCWD, PRIVATE_STUB_RESOLV_CONF);
if (r < 0)
log_error_errno(r, "Failed to move new %s into place: %m", PRIVATE_STUB_RESOLV_CONF);
} else {
r = symlink_atomic(basename(PRIVATE_UPLINK_RESOLV_CONF), PRIVATE_STUB_RESOLV_CONF);
@ -368,8 +369,9 @@ int manager_write_resolv_conf(Manager *m) {
log_error_errno(r, "Failed to symlink %s: %m", PRIVATE_STUB_RESOLV_CONF);
}
if (rename(temp_path_uplink, PRIVATE_UPLINK_RESOLV_CONF) < 0)
r = log_error_errno(errno, "Failed to move new %s into place: %m", PRIVATE_UPLINK_RESOLV_CONF);
r = conservative_rename(AT_FDCWD, temp_path_uplink, AT_FDCWD, PRIVATE_UPLINK_RESOLV_CONF);
if (r < 0)
log_error_errno(r, "Failed to move new %s into place: %m", PRIVATE_UPLINK_RESOLV_CONF);
fail:
if (r < 0) {

View File

@ -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;
}