fs: make fsync opt-in via CopyFileOpt and CopyDirOpt

Add WithFileSync() option to CopyFile and WithCopyFileSync()/WithCopyDirSync()
options to CopyDir, making per-file fsync opt-in for callers that require
durability guarantees. Default behavior is no-fsync, restoring the behavior
prior to a424ba1 which added per-file fsync unconditionally.

The variadic signature for CopyFile is backwards-compatible and existing
callers require no changes.

Signed-off-by: ayush-panta <ayushkp@amazon.com>

Author: ayush-panta

fs/copy.go

@@ -33,7 +33,9 @@ type XAttrErrorHandler func(dst, src, xattrKey string, err error) error
 type copyDirOpts struct {
 	xeh XAttrErrorHandler
 	// xex contains a set of xattrs to exclude when copying
-	xex map[string]struct{}
+	xex      map[string]struct{}
+	fileSync bool
+	dirSync  bool
 }
 
 type CopyDirOpt func(*copyDirOpts) error
@@ -69,6 +71,34 @@ func WithXAttrExclude(keys ...string) CopyDirOpt {
 	}
 }
 
+// WithCopyFileSync ensures each file copied within CopyDir is fsynced to
+// persistent storage before proceeding to the next file.
+//
+// By default, files are not synced. Versions prior to v0.5.0 never synced;
+// v0.5.0 added unconditional per-file fsync which caused a performance
+// regression.
+func WithCopyFileSync() CopyDirOpt {
+	return func(o *copyDirOpts) error {
+		o.fileSync = true
+		return nil
+	}
+}
+
+// WithCopyDirSync ensures full crash durability during CopyDir.
+// This implies file-level fsync (WithCopyFileSync) and additionally fsyncs
+// each directory after all its entries have been copied.
+//
+// By default, neither files nor directories are synced. Versions prior to
+// v0.5.0 never synced; v0.5.0 added unconditional per-file fsync which
+// caused a performance regression.
+func WithCopyDirSync() CopyDirOpt {
+	return func(o *copyDirOpts) error {
+		o.fileSync = true
+		o.dirSync = true
+		return nil
+	}
+}
+
 // CopyDir copies the directory from src to dst.
 // Most efficient copy of files is attempted.
 func CopyDir(dst, src string, opts ...CopyDirOpt) error {
@@ -143,7 +173,7 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er
 				if err := os.Link(link, target); err != nil {
 					return fmt.Errorf("failed to create hard link: %w", err)
 				}
-			} else if err := CopyFile(target, source); err != nil {
+			} else if err := copyFileWithOpts(target, source, o); err != nil {
 				return fmt.Errorf("failed to copy files: %w", err)
 			}
 		case (fileInfo.Mode() & os.ModeSymlink) == os.ModeSymlink:
@@ -185,16 +215,54 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er
 			return err
 		}
 	}
