Skip to content

Commit c7d3c74

Browse files
committed
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>
1 parent f417961 commit c7d3c74

8 files changed

Lines changed: 382 additions & 16 deletions

File tree

fs/copy.go

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ type XAttrErrorHandler func(dst, src, xattrKey string, err error) error
3333
type copyDirOpts struct {
3434
xeh XAttrErrorHandler
3535
// xex contains a set of xattrs to exclude when copying
36-
xex map[string]struct{}
36+
xex map[string]struct{}
37+
fileSync bool
38+
dirSync bool
3739
}
3840

3941
type CopyDirOpt func(*copyDirOpts) error
@@ -69,6 +71,26 @@ func WithXAttrExclude(keys ...string) CopyDirOpt {
6971
}
7072
}
7173

74+
// WithCopyFileSync ensures each file copied within CopyDir is fsynced to
75+
// persistent storage before proceeding to the next file.
76+
func WithCopyFileSync() CopyDirOpt {
77+
return func(o *copyDirOpts) error {
78+
o.fileSync = true
79+
return nil
80+
}
81+
}
82+
83+
// WithCopyDirSync ensures full crash durability during CopyDir.
84+
// This implies file-level fsync (WithCopyFileSync) and additionally fsyncs
85+
// each directory after all its entries have been copied.
86+
func WithCopyDirSync() CopyDirOpt {
87+
return func(o *copyDirOpts) error {
88+
o.fileSync = true
89+
o.dirSync = true
90+
return nil
91+
}
92+
}
93+
7294
// CopyDir copies the directory from src to dst.
7395
// Most efficient copy of files is attempted.
7496
func CopyDir(dst, src string, opts ...CopyDirOpt) error {
@@ -143,7 +165,7 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er
143165
if err := os.Link(link, target); err != nil {
144166
return fmt.Errorf("failed to create hard link: %w", err)
145167
}
146-
} else if err := CopyFile(target, source); err != nil {
168+
} else if err := copyFileWithOpts(target, source, o); err != nil {
147169
return fmt.Errorf("failed to copy files: %w", err)
148170
}
149171
case (fileInfo.Mode() & os.ModeSymlink) == os.ModeSymlink:
@@ -185,16 +207,51 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er
185207
return err
186208
}
187209
}
188-
return dr.Err()
210+
if err := dr.Err(); err != nil {
211+
return err
212+
}
213+
214+
if o.dirSync {
215+
if err := syncDirectory(dst); err != nil {
216+
return err
217+
}
218+
}
219+
220+
return nil
221+
}
222+
223+
func copyFileWithOpts(target, source string, o *copyDirOpts) error {
224+
if o.fileSync {
225+
return CopyFile(target, source, WithFileSync())
226+
}
227+
return CopyFile(target, source)
228+
}
229+
230+
type CopyFileOpt func(*copyFileConfig)
231+
232+
type copyFileConfig struct {
233+
sync bool
234+
}
235+
236+
// WithFileSync ensures the copied file is fsynced to persistent storage before returning.
237+
// By default, files are not synced to avoid unnecessary I/O latency.
238+
func WithFileSync() CopyFileOpt {
239+
return func(c *copyFileConfig) {
240+
c.sync = true
241+
}
189242
}
190243

