From f77e67086c1e47a2b99dbc81651624039875dcd0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 31 Oct 2024 21:53:08 +0100 Subject: [PATCH 1/5] fs: add correct vss support to fixpath Paths that only contain the volume shadow copy snapshot name require special treatment. These paths must end with a slash for regular file operations to work. --- internal/fs/file_windows.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/fs/file_windows.go b/internal/fs/file_windows.go index 50c7e9938..31d495509 100644 --- a/internal/fs/file_windows.go +++ b/internal/fs/file_windows.go @@ -20,6 +20,15 @@ func fixpath(name string) string { if strings.HasPrefix(abspath, `\\?\UNC\`) { return abspath } + // Check if \\?\GLOBALROOT exists which marks volume shadow copy snapshots + if strings.HasPrefix(abspath, `\\?\GLOBALROOT\`) { + if strings.Count(abspath, `\`) == 5 { + // Append slash if this just a volume name, e.g. `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopyXX` + // Without the trailing slash any access to the volume itself will fail. + return abspath + string(filepath.Separator) + } + return abspath + } // Check if \\?\ already exist if strings.HasPrefix(abspath, `\\?\`) { return abspath From e38f6794cde01f2950d900122048f4218eaedd33 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 31 Oct 2024 19:10:01 +0100 Subject: [PATCH 2/5] restic: fix error in fillGenericAttributes for vss volumes Extended attributes and security descriptors apparently cannot be retrieved from a vss volume. Fix the volume check to correctly detect vss volumes and just completely disable extended attributes for volumes. --- internal/restic/node_windows.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/internal/restic/node_windows.go b/internal/restic/node_windows.go index bce01ccad..722ef09db 100644 --- a/internal/restic/node_windows.go +++ b/internal/restic/node_windows.go @@ -372,8 +372,11 @@ func (node *Node) fillGenericAttributes(path string, fi os.FileInfo, stat *statT return false, nil } - if strings.HasSuffix(filepath.Clean(path), `\`) { - // filepath.Clean(path) ends with '\' for Windows root volume paths only + isVolume, err := isVolumePath(path) + if err != nil { + return false, err + } + if isVolume { // Do not process file attributes, created time and sd for windows root volume paths // Security descriptors are not supported for root volume paths. // Though file attributes and created time are supported for root volume paths, @@ -464,6 +467,18 @@ func checkAndStoreEASupport(path string) (isEASupportedVolume bool, err error) { return isEASupportedVolume, err } +// isVolumePath returns whether a path refers to a volume +func isVolumePath(path string) (bool, error) { + volName, err := prepareVolumeName(path) + if err != nil { + return false, err + } + + cleanPath := filepath.Clean(path) + cleanVolume := filepath.Clean(volName + `\`) + return cleanPath == cleanVolume, nil +} + // prepareVolumeName prepares the volume name for different cases in Windows func prepareVolumeName(path string) (volumeName string, err error) { // Check if it's an extended length path From 4380627cb7fe72fe3f909d62e8d11406ee147425 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 31 Oct 2024 19:30:21 +0100 Subject: [PATCH 3/5] backup: run test with absolute path --- cmd/restic/cmd_backup_integration_test.go | 24 +++++++++++++++++----- cmd/restic/cmd_copy_integration_test.go | 4 ++-- cmd/restic/cmd_restore_integration_test.go | 12 +++++------ cmd/restic/integration_test.go | 2 +- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/cmd/restic/cmd_backup_integration_test.go b/cmd/restic/cmd_backup_integration_test.go index 5926fdd54..0cdf8e5b4 100644 --- a/cmd/restic/cmd_backup_integration_test.go +++ b/cmd/restic/cmd_backup_integration_test.go @@ -52,14 +52,14 @@ func testBackup(t *testing.T, useFsSnapshot bool) { opts := BackupOptions{UseFsSnapshot: useFsSnapshot} // first backup - testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata"}, opts, env.gopts) + testRunBackup(t, "", []string{env.testdata}, opts, env.gopts) testListSnapshots(t, env.gopts, 1) testRunCheck(t, env.gopts) stat1 := dirStats(env.repo) // second backup, implicit incremental - testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata"}, opts, env.gopts) + testRunBackup(t, "", []string{env.testdata}, opts, env.gopts) snapshotIDs := testListSnapshots(t, env.gopts, 2) stat2 := dirStats(env.repo) @@ -71,7 +71,7 @@ func testBackup(t *testing.T, useFsSnapshot bool) { testRunCheck(t, env.gopts) // third backup, explicit incremental opts.Parent = snapshotIDs[0].String() - testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata"}, opts, env.gopts) + testRunBackup(t, "", []string{env.testdata}, opts, env.gopts) snapshotIDs = testListSnapshots(t, env.gopts, 3) stat3 := dirStats(env.repo) @@ -84,7 +84,7 @@ func testBackup(t *testing.T, useFsSnapshot bool) { for i, snapshotID := range snapshotIDs { restoredir := filepath.Join(env.base, fmt.Sprintf("restore%d", i)) t.Logf("restoring snapshot %v to %v", snapshotID.Str(), restoredir) - testRunRestore(t, env.gopts, restoredir, snapshotID) + testRunRestore(t, env.gopts, restoredir, snapshotID.String()+":"+toPathInSnapshot(filepath.Dir(env.testdata))) diff := directoriesContentsDiff(env.testdata, filepath.Join(restoredir, "testdata")) rtest.Assert(t, diff == "", "directories are not equal: %v", diff) } @@ -92,6 +92,20 @@ func testBackup(t *testing.T, useFsSnapshot bool) { testRunCheck(t, env.gopts) } +func toPathInSnapshot(path string) string { + // use path as is on most platforms, but convert it on windows + if runtime.GOOS == "windows" { + // the path generated by the test is always local so take the shortcut + vol := filepath.VolumeName(path) + if vol[len(vol)-1] != ':' { + panic(fmt.Sprintf("unexpected path: %q", path)) + } + path = vol[:len(vol)-1] + string(filepath.Separator) + path[len(vol)+1:] + path = filepath.ToSlash(path) + } + return path +} + func TestBackupWithRelativePath(t *testing.T) { env, cleanup := withTestEnvironment(t) defer cleanup() @@ -557,7 +571,7 @@ func TestHardLink(t *testing.T) { for i, snapshotID := range snapshotIDs { restoredir := filepath.Join(env.base, fmt.Sprintf("restore%d", i)) t.Logf("restoring snapshot %v to %v", snapshotID.Str(), restoredir) - testRunRestore(t, env.gopts, restoredir, snapshotID) + testRunRestore(t, env.gopts, restoredir, snapshotID.String()) diff := directoriesContentsDiff(env.testdata, filepath.Join(restoredir, "testdata")) rtest.Assert(t, diff == "", "directories are not equal %v", diff) diff --git a/cmd/restic/cmd_copy_integration_test.go b/cmd/restic/cmd_copy_integration_test.go index 704615870..9ae78ba50 100644 --- a/cmd/restic/cmd_copy_integration_test.go +++ b/cmd/restic/cmd_copy_integration_test.go @@ -62,11 +62,11 @@ func TestCopy(t *testing.T) { for i, snapshotID := range snapshotIDs { restoredir := filepath.Join(env.base, fmt.Sprintf("restore%d", i)) origRestores[restoredir] = struct{}{} - testRunRestore(t, env.gopts, restoredir, snapshotID) + testRunRestore(t, env.gopts, restoredir, snapshotID.String()) } for i, snapshotID := range copiedSnapshotIDs { restoredir := filepath.Join(env2.base, fmt.Sprintf("restore%d", i)) - testRunRestore(t, env2.gopts, restoredir, snapshotID) + testRunRestore(t, env2.gopts, restoredir, snapshotID.String()) foundMatch := false for cmpdir := range origRestores { diff := directoriesContentsDiff(restoredir, cmpdir) diff --git a/cmd/restic/cmd_restore_integration_test.go b/cmd/restic/cmd_restore_integration_test.go index b0543850b..f876bfae1 100644 --- a/cmd/restic/cmd_restore_integration_test.go +++ b/cmd/restic/cmd_restore_integration_test.go @@ -18,17 +18,17 @@ import ( "github.com/restic/restic/internal/ui/termstatus" ) -func testRunRestore(t testing.TB, opts GlobalOptions, dir string, snapshotID restic.ID) { +func testRunRestore(t testing.TB, opts GlobalOptions, dir string, snapshotID string) { testRunRestoreExcludes(t, opts, dir, snapshotID, nil) } -func testRunRestoreExcludes(t testing.TB, gopts GlobalOptions, dir string, snapshotID restic.ID, excludes []string) { +func testRunRestoreExcludes(t testing.TB, gopts GlobalOptions, dir string, snapshotID string, excludes []string) { opts := RestoreOptions{ Target: dir, } opts.Excludes = excludes - rtest.OK(t, testRunRestoreAssumeFailure(snapshotID.String(), opts, gopts)) + rtest.OK(t, testRunRestoreAssumeFailure(snapshotID, opts, gopts)) } func testRunRestoreAssumeFailure(snapshotID string, opts RestoreOptions, gopts GlobalOptions) error { @@ -198,7 +198,7 @@ func TestRestoreFilter(t *testing.T) { snapshotID := testListSnapshots(t, env.gopts, 1)[0] // no restore filter should restore all files - testRunRestore(t, env.gopts, filepath.Join(env.base, "restore0"), snapshotID) + testRunRestore(t, env.gopts, filepath.Join(env.base, "restore0"), snapshotID.String()) for _, testFile := range testfiles { rtest.OK(t, testFileSize(filepath.Join(env.base, "restore0", "testdata", testFile.name), int64(testFile.size))) } @@ -220,7 +220,7 @@ func TestRestoreFilter(t *testing.T) { // restore with excludes restoredir := filepath.Join(env.base, "restore-with-excludes") - testRunRestoreExcludes(t, env.gopts, restoredir, snapshotID, excludePatterns) + testRunRestoreExcludes(t, env.gopts, restoredir, snapshotID.String(), excludePatterns) testRestoredFileExclusions(t, restoredir) // Create an exclude file with some patterns @@ -340,7 +340,7 @@ func TestRestoreWithPermissionFailure(t *testing.T) { _ = withRestoreGlobalOptions(func() error { globalOptions.stderr = io.Discard - testRunRestore(t, env.gopts, filepath.Join(env.base, "restore"), snapshots[0]) + testRunRestore(t, env.gopts, filepath.Join(env.base, "restore"), snapshots[0].String()) return nil }) diff --git a/cmd/restic/integration_test.go b/cmd/restic/integration_test.go index df95031dc..d39ea6980 100644 --- a/cmd/restic/integration_test.go +++ b/cmd/restic/integration_test.go @@ -35,7 +35,7 @@ func TestCheckRestoreNoLock(t *testing.T) { testRunCheck(t, env.gopts) snapshotIDs := testListSnapshots(t, env.gopts, 4) - testRunRestore(t, env.gopts, filepath.Join(env.base, "restore"), snapshotIDs[0]) + testRunRestore(t, env.gopts, filepath.Join(env.base, "restore"), snapshotIDs[0].String()) } // a listOnceBackend only allows listing once per filetype From 0aee70b496dc9979902363189763330263b6dd30 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 31 Oct 2024 19:32:51 +0100 Subject: [PATCH 4/5] restic: test path handling of volume shadow copy root path --- internal/restic/node_windows_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/restic/node_windows_test.go b/internal/restic/node_windows_test.go index 6ba25559b..c3936cfc8 100644 --- a/internal/restic/node_windows_test.go +++ b/internal/restic/node_windows_test.go @@ -450,6 +450,13 @@ func TestPrepareVolumeName(t *testing.T) { expectError: false, expectedEASupported: false, }, + { + name: "Volume Shadow Copy root", + path: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy1`, + expectedVolume: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy1`, + expectError: false, + expectedEASupported: false, + }, { name: "Volume Shadow Copy path", path: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy1\Users\test`, From 962279479d85174e92698a144f30879d1c7f07d2 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 31 Oct 2024 20:01:52 +0100 Subject: [PATCH 5/5] add vss metadata changelog --- changelog/unreleased/issue-5107 | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 changelog/unreleased/issue-5107 diff --git a/changelog/unreleased/issue-5107 b/changelog/unreleased/issue-5107 new file mode 100644 index 000000000..13bb380e4 --- /dev/null +++ b/changelog/unreleased/issue-5107 @@ -0,0 +1,15 @@ +Bugfix: Fix metadata error on Windows for backups using VSS + +Since restic 0.17.2, when creating a backup on Windows using `--use-fs-snapshot`, +restic would report an error like the following: + +``` +error: incomplete metadata for C:\: get EA failed while opening file handle for path \\?\GLOBALROOT\Device\HarddiskVolumeShadowCopyXX\, with: The process cannot access the file because it is being used by another process. +``` + +This has now been fixed by correctly handling paths that refer to volume +shadow copy snapshots. + +https://github.com/restic/restic/issues/5107 +https://github.com/restic/restic/pull/5110 +https://github.com/restic/restic/pull/5112