diff --git a/changelog/unreleased/issue-5063 b/changelog/unreleased/issue-5063 new file mode 100644 index 000000000..95048ec58 --- /dev/null +++ b/changelog/unreleased/issue-5063 @@ -0,0 +1,10 @@ +Bugfix: Correctly `backup` extended metadata when using VSS on Windows + +On Windows, when creating a backup using the `--use-fs-snapshot` option, +then the extended metadata was not read from the filesystem snapshot. This +could result in errors when files have been removed in the meantime. + +This issue has been resolved. + +https://github.com/restic/restic/issues/5063 +https://github.com/restic/restic/pull/5097 diff --git a/cmd/restic/cmd_backup.go b/cmd/restic/cmd_backup.go index 107e8bbe0..b7eed1318 100644 --- a/cmd/restic/cmd_backup.go +++ b/cmd/restic/cmd_backup.go @@ -97,6 +97,7 @@ type BackupOptions struct { } var backupOptions BackupOptions +var backupFSTestHook func(fs fs.FS) fs.FS // ErrInvalidSourceData is used to report an incomplete backup var ErrInvalidSourceData = errors.New("at least one source file could not be read") @@ -582,6 +583,10 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter targets = []string{filename} } + if backupFSTestHook != nil { + targetFS = backupFSTestHook(targetFS) + } + // rejectFuncs collect functions that can reject items from the backup based on path and file info rejectFuncs, err := collectRejectFuncs(opts, targets, targetFS) if err != nil { diff --git a/cmd/restic/cmd_backup_integration_test.go b/cmd/restic/cmd_backup_integration_test.go index 5e00b84b0..5926fdd54 100644 --- a/cmd/restic/cmd_backup_integration_test.go +++ b/cmd/restic/cmd_backup_integration_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "runtime" "testing" + "time" "github.com/restic/restic/internal/fs" "github.com/restic/restic/internal/restic" @@ -111,6 +112,63 @@ func TestBackupWithRelativePath(t *testing.T) { rtest.Assert(t, latestSn.Parent != nil && latestSn.Parent.Equal(firstSnapshotID), "second snapshot selected unexpected parent %v instead of %v", latestSn.Parent, firstSnapshotID) } +type vssDeleteOriginalFS struct { + fs.FS + testdata string + hasRemoved bool +} + +func (f *vssDeleteOriginalFS) Lstat(name string) (os.FileInfo, error) { + if !f.hasRemoved { + // call Lstat to trigger snapshot creation + _, _ = f.FS.Lstat(name) + // nuke testdata + var err error + for i := 0; i < 3; i++ { + // The CI sometimes runs into "The process cannot access the file because it is being used by another process" errors + // thus try a few times to remove the data + err = os.RemoveAll(f.testdata) + if err == nil { + break + } + time.Sleep(10 * time.Millisecond) + } + if err != nil { + return nil, err + } + f.hasRemoved = true + } + return f.FS.Lstat(name) +} + +func TestBackupVSS(t *testing.T) { + if runtime.GOOS != "windows" || fs.HasSufficientPrivilegesForVSS() != nil { + t.Skip("vss fs test can only be run on windows with admin privileges") + } + + env, cleanup := withTestEnvironment(t) + defer cleanup() + + testSetupBackupData(t, env) + opts := BackupOptions{UseFsSnapshot: true} + + var testFS *vssDeleteOriginalFS + backupFSTestHook = func(fs fs.FS) fs.FS { + testFS = &vssDeleteOriginalFS{ + FS: fs, + testdata: env.testdata, + } + return testFS + } + defer func() { + backupFSTestHook = nil + }() + + testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata"}, opts, env.gopts) + testListSnapshots(t, env.gopts, 1) + rtest.Equals(t, true, testFS.hasRemoved, "testdata was not removed") +} + func TestBackupParentSelection(t *testing.T) { env, cleanup := withTestEnvironment(t) defer cleanup() diff --git a/cmd/restic/integration_helpers_test.go b/cmd/restic/integration_helpers_test.go index 978deab3d..8ae3bb78a 100644 --- a/cmd/restic/integration_helpers_test.go +++ b/cmd/restic/integration_helpers_test.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "sync" "testing" @@ -168,6 +169,16 @@ type testEnvironment struct { gopts GlobalOptions } +type logOutputter struct { + t testing.TB +} + +func (l *logOutputter) Write(p []byte) (n int, err error) { + l.t.Helper() + l.t.Log(strings.TrimSuffix(string(p), "\n")) + return len(p), nil +} + // withTestEnvironment creates a test environment and returns a cleanup // function which removes it. func withTestEnvironment(t testing.TB) (env *testEnvironment, cleanup func()) { @@ -200,8 +211,11 @@ func withTestEnvironment(t testing.TB) (env *testEnvironment, cleanup func()) { Quiet: true, CacheDir: env.cache, password: rtest.TestPassword, - stdout: os.Stdout, - stderr: os.Stderr, + // stdout and stderr are written to by Warnf etc. That is the written data + // usually consists of one or multiple lines and therefore can be handled well + // by t.Log. + stdout: &logOutputter{t}, + stderr: &logOutputter{t}, extended: make(options.Options), // replace this hook with "nil" if listing a filetype more than once is necessary diff --git a/internal/archiver/file_saver.go b/internal/archiver/file_saver.go index b9d07434a..dccaa9442 100644 --- a/internal/archiver/file_saver.go +++ b/internal/archiver/file_saver.go @@ -156,7 +156,7 @@ func (s *fileSaver) saveFile(ctx context.Context, chnker *chunker.Chunker, snPat debug.Log("%v", snPath) - node, err := s.NodeFromFileInfo(snPath, f.Name(), fi, false) + node, err := s.NodeFromFileInfo(snPath, target, fi, false) if err != nil { _ = f.Close() completeError(err) diff --git a/internal/fs/fs_local_vss.go b/internal/fs/fs_local_vss.go index 908e744ee..1915e2a7c 100644 --- a/internal/fs/fs_local_vss.go +++ b/internal/fs/fs_local_vss.go @@ -10,6 +10,7 @@ import ( "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/options" + "github.com/restic/restic/internal/restic" ) // VSSConfig holds extended options of windows volume shadow copy service. @@ -127,17 +128,21 @@ func (fs *LocalVss) DeleteSnapshots() { // OpenFile wraps the Open method of the underlying file system. func (fs *LocalVss) OpenFile(name string, flag int, perm os.FileMode) (File, error) { - return os.OpenFile(fs.snapshotPath(name), flag, perm) + return fs.FS.OpenFile(fs.snapshotPath(name), flag, perm) } // Stat wraps the Stat method of the underlying file system. func (fs *LocalVss) Stat(name string) (os.FileInfo, error) { - return os.Stat(fs.snapshotPath(name)) + return fs.FS.Stat(fs.snapshotPath(name)) } // Lstat wraps the Lstat method of the underlying file system. func (fs *LocalVss) Lstat(name string) (os.FileInfo, error) { - return os.Lstat(fs.snapshotPath(name)) + return fs.FS.Lstat(fs.snapshotPath(name)) +} + +func (fs *LocalVss) NodeFromFileInfo(path string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error) { + return fs.FS.NodeFromFileInfo(fs.snapshotPath(path), fi, ignoreXattrListError) } // isMountPointIncluded is true if given mountpoint included by user. diff --git a/internal/fs/fs_local_vss_test.go b/internal/fs/fs_local_vss_test.go index a59882381..f1a043118 100644 --- a/internal/fs/fs_local_vss_test.go +++ b/internal/fs/fs_local_vss_test.go @@ -5,13 +5,18 @@ package fs import ( "fmt" + "io" + "os" + "path/filepath" "regexp" + "runtime" "strings" "testing" "time" ole "github.com/go-ole/go-ole" "github.com/restic/restic/internal/options" + rtest "github.com/restic/restic/internal/test" ) func matchStrings(ptrs []string, strs []string) bool { @@ -284,3 +289,56 @@ func TestParseProvider(t *testing.T) { }) } } + +func TestVSSFS(t *testing.T) { + if runtime.GOOS != "windows" || HasSufficientPrivilegesForVSS() != nil { + t.Skip("vss fs test can only be run on windows with admin privileges") + } + + cfg, err := ParseVSSConfig(options.Options{}) + rtest.OK(t, err) + + errorHandler := func(item string, err error) { + t.Fatalf("unexpected error (%v)", err) + } + messageHandler := func(msg string, args ...interface{}) { + if strings.HasPrefix(msg, "creating VSS snapshot for") || strings.HasPrefix(msg, "successfully created snapshot") { + return + } + t.Fatalf("unexpected message (%s)", fmt.Sprintf(msg, args)) + } + + localVss := NewLocalVss(errorHandler, messageHandler, cfg) + defer localVss.DeleteSnapshots() + + tempdir := t.TempDir() + tempfile := filepath.Join(tempdir, "file") + rtest.OK(t, os.WriteFile(tempfile, []byte("example"), 0o600)) + + // trigger snapshot creation and + // capture FI while file still exists (should already be within the snapshot) + origFi, err := localVss.Stat(tempfile) + rtest.OK(t, err) + + // remove original file + rtest.OK(t, os.Remove(tempfile)) + + statFi, err := localVss.Stat(tempfile) + rtest.OK(t, err) + rtest.Equals(t, origFi.Mode(), statFi.Mode()) + + lstatFi, err := localVss.Lstat(tempfile) + rtest.OK(t, err) + rtest.Equals(t, origFi.Mode(), lstatFi.Mode()) + + f, err := localVss.OpenFile(tempfile, os.O_RDONLY, 0) + rtest.OK(t, err) + data, err := io.ReadAll(f) + rtest.OK(t, err) + rtest.Equals(t, "example", string(data), "unexpected file content") + rtest.OK(t, f.Close()) + + node, err := localVss.NodeFromFileInfo(tempfile, statFi, false) + rtest.OK(t, err) + rtest.Equals(t, node.Mode, statFi.Mode()) +} diff --git a/internal/fs/fs_reader.go b/internal/fs/fs_reader.go index c2bf23bb7..97d4e1660 100644 --- a/internal/fs/fs_reader.go +++ b/internal/fs/fs_reader.go @@ -232,7 +232,7 @@ func (r *readerFile) Close() error { var _ File = &readerFile{} // fakeFile implements all File methods, but only returns errors for anything -// except Stat() and Name(). +// except Stat() type fakeFile struct { name string os.FileInfo @@ -257,10 +257,6 @@ func (f fakeFile) Stat() (os.FileInfo, error) { return f.FileInfo, nil } -func (f fakeFile) Name() string { - return f.name -} - // fakeDir implements Readdirnames and Readdir, everything else is delegated to fakeFile. type fakeDir struct { entries []os.FileInfo diff --git a/internal/fs/interface.go b/internal/fs/interface.go index 0bb3029dc..2967429c0 100644 --- a/internal/fs/interface.go +++ b/internal/fs/interface.go @@ -34,5 +34,4 @@ type File interface { Readdirnames(n int) ([]string, error) Stat() (os.FileInfo, error) - Name() string }