191244
// CopyFile copies the source file to the target.
192245
// The most efficient means of copying is used for the platform.
193-
func CopyFile(target, source string) error {
194-
return copyFile(target, source)
246+
func CopyFile(target, source string, opts ...CopyFileOpt) error {
247+
var cfg copyFileConfig
248+
for _, o := range opts {
249+
o(&cfg)
250+
}
251+
return copyFile(target, source, cfg.sync)
195252
}
196253

197-
func openAndCopyFile(target, source string) error {
254+
func openAndCopyFile(target, source string, sync bool) error {
198255
src, err := os.Open(source)
199256
if err != nil {
200257
return fmt.Errorf("failed to open source %s: %w", source, err)
@@ -206,6 +263,13 @@ func openAndCopyFile(target, source string) error {
206263
}
207264
defer tgt.Close()
208265

209-
_, err = io.Copy(tgt, src)
210-
return err
266+
if _, err = io.Copy(tgt, src); err != nil {
267+
return err
268+
}
269+
if sync {
270+
if err := tgt.Sync(); err != nil {
271+
return fmt.Errorf("failed to sync target %s: %w", target, err)
272+
}
273+
}
274+
return nil
211275
}

fs/copy_darwin.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ import (
2323
"golang.org/x/sys/unix"
2424
)
2525

26-
func copyFile(target, source string) error {
26+
func copyFile(target, source string, sync bool) error {
2727
if err := unix.Clonefile(source, target, unix.CLONE_NOFOLLOW); err != nil {
2828
if !errors.Is(err, unix.ENOTSUP) && !errors.Is(err, unix.EXDEV) {
2929
return fmt.Errorf("clonefile failed: %w", err)
3030
}
3131

32-
return openAndCopyFile(target, source)
32+
return openAndCopyFile(target, source, sync)
3333
}
3434
return nil
3535
}

fs/copy_linux.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const maxCopyChunk = 1 << 30 // 1 GiB
3434
//
3535
// If the filesystem does not support SEEK_DATA/SEEK_HOLE, it falls back
3636
// to a plain io.Copy.
37-
func copyFile(target, source string) error {
37+
func copyFile(target, source string, sync bool) error {
3838
src, err := os.Open(source)
3939
if err != nil {
4040
return fmt.Errorf("failed to open source %s: %w", source, err)
@@ -72,7 +72,7 @@ func copyFile(target, source string) error {
7272
// Filesystem doesn't support SEEK_DATA/SEEK_HOLE. Fall back to a plain copy.
7373
src.Close()
7474
tgt.Close()
75-
return openAndCopyFile(target, source)
75+
return openAndCopyFile(target, source, sync)
7676
}
7777

7878
return fmt.Errorf("failed to seek data in source %s: %w", source, err)
@@ -122,7 +122,7 @@ func copyFile(target, source string) error {
122122
if errors.Is(err, syscall.EXDEV) || errors.Is(err, syscall.ENOSYS) || errors.Is(err, syscall.EOPNOTSUPP) {
123123
src.Close()
124124
tgt.Close()
125-
return openAndCopyFile(target, source)
125+
return openAndCopyFile(target, source, sync)
126126
}
127127
return fmt.Errorf("copy_file_range failed: %w", err)
128128
}
@@ -135,8 +135,10 @@ func copyFile(target, source string) error {
135135
offset = holeStart
136136
}
137137

138-
if err := tgt.Sync(); err != nil {
139-
return fmt.Errorf("failed to sync target %s: %w", target, err)
138+
if sync {
139+
if err := tgt.Sync(); err != nil {
140+
return fmt.Errorf("failed to sync target %s: %w", target, err)
141+
}
140142
}
141143

142144
return nil

fs/copy_linux_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,81 @@ func TestCopyFileSparse(t *testing.T) {
128128
}
129129
}
130130

131+
func TestCopyFileSparseWithSync(t *testing.T) {
132+
dir := t.TempDir()
133+
134+
type testCase struct {
135+
name string
136+
parts []int64
137+
}
138+
139+
tests := []testCase{
140+
{
141+
name: "DataHoleData",
142+
parts: []int64{4096, 1024 * 1024, 4096},
143+
},
144+
{
145+
name: "NoHoles",
146+
parts: []int64{64 * 1024},
147+
},
148+
}
149+
150+
for _, tc := range tests {
151+
t.Run(tc.name, func(t *testing.T) {
152+
srcPath := filepath.Join(dir, tc.name+"-sync-src")
153+
dstPath := filepath.Join(dir, tc.name+"-sync-dst")
154+
155+
applier := createSparseFile(tc.name+"-sync-src", 42, 0o644, tc.parts...)
156+
if err := applier.Apply(dir); err != nil {
157+
t.Fatal(err)
158+
}
159+
160+
if err := CopyFile(dstPath, srcPath, WithFileSync()); err != nil {
161+
t.Fatalf("CopyFile with WithFileSync failed: %v", err)
162+
}
163+
164+
// Verify content matches exactly.
165+
srcData, err := os.ReadFile(srcPath)
166+
if err != nil {
167+
t.Fatal(err)
168+
}
169+
dstData, err := os.ReadFile(dstPath)
170+
if err != nil {
171+
t.Fatal(err)
172+
}
173+
if !bytes.Equal(srcData, dstData) {
174+
t.Fatal("source and destination file contents differ")
175+
}
176+
177+
// Verify sparseness is preserved.
178+
srcStat, err := os.Stat(srcPath)
179+
if err != nil {
180+
t.Fatal(err)
181+
}
182+
dstStat, err := os.Stat(dstPath)
183+
if err != nil {
184+
t.Fatal(err)
185+
}
186+
187+
srcBlocks := srcStat.Sys().(*syscall.Stat_t).Blocks
188+
dstBlocks := dstStat.Sys().(*syscall.Stat_t).Blocks
189+
190+
t.Logf("src size=%d blocks=%d, dst size=%d blocks=%d",
191+
srcStat.Size(), srcBlocks, dstStat.Size(), dstBlocks)
192+
193+
if srcStat.Size() != dstStat.Size() {
194+
t.Fatalf("size mismatch: src=%d dst=%d", srcStat.Size(), dstStat.Size())
195+
}
196+
197+
maxBlocks := srcBlocks + srcBlocks/10 + 8
198+
if dstBlocks > maxBlocks {
199+
t.Fatalf("destination is not sparse: src blocks=%d, dst blocks=%d (max allowed=%d)",
200+
srcBlocks, dstBlocks, maxBlocks)
201+
}
202+
})
203+
}
204+
}
205+
131206
func TestCopyReflinkWithXFS(t *testing.T) {
132207
testutil.RequiresRoot(t)
133208
mnt := t.TempDir()

fs/copy_nondarwin.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,6 @@
1818

1919
package fs
2020

21-
var copyFile = openAndCopyFile
21+
func copyFile(target, source string, sync bool) error {
22+
return openAndCopyFile(target, source, sync)
23+
}

0 commit comments

Comments
 (0)