From f77e67086c1e47a2b99dbc81651624039875dcd0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 31 Oct 2024 21:53:08 +0100 Subject: [PATCH 01/22] 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 02/22] 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 03/22] 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 04/22] 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 05/22] 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 From d8e03849408c73518187f90652bb3970fc0cb3ad Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 17 Oct 2024 19:45:03 +0200 Subject: [PATCH 06/22] doc: document safety feature for --target / --delete --- doc/050_restore.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/050_restore.rst b/doc/050_restore.rst index 1a920fad4..9558ab1d4 100644 --- a/doc/050_restore.rst +++ b/doc/050_restore.rst @@ -132,6 +132,10 @@ options will be deleted. For example, the command ``restic -r /srv/restic-repo restore 79766175:/work --target /tmp/restore-work --include /foo --delete`` would only delete files within ``/tmp/restore-work/foo``. +When using ``--target / --delete`` then the ``restore`` command only works if either an ``--include`` +or ``--exclude`` option is also specified. This ensures that one cannot accidentaly delete +the whole system. + Dry run ------- From 75ec7d32690234271375004cf76a6c75050a0a2f Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 9 Sep 2024 22:15:30 +0200 Subject: [PATCH 07/22] fuse: cache fs.Node instances A particular node should always be represented by a single instance. This is necessary to allow the fuse library to assign a stable nodeId to a node. macOS Sonoma trips over the previous, unstable behavior when using fuse-t. --- internal/fuse/dir.go | 43 +++++++++++++++++++--------------- internal/fuse/snapshots_dir.go | 14 +++++++---- internal/fuse/tree_cache.go | 38 ++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 24 deletions(-) create mode 100644 internal/fuse/tree_cache.go diff --git a/internal/fuse/dir.go b/internal/fuse/dir.go index fd030295b..e87e01247 100644 --- a/internal/fuse/dir.go +++ b/internal/fuse/dir.go @@ -29,6 +29,7 @@ type dir struct { parentInode uint64 node *restic.Node m sync.Mutex + cache treeCache } func cleanupNodeName(name string) string { @@ -43,6 +44,7 @@ func newDir(root *Root, inode, parentInode uint64, node *restic.Node) (*dir, err node: node, inode: inode, parentInode: parentInode, + cache: *newTreeCache(), }, nil } @@ -87,6 +89,7 @@ func newDirFromSnapshot(root *Root, inode uint64, snapshot *restic.Snapshot) (*d Subtree: snapshot.Tree, }, inode: inode, + cache: *newTreeCache(), }, nil } @@ -208,25 +211,27 @@ func (d *dir) Lookup(ctx context.Context, name string) (fs.Node, error) { return nil, err } - node, ok := d.items[name] - if !ok { - debug.Log(" Lookup(%v) -> not found", name) - return nil, syscall.ENOENT - } - inode := inodeFromNode(d.inode, node) - switch node.Type { - case "dir": - return newDir(d.root, inode, d.inode, node) - case "file": - return newFile(d.root, inode, node) - case "symlink": - return newLink(d.root, inode, node) - case "dev", "chardev", "fifo", "socket": - return newOther(d.root, inode, node) - default: - debug.Log(" node %v has unknown type %v", name, node.Type) - return nil, syscall.ENOENT - } + return d.cache.lookupOrCreate(name, func() (fs.Node, error) { + node, ok := d.items[name] + if !ok { + debug.Log(" Lookup(%v) -> not found", name) + return nil, syscall.ENOENT + } + inode := inodeFromNode(d.inode, node) + switch node.Type { + case "dir": + return newDir(d.root, inode, d.inode, node) + case "file": + return newFile(d.root, inode, node) + case "symlink": + return newLink(d.root, inode, node) + case "dev", "chardev", "fifo", "socket": + return newOther(d.root, inode, node) + default: + debug.Log(" node %v has unknown type %v", name, node.Type) + return nil, syscall.ENOENT + } + }) } func (d *dir) Listxattr(_ context.Context, req *fuse.ListxattrRequest, resp *fuse.ListxattrResponse) error { diff --git a/internal/fuse/snapshots_dir.go b/internal/fuse/snapshots_dir.go index 4cae7106c..cfe1f782a 100644 --- a/internal/fuse/snapshots_dir.go +++ b/internal/fuse/snapshots_dir.go @@ -23,6 +23,7 @@ type SnapshotsDir struct { parentInode uint64 dirStruct *SnapshotsDirStructure prefix string + cache treeCache } // ensure that *SnapshotsDir implements these interfaces @@ -38,6 +39,7 @@ func NewSnapshotsDir(root *Root, inode, parentInode uint64, dirStruct *Snapshots parentInode: parentInode, dirStruct: dirStruct, prefix: prefix, + cache: *newTreeCache(), } } @@ -107,8 +109,12 @@ func (d *SnapshotsDir) Lookup(ctx context.Context, name string) (fs.Node, error) return nil, syscall.ENOENT } - entry := meta.names[name] - if entry != nil { + return d.cache.lookupOrCreate(name, func() (fs.Node, error) { + entry := meta.names[name] + if entry == nil { + return nil, syscall.ENOENT + } + inode := inodeFromName(d.inode, name) if entry.linkTarget != "" { return newSnapshotLink(d.root, inode, entry.linkTarget, entry.snapshot) @@ -116,9 +122,7 @@ func (d *SnapshotsDir) Lookup(ctx context.Context, name string) (fs.Node, error) return newDirFromSnapshot(d.root, inode, entry.snapshot) } return NewSnapshotsDir(d.root, inode, d.inode, d.dirStruct, d.prefix+"/"+name), nil - } - - return nil, syscall.ENOENT + }) } // SnapshotLink diff --git a/internal/fuse/tree_cache.go b/internal/fuse/tree_cache.go new file mode 100644 index 000000000..addc54a46 --- /dev/null +++ b/internal/fuse/tree_cache.go @@ -0,0 +1,38 @@ +//go:build darwin || freebsd || linux +// +build darwin freebsd linux + +package fuse + +import ( + "sync" + + "github.com/anacrolix/fuse/fs" +) + +type treeCache struct { + nodes map[string]fs.Node + m sync.Mutex +} + +func newTreeCache() *treeCache { + return &treeCache{ + nodes: map[string]fs.Node{}, + } +} + +func (t *treeCache) lookupOrCreate(name string, create func() (fs.Node, error)) (fs.Node, error) { + t.m.Lock() + defer t.m.Unlock() + + if node, ok := t.nodes[name]; ok { + return node, nil + } + + node, err := create() + if err != nil { + return nil, err + } + + t.nodes[name] = node + return node, nil +} From de4f8b344ea0a17b883ac23496a99721eca42bf7 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 9 Sep 2024 22:37:08 +0200 Subject: [PATCH 08/22] fuse: add missing type assertion for optional interfaces --- internal/fuse/dir.go | 2 ++ internal/fuse/link.go | 2 ++ internal/fuse/other.go | 4 ++++ 3 files changed, 8 insertions(+) diff --git a/internal/fuse/dir.go b/internal/fuse/dir.go index e87e01247..49b32e21a 100644 --- a/internal/fuse/dir.go +++ b/internal/fuse/dir.go @@ -20,6 +20,8 @@ import ( // Statically ensure that *dir implement those interface var _ = fs.HandleReadDirAller(&dir{}) +var _ = fs.NodeGetxattrer(&dir{}) +var _ = fs.NodeListxattrer(&dir{}) var _ = fs.NodeStringLookuper(&dir{}) type dir struct { diff --git a/internal/fuse/link.go b/internal/fuse/link.go index 3aea8b06e..975e640ea 100644 --- a/internal/fuse/link.go +++ b/internal/fuse/link.go @@ -12,6 +12,8 @@ import ( ) // Statically ensure that *link implements the given interface +var _ = fs.NodeGetxattrer(&link{}) +var _ = fs.NodeListxattrer(&link{}) var _ = fs.NodeReadlinker(&link{}) type link struct { diff --git a/internal/fuse/other.go b/internal/fuse/other.go index f536de5c1..d459d0efd 100644 --- a/internal/fuse/other.go +++ b/internal/fuse/other.go @@ -7,9 +7,13 @@ import ( "context" "github.com/anacrolix/fuse" + "github.com/anacrolix/fuse/fs" "github.com/restic/restic/internal/restic" ) +// Statically ensure that *other implements the given interface +var _ = fs.NodeReadlinker(&other{}) + type other struct { root *Root node *restic.Node From 0e9716a6e61b5daf1a25cfed73997f13bbb0069e Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 9 Sep 2024 22:37:51 +0200 Subject: [PATCH 09/22] fuse: forget fs.Node instances on request by the kernel Forget fs.Node instances once the kernel frees the corresponding nodeId. This ensures that restic does not run out of memory on large snapshots. --- internal/fuse/dir.go | 24 ++++++++++++++++-------- internal/fuse/file.go | 23 +++++++++++++++-------- internal/fuse/fuse_test.go | 6 +++--- internal/fuse/link.go | 16 +++++++++++----- internal/fuse/other.go | 16 +++++++++++----- internal/fuse/root.go | 2 +- internal/fuse/snapshots_dir.go | 27 ++++++++++++++++++++------- internal/fuse/tree_cache.go | 11 +++++++++-- 8 files changed, 86 insertions(+), 39 deletions(-) diff --git a/internal/fuse/dir.go b/internal/fuse/dir.go index 49b32e21a..beb3420c7 100644 --- a/internal/fuse/dir.go +++ b/internal/fuse/dir.go @@ -20,12 +20,14 @@ import ( // Statically ensure that *dir implement those interface var _ = fs.HandleReadDirAller(&dir{}) +var _ = fs.NodeForgetter(&dir{}) var _ = fs.NodeGetxattrer(&dir{}) var _ = fs.NodeListxattrer(&dir{}) var _ = fs.NodeStringLookuper(&dir{}) type dir struct { root *Root + forget forgetFn items map[string]*restic.Node inode uint64 parentInode uint64 @@ -38,11 +40,12 @@ func cleanupNodeName(name string) string { return filepath.Base(name) } -func newDir(root *Root, inode, parentInode uint64, node *restic.Node) (*dir, error) { +func newDir(root *Root, forget forgetFn, inode, parentInode uint64, node *restic.Node) (*dir, error) { debug.Log("new dir for %v (%v)", node.Name, node.Subtree) return &dir{ root: root, + forget: forget, node: node, inode: inode, parentInode: parentInode, @@ -79,10 +82,11 @@ func replaceSpecialNodes(ctx context.Context, repo restic.BlobLoader, node *rest return tree.Nodes, nil } -func newDirFromSnapshot(root *Root, inode uint64, snapshot *restic.Snapshot) (*dir, error) { +func newDirFromSnapshot(root *Root, forget forgetFn, inode uint64, snapshot *restic.Snapshot) (*dir, error) { debug.Log("new dir for snapshot %v (%v)", snapshot.ID(), snapshot.Tree) return &dir{ - root: root, + root: root, + forget: forget, node: &restic.Node{ AccessTime: snapshot.Time, ModTime: snapshot.Time, @@ -213,7 +217,7 @@ func (d *dir) Lookup(ctx context.Context, name string) (fs.Node, error) { return nil, err } - return d.cache.lookupOrCreate(name, func() (fs.Node, error) { + return d.cache.lookupOrCreate(name, func(forget forgetFn) (fs.Node, error) { node, ok := d.items[name] if !ok { debug.Log(" Lookup(%v) -> not found", name) @@ -222,13 +226,13 @@ func (d *dir) Lookup(ctx context.Context, name string) (fs.Node, error) { inode := inodeFromNode(d.inode, node) switch node.Type { case "dir": - return newDir(d.root, inode, d.inode, node) + return newDir(d.root, forget, inode, d.inode, node) case "file": - return newFile(d.root, inode, node) + return newFile(d.root, forget, inode, node) case "symlink": - return newLink(d.root, inode, node) + return newLink(d.root, forget, inode, node) case "dev", "chardev", "fifo", "socket": - return newOther(d.root, inode, node) + return newOther(d.root, forget, inode, node) default: debug.Log(" node %v has unknown type %v", name, node.Type) return nil, syscall.ENOENT @@ -244,3 +248,7 @@ func (d *dir) Listxattr(_ context.Context, req *fuse.ListxattrRequest, resp *fus func (d *dir) Getxattr(_ context.Context, req *fuse.GetxattrRequest, resp *fuse.GetxattrResponse) error { return nodeGetXattr(d.node, req, resp) } + +func (d *dir) Forget() { + d.forget() +} diff --git a/internal/fuse/file.go b/internal/fuse/file.go index 494fca283..a69471f83 100644 --- a/internal/fuse/file.go +++ b/internal/fuse/file.go @@ -20,14 +20,16 @@ const blockSize = 512 // Statically ensure that *file and *openFile implement the given interfaces var _ = fs.HandleReader(&openFile{}) -var _ = fs.NodeListxattrer(&file{}) +var _ = fs.NodeForgetter(&file{}) var _ = fs.NodeGetxattrer(&file{}) +var _ = fs.NodeListxattrer(&file{}) var _ = fs.NodeOpener(&file{}) type file struct { - root *Root - node *restic.Node - inode uint64 + root *Root + forget forgetFn + node *restic.Node + inode uint64 } type openFile struct { @@ -36,12 +38,13 @@ type openFile struct { cumsize []uint64 } -func newFile(root *Root, inode uint64, node *restic.Node) (fusefile *file, err error) { +func newFile(root *Root, forget forgetFn, inode uint64, node *restic.Node) (fusefile *file, err error) { debug.Log("create new file for %v with %d blobs", node.Name, len(node.Content)) return &file{ - inode: inode, - root: root, - node: node, + inode: inode, + forget: forget, + root: root, + node: node, }, nil } @@ -172,3 +175,7 @@ func (f *file) Listxattr(_ context.Context, req *fuse.ListxattrRequest, resp *fu func (f *file) Getxattr(_ context.Context, req *fuse.GetxattrRequest, resp *fuse.GetxattrResponse) error { return nodeGetXattr(f.node, req, resp) } + +func (f *file) Forget() { + f.forget() +} diff --git a/internal/fuse/fuse_test.go b/internal/fuse/fuse_test.go index aebcb1272..5818c1edd 100644 --- a/internal/fuse/fuse_test.go +++ b/internal/fuse/fuse_test.go @@ -119,7 +119,7 @@ func TestFuseFile(t *testing.T) { root := &Root{repo: repo, blobCache: bloblru.New(blobCacheSize)} inode := inodeFromNode(1, node) - f, err := newFile(root, inode, node) + f, err := newFile(root, func() {}, inode, node) rtest.OK(t, err) of, err := f.Open(context.TODO(), nil, nil) rtest.OK(t, err) @@ -162,7 +162,7 @@ func TestFuseDir(t *testing.T) { } parentInode := inodeFromName(0, "parent") inode := inodeFromName(1, "foo") - d, err := newDir(root, inode, parentInode, node) + d, err := newDir(root, func() {}, inode, parentInode, node) rtest.OK(t, err) // don't open the directory as that would require setting up a proper tree blob @@ -276,7 +276,7 @@ func TestLink(t *testing.T) { {Name: "foo", Value: []byte("bar")}, }} - lnk, err := newLink(&Root{}, 42, node) + lnk, err := newLink(&Root{}, func() {}, 42, node) rtest.OK(t, err) target, err := lnk.Readlink(context.TODO(), nil) rtest.OK(t, err) diff --git a/internal/fuse/link.go b/internal/fuse/link.go index 975e640ea..f8bf8d3ee 100644 --- a/internal/fuse/link.go +++ b/internal/fuse/link.go @@ -12,18 +12,20 @@ import ( ) // Statically ensure that *link implements the given interface +var _ = fs.NodeForgetter(&link{}) var _ = fs.NodeGetxattrer(&link{}) var _ = fs.NodeListxattrer(&link{}) var _ = fs.NodeReadlinker(&link{}) type link struct { - root *Root - node *restic.Node - inode uint64 + root *Root + forget forgetFn + node *restic.Node + inode uint64 } -func newLink(root *Root, inode uint64, node *restic.Node) (*link, error) { - return &link{root: root, inode: inode, node: node}, nil +func newLink(root *Root, forget forgetFn, inode uint64, node *restic.Node) (*link, error) { + return &link{root: root, forget: forget, inode: inode, node: node}, nil } func (l *link) Readlink(_ context.Context, _ *fuse.ReadlinkRequest) (string, error) { @@ -57,3 +59,7 @@ func (l *link) Listxattr(_ context.Context, req *fuse.ListxattrRequest, resp *fu func (l *link) Getxattr(_ context.Context, req *fuse.GetxattrRequest, resp *fuse.GetxattrResponse) error { return nodeGetXattr(l.node, req, resp) } + +func (l *link) Forget() { + l.forget() +} diff --git a/internal/fuse/other.go b/internal/fuse/other.go index d459d0efd..cbd9667cc 100644 --- a/internal/fuse/other.go +++ b/internal/fuse/other.go @@ -12,16 +12,18 @@ import ( ) // Statically ensure that *other implements the given interface +var _ = fs.NodeForgetter(&other{}) var _ = fs.NodeReadlinker(&other{}) type other struct { - root *Root - node *restic.Node - inode uint64 + root *Root + forget forgetFn + node *restic.Node + inode uint64 } -func newOther(root *Root, inode uint64, node *restic.Node) (*other, error) { - return &other{root: root, inode: inode, node: node}, nil +func newOther(root *Root, forget forgetFn, inode uint64, node *restic.Node) (*other, error) { + return &other{root: root, forget: forget, inode: inode, node: node}, nil } func (l *other) Readlink(_ context.Context, _ *fuse.ReadlinkRequest) (string, error) { @@ -44,3 +46,7 @@ func (l *other) Attr(_ context.Context, a *fuse.Attr) error { return nil } + +func (l *other) Forget() { + l.forget() +} diff --git a/internal/fuse/root.go b/internal/fuse/root.go index ab6116f0d..72a0634fc 100644 --- a/internal/fuse/root.go +++ b/internal/fuse/root.go @@ -66,7 +66,7 @@ func NewRoot(repo restic.Repository, cfg Config) *Root { } } - root.SnapshotsDir = NewSnapshotsDir(root, rootInode, rootInode, NewSnapshotsDirStructure(root, cfg.PathTemplates, cfg.TimeTemplate), "") + root.SnapshotsDir = NewSnapshotsDir(root, func() {}, rootInode, rootInode, NewSnapshotsDirStructure(root, cfg.PathTemplates, cfg.TimeTemplate), "") return root } diff --git a/internal/fuse/snapshots_dir.go b/internal/fuse/snapshots_dir.go index cfe1f782a..bcab16084 100644 --- a/internal/fuse/snapshots_dir.go +++ b/internal/fuse/snapshots_dir.go @@ -19,6 +19,7 @@ import ( // It uses the saved prefix to select the corresponding MetaDirData. type SnapshotsDir struct { root *Root + forget forgetFn inode uint64 parentInode uint64 dirStruct *SnapshotsDirStructure @@ -28,13 +29,15 @@ type SnapshotsDir struct { // ensure that *SnapshotsDir implements these interfaces var _ = fs.HandleReadDirAller(&SnapshotsDir{}) +var _ = fs.NodeForgetter(&SnapshotsDir{}) var _ = fs.NodeStringLookuper(&SnapshotsDir{}) // NewSnapshotsDir returns a new directory structure containing snapshots and "latest" links -func NewSnapshotsDir(root *Root, inode, parentInode uint64, dirStruct *SnapshotsDirStructure, prefix string) *SnapshotsDir { +func NewSnapshotsDir(root *Root, forget forgetFn, inode, parentInode uint64, dirStruct *SnapshotsDirStructure, prefix string) *SnapshotsDir { debug.Log("create snapshots dir, inode %d", inode) return &SnapshotsDir{ root: root, + forget: forget, inode: inode, parentInode: parentInode, dirStruct: dirStruct, @@ -109,7 +112,7 @@ func (d *SnapshotsDir) Lookup(ctx context.Context, name string) (fs.Node, error) return nil, syscall.ENOENT } - return d.cache.lookupOrCreate(name, func() (fs.Node, error) { + return d.cache.lookupOrCreate(name, func(forget forgetFn) (fs.Node, error) { entry := meta.names[name] if entry == nil { return nil, syscall.ENOENT @@ -117,27 +120,33 @@ func (d *SnapshotsDir) Lookup(ctx context.Context, name string) (fs.Node, error) inode := inodeFromName(d.inode, name) if entry.linkTarget != "" { - return newSnapshotLink(d.root, inode, entry.linkTarget, entry.snapshot) + return newSnapshotLink(d.root, forget, inode, entry.linkTarget, entry.snapshot) } else if entry.snapshot != nil { - return newDirFromSnapshot(d.root, inode, entry.snapshot) + return newDirFromSnapshot(d.root, forget, inode, entry.snapshot) } - return NewSnapshotsDir(d.root, inode, d.inode, d.dirStruct, d.prefix+"/"+name), nil + return NewSnapshotsDir(d.root, forget, inode, d.inode, d.dirStruct, d.prefix+"/"+name), nil }) } +func (d *SnapshotsDir) Forget() { + d.forget() +} + // SnapshotLink type snapshotLink struct { root *Root + forget forgetFn inode uint64 target string snapshot *restic.Snapshot } +var _ = fs.NodeForgetter(&snapshotLink{}) var _ = fs.NodeReadlinker(&snapshotLink{}) // newSnapshotLink -func newSnapshotLink(root *Root, inode uint64, target string, snapshot *restic.Snapshot) (*snapshotLink, error) { - return &snapshotLink{root: root, inode: inode, target: target, snapshot: snapshot}, nil +func newSnapshotLink(root *Root, forget forgetFn, inode uint64, target string, snapshot *restic.Snapshot) (*snapshotLink, error) { + return &snapshotLink{root: root, forget: forget, inode: inode, target: target, snapshot: snapshot}, nil } // Readlink @@ -161,3 +170,7 @@ func (l *snapshotLink) Attr(_ context.Context, a *fuse.Attr) error { return nil } + +func (l *snapshotLink) Forget() { + l.forget() +} diff --git a/internal/fuse/tree_cache.go b/internal/fuse/tree_cache.go index addc54a46..d913f9b81 100644 --- a/internal/fuse/tree_cache.go +++ b/internal/fuse/tree_cache.go @@ -14,13 +14,15 @@ type treeCache struct { m sync.Mutex } +type forgetFn func() + func newTreeCache() *treeCache { return &treeCache{ nodes: map[string]fs.Node{}, } } -func (t *treeCache) lookupOrCreate(name string, create func() (fs.Node, error)) (fs.Node, error) { +func (t *treeCache) lookupOrCreate(name string, create func(forget forgetFn) (fs.Node, error)) (fs.Node, error) { t.m.Lock() defer t.m.Unlock() @@ -28,7 +30,12 @@ func (t *treeCache) lookupOrCreate(name string, create func() (fs.Node, error)) return node, nil } - node, err := create() + node, err := create(func() { + t.m.Lock() + defer t.m.Unlock() + + delete(t.nodes, name) + }) if err != nil { return nil, err } From 8aebea7ba23ae19899ad01b0c36bae867335f6b1 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Wed, 11 Sep 2024 21:31:05 +0200 Subject: [PATCH 10/22] fuse: test that the same fs.Node is used for the same file --- internal/fuse/fuse_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/internal/fuse/fuse_test.go b/internal/fuse/fuse_test.go index 5818c1edd..6cd7a450a 100644 --- a/internal/fuse/fuse_test.go +++ b/internal/fuse/fuse_test.go @@ -217,6 +217,34 @@ func testTopUIDGID(t *testing.T, cfg Config, repo restic.Repository, uid, gid ui rtest.Equals(t, uint32(0), attr.Gid) } +// The Lookup method must return the same Node object unless it was forgotten in the meantime +func testStableLookup(t *testing.T, node fs.Node, path string) fs.Node { + t.Helper() + result, err := node.(fs.NodeStringLookuper).Lookup(context.TODO(), path) + rtest.OK(t, err) + result2, err := node.(fs.NodeStringLookuper).Lookup(context.TODO(), path) + rtest.OK(t, err) + rtest.Assert(t, result == result2, "%v are not the same object", path) + + result2.(fs.NodeForgetter).Forget() + result2, err = node.(fs.NodeStringLookuper).Lookup(context.TODO(), path) + rtest.OK(t, err) + rtest.Assert(t, result != result2, "object for %v should change after forget", path) + return result +} + +func TestStableNodeObjects(t *testing.T) { + repo := repository.TestRepository(t) + restic.TestCreateSnapshot(t, repo, time.Unix(1460289341, 207401672), 2) + root := NewRoot(repo, Config{}) + + idsdir := testStableLookup(t, root, "ids") + snapID := loadFirstSnapshot(t, repo).ID().Str() + snapshotdir := testStableLookup(t, idsdir, snapID) + dir := testStableLookup(t, snapshotdir, "dir-0") + testStableLookup(t, dir, "file-2") +} + // Test reporting of fuse.Attr.Blocks in multiples of 512. func TestBlocks(t *testing.T) { root := &Root{} From d0c5b5a9b71d0e87ce528e3e3b084807c51a600d Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Wed, 11 Sep 2024 21:39:35 +0200 Subject: [PATCH 11/22] add changelog for fuse fix --- changelog/unreleased/issue-4971 | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changelog/unreleased/issue-4971 diff --git a/changelog/unreleased/issue-4971 b/changelog/unreleased/issue-4971 new file mode 100644 index 000000000..9ab529408 --- /dev/null +++ b/changelog/unreleased/issue-4971 @@ -0,0 +1,9 @@ +Bugfix: Fix unusable `mount` on macOS Sonoma + +On macOS Sonoma when using fuse-t, it was not possible to access files in +a mounted repository. + +This issue has been resolved. + +https://github.com/restic/restic/issues/4971 +https://github.com/restic/restic/pull/5048 From b8b7896d4c6f298ef06537cf0ab7525daa8fdfbd Mon Sep 17 00:00:00 2001 From: Joram Berger Date: Sun, 27 Oct 2024 19:22:34 +0100 Subject: [PATCH 12/22] doc: Clarify number of blobs are added The numbers reported as `data_blobs` and `tree_blobs` are not total numbers of blobs but numbers of blobs added with the given snapshot. --- doc/075_scripting.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/075_scripting.rst b/doc/075_scripting.rst index 9fa0da6d0..aea788644 100644 --- a/doc/075_scripting.rst +++ b/doc/075_scripting.rst @@ -191,9 +191,9 @@ Summary is the last output line in a successful backup. +---------------------------+---------------------------------------------------------+ | ``dirs_unmodified`` | Number of directories that did not change | +---------------------------+---------------------------------------------------------+ -| ``data_blobs`` | Number of data blobs | +| ``data_blobs`` | Number of data blobs added | +---------------------------+---------------------------------------------------------+ -| ``tree_blobs`` | Number of tree blobs | +| ``tree_blobs`` | Number of tree blobs added | +---------------------------+---------------------------------------------------------+ | ``data_added`` | Amount of (uncompressed) data added, in bytes | +---------------------------+---------------------------------------------------------+ @@ -651,9 +651,9 @@ was created. +---------------------------+---------------------------------------------------------+ | ``dirs_unmodified`` | Number of directories that did not change | +---------------------------+---------------------------------------------------------+ -| ``data_blobs`` | Number of data blobs | +| ``data_blobs`` | Number of data blobs added | +---------------------------+---------------------------------------------------------+ -| ``tree_blobs`` | Number of tree blobs | +| ``tree_blobs`` | Number of tree blobs added | +---------------------------+---------------------------------------------------------+ | ``data_added`` | Amount of (uncompressed) data added, in bytes | +---------------------------+---------------------------------------------------------+ From b8527f4b380093d071469071fd2a9ba6cf40da3d Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 17 Oct 2024 20:52:14 +0200 Subject: [PATCH 13/22] prune: allow dry-run without taking a lock --- changelog/unreleased/pull-5096 | 7 +++++++ cmd/restic/cmd_prune.go | 6 +++++- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 changelog/unreleased/pull-5096 diff --git a/changelog/unreleased/pull-5096 b/changelog/unreleased/pull-5096 new file mode 100644 index 000000000..d1e1d09b2 --- /dev/null +++ b/changelog/unreleased/pull-5096 @@ -0,0 +1,7 @@ +Enhancement: Allow prune dry-run without lock + +The `prune --dry-run --no-lock` now allows performing a dry-run without +taking a lock. If the repository is modified concurrently, `prune` may +return inaccurate statistics or errors. + +https://github.com/restic/restic/pull/5096 diff --git a/cmd/restic/cmd_prune.go b/cmd/restic/cmd_prune.go index e8473bd6f..a74ba23f7 100644 --- a/cmd/restic/cmd_prune.go +++ b/cmd/restic/cmd_prune.go @@ -149,7 +149,11 @@ func runPrune(ctx context.Context, opts PruneOptions, gopts GlobalOptions, term return errors.Fatal("disabled compression and `--repack-uncompressed` are mutually exclusive") } - ctx, repo, unlock, err := openWithExclusiveLock(ctx, gopts, false) + if gopts.NoLock && !opts.DryRun { + return errors.Fatal("--no-lock is only applicable in combination with --dry-run for prune command") + } + + ctx, repo, unlock, err := openWithExclusiveLock(ctx, gopts, opts.DryRun && gopts.NoLock) if err != nil { return err } From 75f317eaf1d7cc458e8bf0ef7a6030e530a007f0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 21 Oct 2024 21:41:56 +0200 Subject: [PATCH 14/22] sftp: check for broken connection in Load/List operation --- changelog/unreleased/pull-5101 | 9 +++++++++ internal/backend/sftp/sftp.go | 8 ++++++++ 2 files changed, 17 insertions(+) create mode 100644 changelog/unreleased/pull-5101 diff --git a/changelog/unreleased/pull-5101 b/changelog/unreleased/pull-5101 new file mode 100644 index 000000000..f784d0c47 --- /dev/null +++ b/changelog/unreleased/pull-5101 @@ -0,0 +1,9 @@ +Bugfix: Do not retry load/list operation is SFTP connection is broken + +When using restic with the SFTP backend, backend operations that load +a file or list files were retried even if the SFTP connection is broken. + +This has been fixed now. + +https://github.com/restic/restic/pull/5101 +https://forum.restic.net/t/restic-hanging-on-backup/8559/2 diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index efbd0c8d5..6b9620a36 100644 --- a/internal/backend/sftp/sftp.go +++ b/internal/backend/sftp/sftp.go @@ -421,6 +421,10 @@ func (r *SFTP) checkNoSpace(dir string, size int64, origErr error) error { // Load runs fn with a reader that yields the contents of the file at h at the // given offset. func (r *SFTP) Load(ctx context.Context, h backend.Handle, length int, offset int64, fn func(rd io.Reader) error) error { + if err := r.clientError(); err != nil { + return err + } + return util.DefaultLoad(ctx, h, length, offset, r.openReader, func(rd io.Reader) error { if length == 0 || !feature.Flag.Enabled(feature.BackendErrorRedesign) { return fn(rd) @@ -490,6 +494,10 @@ func (r *SFTP) Remove(_ context.Context, h backend.Handle) error { // List runs fn for each file in the backend which has the type t. When an // error occurs (or fn returns an error), List stops and returns it. func (r *SFTP) List(ctx context.Context, t backend.FileType, fn func(backend.FileInfo) error) error { + if err := r.clientError(); err != nil { + return err + } + basedir, subdirs := r.Basedir(t) walker := r.c.Walk(basedir) for { From 3800eac54bedca06d8f5e32beca7fd4c2ece8d28 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 1 Nov 2024 16:22:32 +0100 Subject: [PATCH 15/22] prepare-release: improve handling of release from non-master branch The final push command now states the correct branch to push. --- helpers/prepare-release/main.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/helpers/prepare-release/main.go b/helpers/prepare-release/main.go index ba3de38a5..607d16936 100644 --- a/helpers/prepare-release/main.go +++ b/helpers/prepare-release/main.go @@ -31,7 +31,7 @@ var opts = struct { var versionRegex = regexp.MustCompile(`^\d+\.\d+\.\d+$`) func init() { - pflag.BoolVar(&opts.IgnoreBranchName, "ignore-branch-name", false, "allow releasing from other branches as 'master'") + pflag.BoolVar(&opts.IgnoreBranchName, "ignore-branch-name", false, "allow releasing from other branches than 'master'") pflag.BoolVar(&opts.IgnoreUncommittedChanges, "ignore-uncommitted-changes", false, "allow uncommitted changes") pflag.BoolVar(&opts.IgnoreChangelogVersion, "ignore-changelog-version", false, "ignore missing entry in CHANGELOG.md") pflag.BoolVar(&opts.IgnoreChangelogReleaseDate, "ignore-changelog-release-date", false, "ignore missing subdir with date in changelog/") @@ -128,17 +128,22 @@ func uncommittedChanges(dirs ...string) string { return string(changes) } -func preCheckBranchMaster() { - if opts.IgnoreBranchName { - return - } - +func getBranchName() string { branch, err := exec.Command("git", "rev-parse", "--abbrev-ref", "HEAD").Output() if err != nil { die("error running 'git': %v", err) } - if strings.TrimSpace(string(branch)) != "master" { + return strings.TrimSpace(string(branch)) +} + +func preCheckBranchMaster() { + if opts.IgnoreBranchName { + return + } + + branch := getBranchName() + if branch != "master" { die("wrong branch: %s", branch) } } @@ -449,6 +454,7 @@ func main() { } preCheckBranchMaster() + branch := getBranchName() preCheckUncommittedChanges() preCheckVersionExists() preCheckDockerBuilderGoVersion() @@ -485,5 +491,5 @@ func main() { msg("done, output dir is %v", opts.OutputDir) - msg("now run:\n\ngit push --tags origin master\n%s\n\nrm -rf %q", dockerCmds, sourceDir) + msg("now run:\n\ngit push --tags origin %s\n%s\n\nrm -rf %q", branch, dockerCmds, sourceDir) } From d46525a51bbe519214637b396a7c64fbdcd2c2c0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 1 Nov 2024 16:36:23 +0100 Subject: [PATCH 16/22] fix double printf usage --- cmd/restic/cmd_rewrite.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/restic/cmd_rewrite.go b/cmd/restic/cmd_rewrite.go index 7788016b7..aa6dc4903 100644 --- a/cmd/restic/cmd_rewrite.go +++ b/cmd/restic/cmd_rewrite.go @@ -2,7 +2,6 @@ package main import ( "context" - "fmt" "time" "github.com/spf13/cobra" @@ -140,7 +139,7 @@ func rewriteSnapshot(ctx context.Context, repo *repository.Repository, sn *resti if selectByName(path) { return node } - Verbosef(fmt.Sprintf("excluding %s\n", path)) + Verbosef("excluding %s\n", path) return nil } From 7bfe3d99ae2d2c2bb353016e357c4cc9f596a05b Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 1 Nov 2024 18:56:11 +0100 Subject: [PATCH 17/22] fs: fallback to low privilege security descriptors on access denied --- changelog/unreleased/issue-5003 | 14 ++++++++++++++ internal/fs/sd_windows.go | 22 ++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 changelog/unreleased/issue-5003 diff --git a/changelog/unreleased/issue-5003 b/changelog/unreleased/issue-5003 new file mode 100644 index 000000000..d02b06bc7 --- /dev/null +++ b/changelog/unreleased/issue-5003 @@ -0,0 +1,14 @@ +Bugfix: fix metadata errors during backup of removable disks on Windows + +Since restic 0.17.0, backups of removable disks on Windows could report +errors with retrieving metadata like shown below. + +``` +error: incomplete metadata for d:\filename: get named security info failed with: Access is denied. +``` + +This has now been fixed. + +https://github.com/restic/restic/issues/5003 +https://github.com/restic/restic/pull/5123 +https://forum.restic.net/t/backing-up-a-folder-from-a-veracrypt-volume-brings-up-errors-since-restic-v17-0/8444 diff --git a/internal/fs/sd_windows.go b/internal/fs/sd_windows.go index 0004f1809..a39c06f2c 100644 --- a/internal/fs/sd_windows.go +++ b/internal/fs/sd_windows.go @@ -54,6 +54,15 @@ func GetSecurityDescriptor(filePath string) (securityDescriptor *[]byte, err err sd, err = getNamedSecurityInfoLow(filePath) } else { sd, err = getNamedSecurityInfoHigh(filePath) + // Fallback to the low privilege version when receiving an access denied error. + // For some reason the ERROR_PRIVILEGE_NOT_HELD error is not returned for removable media + // but instead an access denied error is returned. Workaround that by just retrying with + // the low privilege version, but don't switch privileges as we cannot distinguish this + // case from actual access denied errors. + // see https://github.com/restic/restic/issues/5003#issuecomment-2452314191 for details + if err != nil && isAccessDeniedError(err) { + sd, err = getNamedSecurityInfoLow(filePath) + } } if err != nil { if !useLowerPrivileges && isHandlePrivilegeNotHeldError(err) { @@ -114,6 +123,10 @@ func SetSecurityDescriptor(filePath string, securityDescriptor *[]byte) error { err = setNamedSecurityInfoLow(filePath, dacl) } else { err = setNamedSecurityInfoHigh(filePath, owner, group, dacl, sacl) + // See corresponding fallback in getSecurityDescriptor for an explanation + if err != nil && isAccessDeniedError(err) { + err = setNamedSecurityInfoLow(filePath, dacl) + } } if err != nil { @@ -174,6 +187,15 @@ func isHandlePrivilegeNotHeldError(err error) bool { return false } +// isAccessDeniedError checks if the error is ERROR_ACCESS_DENIED +func isAccessDeniedError(err error) bool { + if errno, ok := err.(syscall.Errno); ok { + // Compare the error code to the expected value + return errno == windows.ERROR_ACCESS_DENIED + } + return false +} + // SecurityDescriptorBytesToStruct converts the security descriptor bytes representation // into a pointer to windows SECURITY_DESCRIPTOR. func SecurityDescriptorBytesToStruct(sd []byte) (*windows.SECURITY_DESCRIPTOR, error) { From 06ba4af436b34578cbad75c16456a859f67a0ebe Mon Sep 17 00:00:00 2001 From: "Leo R. Lundgren" Date: Sun, 3 Nov 2024 22:53:09 +0100 Subject: [PATCH 18/22] doc: Polish changelogs before release --- changelog/unreleased/issue-4971 | 6 ++---- changelog/unreleased/issue-5003 | 4 ++-- changelog/unreleased/pull-5096 | 9 +++++---- changelog/unreleased/pull-5101 | 11 +++++------ 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/changelog/unreleased/issue-4971 b/changelog/unreleased/issue-4971 index 9ab529408..235d18cb5 100644 --- a/changelog/unreleased/issue-4971 +++ b/changelog/unreleased/issue-4971 @@ -1,9 +1,7 @@ Bugfix: Fix unusable `mount` on macOS Sonoma -On macOS Sonoma when using fuse-t, it was not possible to access files in -a mounted repository. - -This issue has been resolved. +On macOS Sonoma when using FUSE-T, it was not possible to access files in +a mounted repository. This issue is now resolved. https://github.com/restic/restic/issues/4971 https://github.com/restic/restic/pull/5048 diff --git a/changelog/unreleased/issue-5003 b/changelog/unreleased/issue-5003 index d02b06bc7..f88ed3113 100644 --- a/changelog/unreleased/issue-5003 +++ b/changelog/unreleased/issue-5003 @@ -1,6 +1,6 @@ -Bugfix: fix metadata errors during backup of removable disks on Windows +Bugfix: Fix metadata errors during backup of removable disks on Windows -Since restic 0.17.0, backups of removable disks on Windows could report +Since restic 0.17.0, backing up removable disks on Windows could report errors with retrieving metadata like shown below. ``` diff --git a/changelog/unreleased/pull-5096 b/changelog/unreleased/pull-5096 index d1e1d09b2..b1cc6edd3 100644 --- a/changelog/unreleased/pull-5096 +++ b/changelog/unreleased/pull-5096 @@ -1,7 +1,8 @@ -Enhancement: Allow prune dry-run without lock +Enhancement: Allow `prune --dry-run` without lock -The `prune --dry-run --no-lock` now allows performing a dry-run without -taking a lock. If the repository is modified concurrently, `prune` may -return inaccurate statistics or errors. +The `prune --dry-run --no-lock` now allows performing a dry-run +without locking the repository. Note that if the repository is +modified concurrently, `prune` may return inaccurate statistics +or errors. https://github.com/restic/restic/pull/5096 diff --git a/changelog/unreleased/pull-5101 b/changelog/unreleased/pull-5101 index f784d0c47..4152eb185 100644 --- a/changelog/unreleased/pull-5101 +++ b/changelog/unreleased/pull-5101 @@ -1,9 +1,8 @@ -Bugfix: Do not retry load/list operation is SFTP connection is broken +Bugfix: Do not retry load/list operation if SFTP connection is broken -When using restic with the SFTP backend, backend operations that load -a file or list files were retried even if the SFTP connection is broken. - -This has been fixed now. +When using restic with the SFTP backend, backend operations that load a +file or list files were retried even if the SFTP connection was broken. +This has now been fixed. https://github.com/restic/restic/pull/5101 -https://forum.restic.net/t/restic-hanging-on-backup/8559/2 +https://forum.restic.net/t/restic-hanging-on-backup/8559 From 83480246644098f2f41fb32ba07b885e0af9248d Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Fri, 8 Nov 2024 20:36:25 +0100 Subject: [PATCH 19/22] Prepare changelog for 0.17.3 --- changelog/{unreleased => 0.17.3_2024-11-08}/issue-4971 | 0 changelog/{unreleased => 0.17.3_2024-11-08}/issue-5003 | 0 changelog/{unreleased => 0.17.3_2024-11-08}/issue-5107 | 0 changelog/{unreleased => 0.17.3_2024-11-08}/pull-5096 | 0 changelog/{unreleased => 0.17.3_2024-11-08}/pull-5101 | 0 5 files changed, 0 insertions(+), 0 deletions(-) rename changelog/{unreleased => 0.17.3_2024-11-08}/issue-4971 (100%) rename changelog/{unreleased => 0.17.3_2024-11-08}/issue-5003 (100%) rename changelog/{unreleased => 0.17.3_2024-11-08}/issue-5107 (100%) rename changelog/{unreleased => 0.17.3_2024-11-08}/pull-5096 (100%) rename changelog/{unreleased => 0.17.3_2024-11-08}/pull-5101 (100%) diff --git a/changelog/unreleased/issue-4971 b/changelog/0.17.3_2024-11-08/issue-4971 similarity index 100% rename from changelog/unreleased/issue-4971 rename to changelog/0.17.3_2024-11-08/issue-4971 diff --git a/changelog/unreleased/issue-5003 b/changelog/0.17.3_2024-11-08/issue-5003 similarity index 100% rename from changelog/unreleased/issue-5003 rename to changelog/0.17.3_2024-11-08/issue-5003 diff --git a/changelog/unreleased/issue-5107 b/changelog/0.17.3_2024-11-08/issue-5107 similarity index 100% rename from changelog/unreleased/issue-5107 rename to changelog/0.17.3_2024-11-08/issue-5107 diff --git a/changelog/unreleased/pull-5096 b/changelog/0.17.3_2024-11-08/pull-5096 similarity index 100% rename from changelog/unreleased/pull-5096 rename to changelog/0.17.3_2024-11-08/pull-5096 diff --git a/changelog/unreleased/pull-5101 b/changelog/0.17.3_2024-11-08/pull-5101 similarity index 100% rename from changelog/unreleased/pull-5101 rename to changelog/0.17.3_2024-11-08/pull-5101 From 633883bdb6554f2ddc826c15283f09d72736730b Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Fri, 8 Nov 2024 20:36:25 +0100 Subject: [PATCH 20/22] Generate CHANGELOG.md for 0.17.3 --- CHANGELOG.md | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5e638c51..7ab47f11d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Table of Contents +* [Changelog for 0.17.3](#changelog-for-restic-0173-2024-11-08) * [Changelog for 0.17.2](#changelog-for-restic-0172-2024-10-27) * [Changelog for 0.17.1](#changelog-for-restic-0171-2024-09-05) * [Changelog for 0.17.0](#changelog-for-restic-0170-2024-07-26) @@ -37,6 +38,77 @@ * [Changelog for 0.6.0](#changelog-for-restic-060-2017-05-29) +# Changelog for restic 0.17.3 (2024-11-08) +The following sections list the changes in restic 0.17.3 relevant to +restic users. The changes are ordered by importance. + +## Summary + + * Fix #4971: Fix unusable `mount` on macOS Sonoma + * Fix #5003: Fix metadata errors during backup of removable disks on Windows + * Fix #5101: Do not retry load/list operation if SFTP connection is broken + * Fix #5107: Fix metadata error on Windows for backups using VSS + * Enh #5096: Allow `prune --dry-run` without lock + +## Details + + * Bugfix #4971: Fix unusable `mount` on macOS Sonoma + + On macOS Sonoma when using FUSE-T, it was not possible to access files in a + mounted repository. This issue is now resolved. + + https://github.com/restic/restic/issues/4971 + https://github.com/restic/restic/pull/5048 + + * Bugfix #5003: Fix metadata errors during backup of removable disks on Windows + + Since restic 0.17.0, backing up removable disks on Windows could report errors + with retrieving metadata like shown below. + + ``` + error: incomplete metadata for d:\filename: get named security info failed with: Access is denied. + ``` + + This has now been fixed. + + https://github.com/restic/restic/issues/5003 + https://github.com/restic/restic/pull/5123 + https://forum.restic.net/t/backing-up-a-folder-from-a-veracrypt-volume-brings-up-errors-since-restic-v17-0/8444 + + * Bugfix #5101: Do not retry load/list operation if SFTP connection is broken + + When using restic with the SFTP backend, backend operations that load a file or + list files were retried even if the SFTP connection was broken. This has now + been fixed. + + https://github.com/restic/restic/pull/5101 + https://forum.restic.net/t/restic-hanging-on-backup/8559 + + * Bugfix #5107: 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 + + * Enhancement #5096: Allow `prune --dry-run` without lock + + The `prune --dry-run --no-lock` now allows performing a dry-run without locking + the repository. Note that if the repository is modified concurrently, `prune` + may return inaccurate statistics or errors. + + https://github.com/restic/restic/pull/5096 + + # Changelog for restic 0.17.2 (2024-10-27) The following sections list the changes in restic 0.17.2 relevant to restic users. The changes are ordered by importance. From bc64921a8ea73dfaeaf4d9b66676a76998e144fc Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Fri, 8 Nov 2024 20:36:36 +0100 Subject: [PATCH 21/22] Add version for 0.17.3 --- VERSION | 2 +- cmd/restic/global.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/VERSION b/VERSION index a79916035..884e9604b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.17.2-dev +0.17.3 diff --git a/cmd/restic/global.go b/cmd/restic/global.go index 2b67708a8..56b1eba37 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -47,7 +47,7 @@ import ( // to a missing backend storage location or config file var ErrNoRepository = errors.New("repository does not exist") -var version = "0.17.2-dev (compiled manually)" +var version = "0.17.3" // TimeFormat is the format used for all timestamps printed by restic. const TimeFormat = "2006-01-02 15:04:05" From e2a98aa9557867309973db84651ba76ce5d812eb Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Fri, 8 Nov 2024 20:36:48 +0100 Subject: [PATCH 22/22] Set development version for 0.17.3 --- VERSION | 2 +- cmd/restic/global.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/VERSION b/VERSION index 884e9604b..e2d1ad6ac 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.17.3 +0.17.3-dev diff --git a/cmd/restic/global.go b/cmd/restic/global.go index 56b1eba37..133cf3744 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -47,7 +47,7 @@ import ( // to a missing backend storage location or config file var ErrNoRepository = errors.New("repository does not exist") -var version = "0.17.3" +var version = "0.17.3-dev (compiled manually)" // TimeFormat is the format used for all timestamps printed by restic. const TimeFormat = "2006-01-02 15:04:05"