-	return dr.Err()
+	if err := dr.Err(); err != nil {
+		return err
+	}
+
+	if o.dirSync {
+		if err := syncDirectory(dst); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func copyFileWithOpts(target, source string, o *copyDirOpts) error {
+	if o.fileSync {
+		return CopyFile(target, source, WithFileSync())
+	}
+	return CopyFile(target, source)
+}
+
+type CopyFileOpt func(*copyFileConfig)
+
+type copyFileConfig struct {
+	sync bool
+}
+
+// WithFileSync ensures the copied file is fsynced to persistent
+// storage before returning.
+//
+// By default, files are not synced. Versions prior to v0.5.0 never synced;
+// v0.5.0 added unconditional per-file fsync which caused a performance regression.
+func WithFileSync() CopyFileOpt {
+	return func(c *copyFileConfig) {
+		c.sync = true
+	}
 }
 
 // CopyFile copies the source file to the target.
 // The most efficient means of copying is used for the platform.
-func CopyFile(target, source string) error {
-	return copyFile(target, source)
+func CopyFile(target, source string, opts ...CopyFileOpt) error {
+	var cfg copyFileConfig
+	for _, o := range opts {
+		o(&cfg)
+	}
+	return copyFile(target, source, cfg.sync)
 }
 
-func openAndCopyFile(target, source string) error {
+func openAndCopyFile(target, source string, sync bool) error {
 	src, err := os.Open(source)
 	if err != nil {
 		return fmt.Errorf("failed to open source %s: %w", source, err)
@@ -206,6 +274,13 @@ func openAndCopyFile(target, source string) error {
 	}
 	defer tgt.Close()
 
-	_, err = io.Copy(tgt, src)
-	return err
+	if _, err = io.Copy(tgt, src); err != nil {
+		return err
+	}
+	if sync {
+		if err := tgt.Sync(); err != nil {
+			return fmt.Errorf("failed to sync target %s: %w", target, err)
+		}
+	}
+	return nil
 }

fs/copy_darwin.go

@@ -23,13 +23,13 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-func copyFile(target, source string) error {
+func copyFile(target, source string, sync bool) error {
 	if err := unix.Clonefile(source, target, unix.CLONE_NOFOLLOW); err != nil {
 		if !errors.Is(err, unix.ENOTSUP) && !errors.Is(err, unix.EXDEV) {
 			return fmt.Errorf("clonefile failed: %w", err)
 		}
 
-		return openAndCopyFile(target, source)
+		return openAndCopyFile(target, source, sync)
 	}
 	return nil
 }

fs/copy_linux.go

@@ -34,7 +34,7 @@ const maxCopyChunk = 1 << 30 // 1 GiB
 //
 // If the filesystem does not support SEEK_DATA/SEEK_HOLE, it falls back
 // to a plain io.Copy.
-func copyFile(target, source string) error {
+func copyFile(target, source string, sync bool) error {
 	src, err := os.Open(source)
 	if err != nil {
 		return fmt.Errorf("failed to open source %s: %w", source, err)
@@ -72,7 +72,7 @@ func copyFile(target, source string) error {
 			// Filesystem doesn't support SEEK_DATA/SEEK_HOLE. Fall back to a plain copy.
 			src.Close()
 			tgt.Close()
-			return openAndCopyFile(target, source)
+			return openAndCopyFile(target, source, sync)
 		}
 
 		return fmt.Errorf("failed to seek data in source %s: %w", source, err)
@@ -122,7 +122,7 @@ func copyFile(target, source string) error {
 				if errors.Is(err, syscall.EXDEV) || errors.Is(err, syscall.ENOSYS) || errors.Is(err, syscall.EOPNOTSUPP) {
 					src.Close()
 					tgt.Close()
-					return openAndCopyFile(target, source)
+					return openAndCopyFile(target, source, sync)
 				}
 				return fmt.Errorf("copy_file_range failed: %w", err)
 			}
@@ -135,8 +135,10 @@ func copyFile(target, source string) error {
 		offset = holeStart
 	}
 
-	if err := tgt.Sync(); err != nil {
-		return fmt.Errorf("failed to sync target %s: %w", target, err)
+	if sync {
+		if err := tgt.Sync(); err != nil {
+			return fmt.Errorf("failed to sync target %s: %w", target, err)
+		}
 	}
 
 	return nil

fs/copy_linux_test.go

@@ -128,6 +128,81 @@ func TestCopyFileSparse(t *testing.T) {
 	}
 }
 
+func TestCopyFileSparseWithSync(t *testing.T) {
+	dir := t.TempDir()
+
+	type testCase struct {
+		name  string
+		parts []int64
+	}
+
+	tests := []testCase{
+		{
+			name:  "DataHoleData",
+			parts: []int64{4096, 1024 * 1024, 4096},
+		},
+		{
+			name:  "NoHoles",
+			parts: []int64{64 * 1024},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			srcPath := filepath.Join(dir, tc.name+"-sync-src")
+			dstPath := filepath.Join(dir, tc.name+"-sync-dst")
+
+			applier := createSparseFile(tc.name+"-sync-src", 42, 0o644, tc.parts...)
+			if err := applier.Apply(dir); err != nil {
+				t.Fatal(err)
+			}
+
+			if err := CopyFile(dstPath, srcPath, WithFileSync()); err != nil {
+				t.Fatalf("CopyFile with WithFileSync failed: %v", err)
+			}
+
+			// Verify content matches exactly.
+			srcData, err := os.ReadFile(srcPath)
+			if err != nil {
+				t.Fatal(err)
+			}
+			dstData, err := os.ReadFile(dstPath)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !bytes.Equal(srcData, dstData) {
+				t.Fatal("source and destination file contents differ")
+			}
+
+			// Verify sparseness is preserved.
+			srcStat, err := os.Stat(srcPath)
+			if err != nil {
+				t.Fatal(err)
+			}
+			dstStat, err := os.Stat(dstPath)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			srcBlocks := srcStat.Sys().(*syscall.Stat_t).Blocks
+			dstBlocks := dstStat.Sys().(*syscall.Stat_t).Blocks
+
+			t.Logf("src size=%d blocks=%d, dst size=%d blocks=%d",
+				srcStat.Size(), srcBlocks, dstStat.Size(), dstBlocks)
+
+			if srcStat.Size() != dstStat.Size() {
+				t.Fatalf("size mismatch: src=%d dst=%d", srcStat.Size(), dstStat.Size())
+			}
+
+			maxBlocks := srcBlocks + srcBlocks/10 + 8
+			if dstBlocks > maxBlocks {
+				t.Fatalf("destination is not sparse: src blocks=%d, dst blocks=%d (max allowed=%d)",
+					srcBlocks, dstBlocks, maxBlocks)
+			}
+		})
+	}
+}
+
 func TestCopyReflinkWithXFS(t *testing.T) {
 	testutil.RequiresRoot(t)
 	mnt := t.TempDir()

fs/copy_nondarwin.go

@@ -18,4 +18,6 @@
 
 package fs
 
-var copyFile = openAndCopyFile
+func copyFile(target, source string, sync bool) error {
+	return openAndCopyFile(target, source, sync)
+}