From: Daan De Meyer Date: Fri, 23 Aug 2024 11:40:40 +0000 (+0200) Subject: copy: Introduce COPY_NOCOW_AFTER and use it when copying images X-Git-Tag: v257-rc1~557^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b1cfa9308061d5aacc65ef6d898ce605edde417d;p=thirdparty%2Fsystemd.git copy: Introduce COPY_NOCOW_AFTER and use it when copying images When dealing with copying COW images, we have to make a tradeoff: - Either we don't touch the NOCOW bit on the copied file COW and get an instant copy because we're able to reflink, but we might get reduced performance if the source file was COW as COW files and lots of random writes don't play well together. - Or we force NOCOW for the copied file, which means we have to do a full copy as reflinking from COW files to NOCOW files or vice versa is not supported. In exec-invoke.c, we've opted for the first option. In nspawn.c and discover-image.c, we've opted for the second option. In nspawn, this applies to the --ephemeral option to make ephemeral copies. In discover-image.c, this applies to cloning images into /var/lib/machines. Both these features might be used to run many machines of the same original image. We really don't want to force a full copy onto users in these scenarios when they're expecting reflink behavior, leading to them running out of disk space. Instead, degraded performance in their machines is a much less severe issue, which they will discover on their own if it affects them, at which point they can make their original image NOCOW at which point they'll get both the reflinks and better performance. Given the above reasoning, let's switch nspawn.c and discover-image.c to use COPY_NOCOW_AFTER as well instead of enabling NOCOW upfront and forcing a copy if the original source image is COW. --- diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index 57f96f7eab5..1a2595c8bf4 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -2881,26 +2881,11 @@ static int setup_ephemeral( if (*root_image) { log_debug("Making ephemeral copy of %s to %s", *root_image, new_root); - fd = copy_file(*root_image, - new_root, - O_EXCL, - 0600, - COPY_LOCK_BSD| - COPY_REFLINK| - COPY_CRTIME); + fd = copy_file(*root_image, new_root, O_EXCL, 0600, + COPY_LOCK_BSD|COPY_REFLINK|COPY_CRTIME|COPY_NOCOW_AFTER); if (fd < 0) return log_debug_errno(fd, "Failed to copy image %s to %s: %m", *root_image, new_root); - - /* A root image might be subject to lots of random writes so let's try to disable COW on it - * which tends to not perform well in combination with lots of random writes. - * - * Note: btrfs actually isn't impressed by us setting the flag after making the reflink'ed - * copy, but we at least want to make the intention clear. - */ - r = chattr_fd(fd, FS_NOCOW_FL, FS_NOCOW_FL, NULL); - if (r < 0) - log_debug_errno(r, "Failed to disable copy-on-write for %s, ignoring: %m", new_root); } else { assert(*root_directory); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index c1820690310..a27dc8861a2 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -36,6 +36,7 @@ #include "capability-util.h" #include "cgroup-util.h" #include "chase.h" +#include "chattr-util.h" #include "common-signal.h" #include "copy.h" #include "cpu-set-util.h" @@ -6072,10 +6073,8 @@ static int run(int argc, char *argv[]) { { BLOCK_SIGNALS(SIGINT); - r = copy_file_full(arg_image, np, O_EXCL, arg_read_only ? 0400 : 0600, - FS_NOCOW_FL, FS_NOCOW_FL, - COPY_REFLINK|COPY_CRTIME|COPY_SIGINT, - NULL, NULL); + r = copy_file(arg_image, np, O_EXCL, arg_read_only ? 0400 : 0600, + COPY_REFLINK|COPY_CRTIME|COPY_SIGINT|COPY_NOCOW_AFTER); } if (r == -EINTR) { log_error_errno(r, "Interrupted while copying image file to %s, removed again.", np); diff --git a/src/shared/copy.c b/src/shared/copy.c index 8af1cb5ce5f..836753cc750 100644 --- a/src/shared/copy.c +++ b/src/shared/copy.c @@ -1505,8 +1505,9 @@ int copy_file_at_full( goto fail; } - if (chattr_mask != 0) - (void) chattr_fd(fdt, chattr_flags, chattr_mask & ~CHATTR_EARLY_FL, NULL); + unsigned nocow = FLAGS_SET(copy_flags, COPY_NOCOW_AFTER) ? FS_NOCOW_FL : 0; + if ((chattr_mask | nocow) != 0) + (void) chattr_fd(fdt, chattr_flags | nocow, (chattr_mask & ~CHATTR_EARLY_FL) | nocow, NULL); if (copy_flags & (COPY_FSYNC|COPY_FSYNC_FULL)) { if (fsync(fdt) < 0) { @@ -1593,8 +1594,9 @@ int copy_file_atomic_at_full( t = mfree(t); - if (chattr_mask != 0) - (void) chattr_fd(fdt, chattr_flags, chattr_mask & ~CHATTR_EARLY_FL, NULL); + unsigned nocow = FLAGS_SET(copy_flags, COPY_NOCOW_AFTER) ? FS_NOCOW_FL : 0; + if ((chattr_mask | nocow) != 0) + (void) chattr_fd(fdt, chattr_flags | nocow, (chattr_mask & ~CHATTR_EARLY_FL) | nocow, NULL); r = close_nointr(TAKE_FD(fdt)); /* even if this fails, the fd is now invalidated */ if (r < 0) diff --git a/src/shared/copy.h b/src/shared/copy.h index db95738b805..4eea560bed1 100644 --- a/src/shared/copy.h +++ b/src/shared/copy.h @@ -32,6 +32,14 @@ typedef enum CopyFlags { COPY_LOCK_BSD = 1 << 17, /* Return a BSD exclusively locked file descriptor referring to the copied image/directory. */ COPY_VERIFY_LINKED = 1 << 18, /* Check the source file is still linked after copying. */ COPY_RESTORE_DIRECTORY_TIMESTAMPS = 1 << 19, /* Make sure existing directory timestamps don't change during copying. */ + /* A root image might be subject to lots of random writes so we provide a flag to try to disable COW + * on a copied file which tends to not perform well in combination with lots of random writes. + * + * Note: btrfs actually isn't impressed by us setting the flag after making the copy, but this at + * least makes the intention clear. We don't want to unconditionally set the flag before doing the + * copy because reflinking from COW to NOCOW files is not supported. + */ + COPY_NOCOW_AFTER = 1 << 20, } CopyFlags; typedef enum DenyType { diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c index dd78a013aaf..10b2405cc37 100644 --- a/src/shared/discover-image.c +++ b/src/shared/discover-image.c @@ -1205,8 +1205,8 @@ 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_full(i->path, new_path, read_only ? 0444 : 0644, FS_NOCOW_FL, FS_NOCOW_FL, - COPY_REFLINK|COPY_CRTIME, NULL, NULL); + r = copy_file_atomic(i->path, new_path, read_only ? 0444 : 0644, + COPY_REFLINK|COPY_CRTIME|COPY_NOCOW_AFTER); break; case IMAGE_BLOCK: