From adc6f43b148b097c846f63522876f0f1c91ea0c0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 20 Dec 2018 16:01:57 +0100 Subject: [PATCH] copy: don't synthesize a 'user.crtime_usec' xattr on copy unless explicitly requested Previously, when we'd copy an individual file we'd synthesize a user.crtime_usec xattr with the source's creation time if we can determine it. As the creation/birth time was until recently not queriable form userspace this effectively just propagated the same xattr on the source to the same xattr on the destination. However, current kernels now allow to query the birthtime using statx() and we do make use of that now. Which means that suddenly we started synthesizing these xattrs much more regularly. Doing this actually does make sense, but only in very few cases: not for the typical regular files we copy, but certainly when dealing with disk images. Hence, let's keep this kind of propagation, but let's make it a flag and default to off. Then turn it on whenever we deal with disk images, and leave it off otherwise. This is particularly relevant as overlayfs combining a real fs, and a tmpfs on top will result in EOPNOTSUPP when it is attempted to open a file with xattrs for writing, as tmpfs does not support xattrs, and hence the copy-up cannot work. Hence, let's avoid synthesizing this needlessly, to increase compat with overlayfs. --- src/basic/copy.c | 13 ++++++++----- src/basic/copy.h | 3 ++- src/boot/bootctl.c | 2 +- src/import/export-raw.c | 2 +- src/import/import-raw.c | 2 +- src/import/pull-raw.c | 2 +- src/nspawn/nspawn.c | 2 +- src/shared/machine-image.c | 2 +- 8 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/basic/copy.c b/src/basic/copy.c index 46e02a3759..2f36c8eb87 100644 --- a/src/basic/copy.c +++ b/src/basic/copy.c @@ -743,7 +743,7 @@ int copy_file_fd_full( r = copy_bytes_full(fdf, fdt, (uint64_t) -1, copy_flags, NULL, NULL, progress_bytes, userdata); - (void) copy_times(fdf, fdt); + (void) copy_times(fdf, fdt, copy_flags); (void) copy_xattr(fdf, fdt); return r; @@ -849,10 +849,9 @@ int copy_file_atomic_full( return 0; } -int copy_times(int fdf, int fdt) { +int copy_times(int fdf, int fdt, CopyFlags flags) { struct timespec ut[2]; struct stat st; - usec_t crtime = 0; assert(fdf >= 0); assert(fdt >= 0); @@ -866,8 +865,12 @@ int copy_times(int fdf, int fdt) { if (futimens(fdt, ut) < 0) return -errno; - if (fd_getcrtime(fdf, &crtime) >= 0) - (void) fd_setcrtime(fdt, crtime); + if (FLAGS_SET(flags, COPY_CRTIME)) { + usec_t crtime; + + if (fd_getcrtime(fdf, &crtime) >= 0) + (void) fd_setcrtime(fdt, crtime); + } return 0; } diff --git a/src/basic/copy.h b/src/basic/copy.h index f677021881..a33546d3ab 100644 --- a/src/basic/copy.h +++ b/src/basic/copy.h @@ -14,6 +14,7 @@ typedef enum CopyFlags { COPY_REPLACE = 1 << 2, /* Replace an existing file if there's one */ COPY_SAME_MOUNT = 1 << 3, /* Don't descend recursively into other file systems, across mount point boundaries */ COPY_MERGE_EMPTY = 1 << 4, /* Merge an existing, empty directory with our new tree to copy */ + COPY_CRTIME = 1 << 5, /* Generate a user.crtime_usec xattr off the source crtime if there is one, on copying */ } CopyFlags; typedef int (*copy_progress_bytes_t)(uint64_t n_bytes, void *userdata); @@ -57,5 +58,5 @@ static inline int copy_bytes(int fdf, int fdt, uint64_t max_bytes, CopyFlags cop return copy_bytes_full(fdf, fdt, max_bytes, copy_flags, NULL, NULL, NULL, NULL); } -int copy_times(int fdf, int fdt); +int copy_times(int fdf, int fdt, CopyFlags flags); int copy_xattr(int fdf, int fdt); diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index dc2fd96628..323806a534 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -436,7 +436,7 @@ static int copy_file_with_version_check(const char *from, const char *to, bool f return log_error_errno(r, "Failed to copy data from \"%s\" to \"%s\": %m", from, t); } - (void) copy_times(fd_from, fd_to); + (void) copy_times(fd_from, fd_to, 0); if (fsync(fd_to) < 0) { (void) unlink_noerrno(t); diff --git a/src/import/export-raw.c b/src/import/export-raw.c index 6a02b47a17..c1c946cd2b 100644 --- a/src/import/export-raw.c +++ b/src/import/export-raw.c @@ -223,7 +223,7 @@ static int raw_export_process(RawExport *e) { finish: if (r >= 0) { - (void) copy_times(e->input_fd, e->output_fd); + (void) copy_times(e->input_fd, e->output_fd, COPY_CRTIME); (void) copy_xattr(e->input_fd, e->output_fd); } diff --git a/src/import/import-raw.c b/src/import/import-raw.c index 4b1161557d..56f3431a08 100644 --- a/src/import/import-raw.c +++ b/src/import/import-raw.c @@ -215,7 +215,7 @@ static int raw_import_finish(RawImport *i) { return r; if (S_ISREG(i->st.st_mode)) { - (void) copy_times(i->input_fd, i->output_fd); + (void) copy_times(i->input_fd, i->output_fd, COPY_CRTIME); (void) copy_xattr(i->input_fd, i->output_fd); } diff --git a/src/import/pull-raw.c b/src/import/pull-raw.c index 3a3e015df8..72b9054e49 100644 --- a/src/import/pull-raw.c +++ b/src/import/pull-raw.c @@ -368,7 +368,7 @@ static int raw_pull_make_local_copy(RawPull *i) { return log_error_errno(r, "Failed to make writable copy of image: %m"); } - (void) copy_times(i->raw_job->disk_fd, dfd); + (void) copy_times(i->raw_job->disk_fd, dfd, COPY_CRTIME); (void) copy_xattr(i->raw_job->disk_fd, dfd); dfd = safe_close(dfd); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 14f251a277..5cb049e5f7 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4400,7 +4400,7 @@ int main(int argc, char *argv[]) { goto finish; } - r = copy_file(arg_image, np, O_EXCL, arg_read_only ? 0400 : 0600, FS_NOCOW_FL, COPY_REFLINK); + r = copy_file(arg_image, np, O_EXCL, arg_read_only ? 0400 : 0600, FS_NOCOW_FL, COPY_REFLINK|COPY_CRTIME); if (r < 0) { r = log_error_errno(r, "Failed to copy image file: %m"); goto finish; diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c index af06ab22e8..3d61221056 100644 --- a/src/shared/machine-image.c +++ b/src/shared/machine-image.c @@ -870,7 +870,7 @@ int image_clone(Image *i, const char *new_name, bool read_only) { case IMAGE_RAW: new_path = strjoina("/var/lib/machines/", new_name, ".raw"); - r = copy_file_atomic(i->path, new_path, read_only ? 0444 : 0644, FS_NOCOW_FL, COPY_REFLINK); + r = copy_file_atomic(i->path, new_path, read_only ? 0444 : 0644, FS_NOCOW_FL, COPY_REFLINK|COPY_CRTIME); break; case IMAGE_BLOCK: