From d8e03849408c73518187f90652bb3970fc0cb3ad Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 17 Oct 2024 19:45:03 +0200 Subject: [PATCH 01/12] 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 02/12] 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 03/12] 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 04/12] 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 05/12] 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 06/12] 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 07/12] 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 08/12] 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 09/12] 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 10/12] 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 11/12] 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 12/12] 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) {