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 57f96f7eab543868a181063ce6594fc70c42f5b8..1a2595c8bf4b5541fe9a4e1adc5cd7d758f64df8 100644 (file)
--- 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 c1820690310d1f1daaaf1851d764c30ef14fd304..a27dc8861a22b931fbd26bf59a9bcbe6b6362d83 100644 (file)
--- 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 8af1cb5ce5f3b6123441e96789811d67e2b04ac7..836753cc750fe382f5a2fb585dfe9ecea1701a3f 100644 (file)
--- 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 db95738b8056147abcbf05e3aafee45209d4f797..4eea560bed10d10e2b8a37c907b7b8bbf410e5cc 100644 (file)
--- 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 dd78a013aaf26c49d3b94ff6c1c21b27477c0dac..10b2405cc37a05b60ea006b209651ddfb4cbf045 100644 (file)
--- 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: