btrfstree: Fuss with the TreeWalk API

10 files changed, 178 insertions, 220 deletions

diff --git a/cmd/btrfs-rec/inspect/dumptrees/print_tree.go b/cmd/btrfs-rec/inspect/dumptrees/print_tree.go
index 60303e9..7703078 100644
--- a/cmd/btrfs-rec/inspect/dumptrees/print_tree.go
+++ b/cmd/btrfs-rec/inspect/dumptrees/print_tree.go
@@ -53,9 +53,9 @@ func DumpTrees(ctx context.Context, out io.Writer, fs *btrfs.FS) {
 			dlog.Error(ctx, err)
 		},
 		btrfstree.TreeWalkHandler{
-			Item: func(_ btrfstree.Path, item btrfstree.Item) error {
+			Item: func(_ btrfstree.Path, item btrfstree.Item) {
 				if item.Key.ItemType != btrfsitem.ROOT_ITEM_KEY {
-					return nil
+					return
 				}
 				treeName, ok := map[btrfsprim.ObjID]string{
 					btrfsprim.ROOT_TREE_OBJECTID:        "root",
@@ -82,7 +82,6 @@ func DumpTrees(ctx context.Context, out io.Writer, fs *btrfs.FS) {
 				}
 				textui.Fprintf(out, "%v tree key %v \n", treeName, item.Key.Format(btrfsprim.ROOT_TREE_OBJECTID))
 				printTree(ctx, out, fs, item.Key.ObjectID)
-				return nil
 			},
 		},
 	)
@@ -99,20 +98,19 @@ var nodeHeaderSize = binstruct.StaticSize(btrfstree.NodeHeader{})
 func printTree(ctx context.Context, out io.Writer, fs *btrfs.FS, treeID btrfsprim.ObjID) {
 	var itemOffset uint32
 	handlers := btrfstree.TreeWalkHandler{
-		Node: func(path btrfstree.Path, node *btrfstree.Node) error {
+		Node: func(path btrfstree.Path, node *btrfstree.Node) {
 			printHeaderInfo(out, node)
 			itemOffset = node.Size - uint32(nodeHeaderSize)
-			return nil
 		},
-		PreKeyPointer: func(path btrfstree.Path, item btrfstree.KeyPointer) error {
+		KeyPointer: func(path btrfstree.Path, item btrfstree.KeyPointer) bool {
 			treeID := path[0].FromTree
 			textui.Fprintf(out, "\tkey %v block %v gen %v\n",
 				item.Key.Format(treeID),
 				item.BlockPtr,
 				item.Generation)
-			return nil
+			return true
 		},
-		Item: func(path btrfstree.Path, item btrfstree.Item) error {
+		Item: func(path btrfstree.Path, item btrfstree.Item) {
 			treeID := path[0].FromTree
 			i := path.Node(-1).FromItemSlot
 			bs, _ := binstruct.Marshal(item.Body)
@@ -359,7 +357,6 @@ func printTree(ctx context.Context, out io.Writer, fs *btrfs.FS, treeID btrfspri
 			default:
 				textui.Fprintf(out, "\t\t(error) unhandled item type: %T\n", body)
 			}
-			return nil
 		},
 	}
 	handlers.BadItem = handlers.Item
diff --git a/cmd/btrfs-rec/inspect_lstrees.go b/cmd/btrfs-rec/inspect_lstrees.go
index cad1a37..1449a21 100644
--- a/cmd/btrfs-rec/inspect_lstrees.go
+++ b/cmd/btrfs-rec/inspect_lstrees.go
@@ -75,19 +75,21 @@ func init() {
 					treeErrCnt++
 				},
 				TreeWalkHandler: btrfstree.TreeWalkHandler{
-					Node: func(path btrfstree.Path, node *btrfstree.Node) error {
+					Node: func(path btrfstree.Path, node *btrfstree.Node) {
 						visitedNodes.Insert(path.Node(-1).ToNodeAddr)
-						return nil
 					},
-					Item: func(_ btrfstree.Path, item btrfstree.Item) error {
+					BadNode: func(path btrfstree.Path, node *btrfstree.Node, err error) bool {
+						visitedNodes.Insert(path.Node(-1).ToNodeAddr)
+						treeErrCnt++
+						return false
+					},
+					Item: func(_ btrfstree.Path, item btrfstree.Item) {
 						typ := item.Key.ItemType
 						treeItemCnt[typ]++
-						return nil
 					},
-					BadItem: func(_ btrfstree.Path, item btrfstree.Item) error {
+					BadItem: func(_ btrfstree.Path, item btrfstree.Item) {
 						typ := item.Key.ItemType
 						treeItemCnt[typ]++
-						return nil
 					},
 				},
 				PostTree: func(_ string, _ btrfsprim.ObjID) {
diff --git a/cmd/btrfs-rec/inspect_spewitems.go b/cmd/btrfs-rec/inspect_spewitems.go
index b83e989..c3a1e6b 100644
--- a/cmd/btrfs-rec/inspect_spewitems.go
+++ b/cmd/btrfs-rec/inspect_spewitems.go
@@ -34,17 +34,15 @@ func init() {
 					dlog.Error(ctx, err)
 				},
 				TreeWalkHandler: btrfstree.TreeWalkHandler{
-					Item: func(path btrfstree.Path, item btrfstree.Item) error {
+					Item: func(path btrfstree.Path, item btrfstree.Item) {
 						textui.Fprintf(os.Stdout, "%s = ", path)
 						spew.Dump(item)
 						_, _ = os.Stdout.WriteString("\n")
-						return nil
 					},
-					BadItem: func(path btrfstree.Path, item btrfstree.Item) error {
+					BadItem: func(path btrfstree.Path, item btrfstree.Item) {
 						textui.Fprintf(os.Stdout, "%s = ", path)
 						spew.Dump(item)
 						_, _ = os.Stdout.WriteString("\n")
-						return nil
 					},
 				},
 			})
diff --git a/lib/btrfs/btrfstree/btree.go b/lib/btrfs/btrfstree/btree.go
index 0e02a06..bc1bac2 100644
--- a/lib/btrfs/btrfstree/btree.go
+++ b/lib/btrfs/btrfstree/btree.go
@@ -23,22 +23,24 @@ type TreeSearcher interface {
 }
 
 type TreeWalkHandler struct {
+	BadSuperblock func(error)
+
 	// Callbacks for entire nodes.
 	//
-	// If any of these return an error that is io/fs.SkipDir, the
-	// node immediately stops getting processed; if PreNode, Node,
-	// or BadNode return io/fs.SkipDir then key pointers and items
-	// within the node are not processed.
-	PreNode  func(Path) error
-	Node     func(Path, *Node) error
-	BadNode  func(Path, *Node, error) error
-	PostNode func(Path, *Node) error
-	// Callbacks for items on interior nodes
-	PreKeyPointer  func(Path, KeyPointer) error // io/fs.SkipDir causes the KP to not be traversed
-	PostKeyPointer func(Path, KeyPointer) error
-	// Callbacks for items on leaf nodes
-	Item    func(Path, Item) error
-	BadItem func(Path, Item) error
+	// The return value from BadNode is whether to process the
+	// slots in this node or not; if no BadNode function is given,
+	// then it is not processed.
+	Node    func(Path, *Node)
+	BadNode func(Path, *Node, error) bool
+
+	// Callbacks for slots in nodes.
+	//
+	// The return value from KeyPointer is whether to recurse or
+	// not; if no KeyPointer function is given, then it is
+	// recursed.
+	KeyPointer func(Path, KeyPointer) bool
+	Item       func(Path, Item)
+	BadItem    func(Path, Item)
 }
 
 // TreeOperator is an interface for performing basic btree operations.
@@ -46,25 +48,25 @@ type TreeOperator interface {
 	// TreeWalk walks a tree, triggering callbacks for every node,
 	// key-pointer, and item; as well as for any errors encountered.
 	//
-	// If the tree is valid, then everything is walked in key-order; but if
-	// the tree is broken, then ordering is not guaranteed.
+	// If the tree is valid, then everything is walked in key-order; but
+	// if the tree is broken, then ordering is not guaranteed.
 	//
-	// Canceling the Context causes TreeWalk to return early; no
-	// values from the Context are used.
+	// Canceling the Context causes TreeWalk to return early; no values
+	// from the Context are used.
 	//
 	// The lifecycle of callbacks is:
 	//
-	//     001 .PreNode()
-	//     002 (read node)
-	//     003 .Node() (or .BadNode())
-	//         for item in node.items:
-	//           if interior:
-	//     004     .PreKeyPointer()
-	//     005     (recurse)
-	//     006     .PostKeyPointer()
-	//           else:
-	//     004     .Item() (or .BadItem())
-	//     007 .PostNode()
+	//	000  (read superblock) (maybe cbs.BadSuperblock())
+	//
+	//	001  (read node)
+	//	002  cbs.Node() or cbs.BadNode()
+	//	     if interior:
+	//	       for kp in node.items:
+	//	003a     if cbs.PreKeyPointer == nil || cbs.PreKeyPointer() {
+	//	004b       (recurse)
+	//	     else:
+	//	       for item in node.items:
+	//	003b     cbs.Item() or cbs.BadItem()
 	TreeWalk(ctx context.Context, treeID btrfsprim.ObjID, errHandle func(*TreeError), cbs TreeWalkHandler)
 
 	TreeLookup(treeID btrfsprim.ObjID, key btrfsprim.Key) (Item, error)
diff --git a/lib/btrfs/btrfstree/btree_forrest.go b/lib/btrfs/btrfstree/btree_forrest.go
index f7182d8..8f8e2de 100644
--- a/lib/btrfs/btrfstree/btree_forrest.go
+++ b/lib/btrfs/btrfstree/btree_forrest.go
@@ -105,7 +105,7 @@ func (fs TreeOperatorImpl) TreeWalk(ctx context.Context, treeID btrfsprim.ObjID,
 		errHandle(&TreeError{Path: Path{{FromTree: treeID, ToMaxKey: btrfsprim.MaxKey}}, Err: err})
 		return
 	}
-	tree.TreeWalk(ctx, errHandle, cbs)
+	tree.TreeWalk(ctx, cbs)
 }
 
 // TreeLookup implements the 'TreeOperator' interface.
diff --git a/lib/btrfs/btrfstree/btree_tree.go b/lib/btrfs/btrfstree/btree_tree.go
index 35c166f..a943803 100644
--- a/lib/btrfs/btrfstree/btree_tree.go
+++ b/lib/btrfs/btrfstree/btree_tree.go
@@ -6,9 +6,7 @@ package btrfstree
 
 import (
 	"context"
-	"errors"
 	"fmt"
-	iofs "io/fs"
 	"math"
 
 	"github.com/datawire/dlib/derror"
@@ -23,7 +21,7 @@ type RawTree struct {
 	TreeRoot
 }
 
-func (tree *RawTree) TreeWalk(ctx context.Context, errHandle func(*TreeError), cbs TreeWalkHandler) {
+func (tree *RawTree) TreeWalk(ctx context.Context, cbs TreeWalkHandler) {
 	path := Path{{
 		FromTree:         tree.ID,
 		FromItemSlot:     -1,
@@ -32,10 +30,10 @@ func (tree *RawTree) TreeWalk(ctx context.Context, errHandle func(*TreeError), c
 		ToNodeLevel:      tree.Level,
 		ToMaxKey:         btrfsprim.MaxKey,
 	}}
-	tree.walk(ctx, path, errHandle, cbs)
+	tree.walk(ctx, path, cbs)
 }
 
-func (tree *RawTree) walk(ctx context.Context, path Path, errHandle func(*TreeError), cbs TreeWalkHandler) {
+func (tree *RawTree) walk(ctx context.Context, path Path, cbs TreeWalkHandler) {
 	if ctx.Err() != nil {
 		return
 	}
@@ -43,114 +41,85 @@ func (tree *RawTree) walk(ctx context.Context, path Path, errHandle func(*TreeEr
 		return
 	}
 
-	if cbs.PreNode != nil {
-		if err := cbs.PreNode(path); err != nil {
-			if errors.Is(err, iofs.SkipDir) {
-				return
-			}
-			errHandle(&TreeError{Path: path, Err: err})
+	// 001
+	node, err := tree.Forrest.ReadNode(path)
+	defer node.Free()
+	if ctx.Err() != nil {
+		return
+	}
+
+	// 002
+	switch {
+	case err == nil:
+		if cbs.Node != nil {
+			cbs.Node(path, node)
 		}
-		if ctx.Err() != nil {
+	default:
+		process := cbs.BadNode != nil && cbs.BadNode(path, node, err)
+		if !process {
 			return
 		}
 	}
-	node, err := tree.Forrest.ReadNode(path)
-	defer node.Free()
 	if ctx.Err() != nil {
 		return
 	}
-	if err != nil && node != nil && cbs.BadNode != nil {
-		// opportunity to fix the node
-		err = cbs.BadNode(path, node, err)
-		if errors.Is(err, iofs.SkipDir) {
+
+	// 003-004
+	if node == nil {
+		return
+	}
+	// branch a (interior)
+	for i, item := range node.BodyInterior {
+		toMaxKey := path.Node(-1).ToMaxKey
+		if i+1 < len(node.BodyInterior) {
+			toMaxKey = node.BodyInterior[i+1].Key.Mm()
+		}
+		itemPath := append(path, PathElem{
+			FromTree:         node.Head.Owner,
+			FromItemSlot:     i,
+			ToNodeAddr:       item.BlockPtr,
+			ToNodeGeneration: item.Generation,
+			ToNodeLevel:      node.Head.Level - 1,
+			ToKey:            item.Key,
+			ToMaxKey:         toMaxKey,
+		})
+		// 003a
+		recurse := cbs.KeyPointer == nil || cbs.KeyPointer(itemPath, item)
+		if ctx.Err() != nil {
 			return
 		}
-	}
-	if err != nil {
-		errHandle(&TreeError{Path: path, Err: err})
-	} else if cbs.Node != nil {
-		if err := cbs.Node(path, node); err != nil {
-			if errors.Is(err, iofs.SkipDir) {
+		// 004a
+		if recurse {
+			tree.walk(ctx, itemPath, cbs)
+			if ctx.Err() != nil {
 				return
 			}
-			errHandle(&TreeError{Path: path, Err: err})
 		}
 	}
-	if ctx.Err() != nil {
+	// branch b (leaf)
+	if cbs.Item == nil && cbs.BadItem == nil {
 		return
 	}
-	if node != nil {
-		for i, item := range node.BodyInterior {
-			toMaxKey := path.Node(-1).ToMaxKey
-			if i+1 < len(node.BodyInterior) {
-				toMaxKey = node.BodyInterior[i+1].Key.Mm()
-			}
-			itemPath := append(path, PathElem{
-				FromTree:         node.Head.Owner,
-				FromItemSlot:     i,
-				ToNodeAddr:       item.BlockPtr,
-				ToNodeGeneration: item.Generation,
-				ToNodeLevel:      node.Head.Level - 1,
-				ToKey:            item.Key,
-				ToMaxKey:         toMaxKey,
-			})
-			if cbs.PreKeyPointer != nil {
-				if err := cbs.PreKeyPointer(itemPath, item); err != nil {
-					if errors.Is(err, iofs.SkipDir) {
-						continue
-					}
-					errHandle(&TreeError{Path: itemPath, Err: err})
-				}
-				if ctx.Err() != nil {
-					return
-				}
-			}
-			tree.walk(ctx, itemPath, errHandle, cbs)
-			if cbs.PostKeyPointer != nil {
-				if err := cbs.PostKeyPointer(itemPath, item); err != nil {
-					errHandle(&TreeError{Path: itemPath, Err: err})
-				}
-				if ctx.Err() != nil {
-					return
-				}
+	for i, item := range node.BodyLeaf {
+		itemPath := append(path, PathElem{
+			FromTree:     node.Head.Owner,
+			FromItemSlot: i,
+			ToKey:        item.Key,
+			ToMaxKey:     item.Key,
+		})
+		// 003b
+		switch item.Body.(type) {
+		case *btrfsitem.Error:
+			if cbs.BadItem != nil {
+				cbs.BadItem(itemPath, item)
 			}
-		}
-		for i, item := range node.BodyLeaf {
-			itemPath := append(path, PathElem{
-				FromTree:     node.Head.Owner,
-				FromItemSlot: i,
-				ToKey:        item.Key,
-				ToMaxKey:     item.Key,
-			})
-			if errBody, isErr := item.Body.(*btrfsitem.Error); isErr {
-				if cbs.BadItem == nil {
-					errHandle(&TreeError{Path: itemPath, Err: errBody.Err})
-				} else {
-					if err := cbs.BadItem(itemPath, item); err != nil {
-						errHandle(&TreeError{Path: itemPath, Err: err})
-					}
-					if ctx.Err() != nil {
-						return
-					}
-				}
-			} else {
-				if cbs.Item != nil {
-					if err := cbs.Item(itemPath, item); err != nil {
-						errHandle(&TreeError{Path: itemPath, Err: err})
-					}
-					if ctx.Err() != nil {
-						return
-					}
-				}
+		default:
+			if cbs.Item != nil {
+				cbs.Item(itemPath, item)
 			}
 		}
-	}
-	if cbs.PostNode != nil {
-		if err := cbs.PostNode(path, node); err != nil {
-			if errors.Is(err, iofs.SkipDir) {
-				return
-			}
-			errHandle(&TreeError{Path: path, Err: err})
+		if ctx.Err() != nil {
+			return
 		}
 	}
 }
@@ -181,20 +150,22 @@ func searchKP(haystack []KeyPointer, searchFn func(key btrfsprim.Key, size uint3
 func (tree *RawTree) TreeSearch(ctx context.Context, searcher TreeSearcher) (Item, error) {
 	ctx, cancel := context.WithCancel(ctx)
 	var retErr error
-
-	var ret Item
-	var selKP KeyPointer
-	tree.TreeWalk(ctx, func(err *TreeError) {
-		if retErr == nil {
+	setErr := func(err error) {
+		if retErr == nil && err != nil {
 			retErr = fmt.Errorf("item with %s: %w", searcher, err)
 		}
 		cancel()
-	}, TreeWalkHandler{
-		Node: func(_ Path, node *Node) error {
+	}
+
+	var ret Item
+	var selKP KeyPointer
+	tree.TreeWalk(ctx, TreeWalkHandler{
+		Node: func(_ Path, node *Node) {
 			if node.Head.Level > 0 { // interior node
 				kp, ok := searchKP(node.BodyInterior, searcher.Search)
 				if !ok {
-					return ErrNoItem
+					setErr(ErrNoItem)
+					return
 				}
 				selKP = kp
 			} else { // leaf node
@@ -202,18 +173,19 @@ func (tree *RawTree) TreeSearch(ctx context.Context, searcher TreeSearcher) (Ite
 					return searcher.Search(item.Key, item.BodySize)
 				})
 				if !ok {
-					return ErrNoItem
+					setErr(ErrNoItem)
+					return
 				}
 				ret = node.BodyLeaf[slot]
 				ret.Body = ret.Body.CloneItem()
 			}
-			return nil
 		},
-		PreKeyPointer: func(_ Path, kp KeyPointer) error {
-			if kp == selKP {
-				return nil
-			}
-			return iofs.SkipDir
+		BadNode: func(path Path, _ *Node, err error) bool {
+			setErr(fmt.Errorf("%v: %w", path, err))
+			return false
+		},
+		KeyPointer: func(_ Path, kp KeyPointer) bool {
+			return kp == selKP
 		},
 	})
 
@@ -230,33 +202,34 @@ func (tree *RawTree) TreeSubrange(ctx context.Context, min int, searcher TreeSea
 
 	var minKP btrfsprim.Key
 	var cnt int
-	tree.TreeWalk(ctx, func(err *TreeError) {
-		errs = append(errs, err)
-	}, TreeWalkHandler{
-		Node: func(_ Path, node *Node) error {
+	tree.TreeWalk(ctx, TreeWalkHandler{
+		Node: func(_ Path, node *Node) {
 			// Only bother for interior nodes.
 			if node.Head.Level == 0 {
-				return nil
+				return
 			}
 			kp, ok := searchKP(node.BodyInterior, searcher.Search)
 			if !ok {
 				cancel()
-				return nil
+				return
 			}
 			minKP = kp.Key
-			return nil
 		},
-		PreKeyPointer: func(_ Path, kp KeyPointer) error {
+		BadNode: func(path Path, _ *Node, err error) bool {
+			errs = append(errs, fmt.Errorf("%v: %w", path, err))
+			return false
+		},
+		KeyPointer: func(_ Path, kp KeyPointer) bool {
 			if searcher.Search(kp.Key, math.MaxUint32) < 0 {
 				cancel()
-				return iofs.SkipDir
+				return false
 			}
 			if kp.Key.Compare(minKP) > 0 {
-				return iofs.SkipDir
+				return false
 			}
-			return nil
+			return true
 		},
-		Item: func(_ Path, item Item) error {
+		Item: func(_ Path, item Item) {
 			d := searcher.Search(item.Key, item.BodySize)
 			switch {
 			case d > 1:
@@ -269,9 +242,8 @@ func (tree *RawTree) TreeSubrange(ctx context.Context, min int, searcher TreeSea
 			case d < 1:
 				cancel()
 			}
-			return nil
 		},
-		BadItem: func(_ Path, item Item) error {
+		BadItem: func(_ Path, item Item) {
 			d := searcher.Search(item.Key, item.BodySize)
 			switch {
 			case d > 1:
@@ -284,7 +256,6 @@ func (tree *RawTree) TreeSubrange(ctx context.Context, min int, searcher TreeSea
 			case d < 1:
 				cancel()
 			}
-			return nil
 		},
 	})
 
diff --git a/lib/btrfs/io2_lv.go b/lib/btrfs/io2_lv.go
index d05d51f..9f4e53f 100644
--- a/lib/btrfs/io2_lv.go
+++ b/lib/btrfs/io2_lv.go
@@ -168,15 +168,15 @@ func (fs *FS) initDev(ctx context.Context, sb btrfstree.Superblock) error {
 			errs = append(errs, err)
 		},
 		btrfstree.TreeWalkHandler{
-			Item: func(_ btrfstree.Path, item btrfstree.Item) error {
+			Item: func(_ btrfstree.Path, item btrfstree.Item) {
 				if item.Key.ItemType != btrfsitem.CHUNK_ITEM_KEY {
-					return nil
+					return
 				}
 				switch itemBody := item.Body.(type) {
 				case *btrfsitem.Chunk:
 					for _, mapping := range itemBody.Mappings(item.Key) {
 						if err := fs.LV.AddMapping(mapping); err != nil {
-							return err
+							errs = append(errs, err)
 						}
 					}
 				case *btrfsitem.Error:
@@ -187,7 +187,6 @@ func (fs *FS) initDev(ctx context.Context, sb btrfstree.Superblock) error {
 					// updated.
 					panic(fmt.Errorf("should not happen: CHUNK_ITEM has unexpected item type: %T", itemBody))
 				}
-				return nil
 			},
 		},
 	)
diff --git a/lib/btrfs/io3_btree.go b/lib/btrfs/io3_btree.go
index 80ab10f..6df88f5 100644
--- a/lib/btrfs/io3_btree.go
+++ b/lib/btrfs/io3_btree.go
@@ -37,15 +37,14 @@ func (fs *FS) populateTreeUUIDs(ctx context.Context) {
 			// do nothing
 		},
 		btrfstree.TreeWalkHandler{
-			Item: func(_ btrfstree.Path, item btrfstree.Item) error {
+			Item: func(_ btrfstree.Path, item btrfstree.Item) {
 				itemBody, ok := item.Body.(*btrfsitem.Root)
 				if !ok {
-					return nil
+					return
 				}
 				fs.cacheObjID2UUID[item.Key.ObjectID] = itemBody.UUID
 				fs.cacheTreeParent[item.Key.ObjectID] = itemBody.ParentUUID
 				fs.cacheUUID2ObjID[itemBody.UUID] = item.Key.ObjectID
-				return nil
 			},
 		},
 	)
diff --git a/lib/btrfsutil/old_rebuilt_forrest.go b/lib/btrfsutil/old_rebuilt_forrest.go
index c4ba6d3..bb5ab59 100644
--- a/lib/btrfsutil/old_rebuilt_forrest.go
+++ b/lib/btrfsutil/old_rebuilt_forrest.go
@@ -13,6 +13,7 @@ import (
 	"github.com/datawire/dlib/dlog"
 
 	"git.lukeshu.com/btrfs-progs-ng/lib/btrfs"
+	"git.lukeshu.com/btrfs-progs-ng/lib/btrfs/btrfsitem"
 	"git.lukeshu.com/btrfs-progs-ng/lib/btrfs/btrfsprim"
 	"git.lukeshu.com/btrfs-progs-ng/lib/btrfs/btrfstree"
 	"git.lukeshu.com/btrfs-progs-ng/lib/btrfs/btrfsvol"
@@ -171,35 +172,17 @@ func (bt *OldRebuiltForrest) rawTreeWalk(ctx context.Context, treeID btrfsprim.O
 		TreeRoot: *root,
 	}
 
-	errHandle := func(err *btrfstree.TreeError) {
-		if len(err.Path) > 0 && err.Path.Node(-1).ToNodeAddr == 0 {
-			// This is a panic because on the filesystems I'm working with it more likely
-			// indicates a bug in my item parser than a problem with the filesystem.
-			panic(fmt.Errorf("TODO: error parsing item: %w", err))
-		}
-		cacheEntry.Errors.Insert(oldRebuiltTreeError{
-			Min: err.Path.Node(-1).ToKey,
-			Max: err.Path.Node(-1).ToMaxKey,
-			Err: err.Err,
-		})
-	}
-
 	var curNode nodeInfo
 	cbs := btrfstree.TreeWalkHandler{
-		BadNode: func(path btrfstree.Path, node *btrfstree.Node, err error) error {
-			if node != nil {
-				curNode = nodeInfo{
-					LAddr:      path.Node(-1).ToNodeAddr,
-					Level:      node.Head.Level,
-					Generation: node.Head.Generation,
-					Owner:      node.Head.Owner,
-					MinItem:    discardOK(node.MinItem()),
-					MaxItem:    discardOK(node.MaxItem()),
-				}
-			}
-			return err
+		BadNode: func(path btrfstree.Path, node *btrfstree.Node, err error) bool {
+			cacheEntry.Errors.Insert(oldRebuiltTreeError{
+				Min: path.Node(-1).ToKey,
+				Max: path.Node(-1).ToMaxKey,
+				Err: err,
+			})
+			return false
 		},
-		Node: func(path btrfstree.Path, node *btrfstree.Node) error {
+		Node: func(path btrfstree.Path, node *btrfstree.Node) {
 			curNode = nodeInfo{
 				LAddr:      path.Node(-1).ToNodeAddr,
 				Level:      node.Head.Level,
@@ -208,9 +191,8 @@ func (bt *OldRebuiltForrest) rawTreeWalk(ctx context.Context, treeID btrfsprim.O
 				MinItem:    discardOK(node.MinItem()),
 				MaxItem:    discardOK(node.MaxItem()),
 			}
-			return nil
 		},
-		Item: func(path btrfstree.Path, item btrfstree.Item) error {
+		Item: func(path btrfstree.Path, item btrfstree.Item) {
 			if cacheEntry.Items.Search(func(v oldRebuiltTreeValue) int { return item.Key.Compare(v.Key) }) != nil {
 				// This is a panic because I'm not really sure what the best way to
 				// handle this is, and so if this happens I want the program to crash
@@ -224,11 +206,11 @@ func (bt *OldRebuiltForrest) rawTreeWalk(ctx context.Context, treeID btrfsprim.O
 				Node: curNode,
 				Slot: path.Node(-1).FromItemSlot,
 			})
-			return nil
 		},
 	}
+	cbs.BadItem = cbs.Item
 
-	tree.TreeWalk(ctx, errHandle, cbs)
+	tree.TreeWalk(ctx, cbs)
 }
 
 func (tree oldRebuiltTree) addErrs(fn func(btrfsprim.Key, uint32) int, err error) error {
@@ -358,6 +340,8 @@ func (tree oldRebuiltTree) treeSubrange(_ context.Context, min int, searcher btr
 	return err
 }
 
+// TreeWalk implements btrfstree.TreeOperator.  It doesn't actually
+// visit nodes or keypointers (just items).
 func (bt *OldRebuiltForrest) TreeWalk(ctx context.Context, treeID btrfsprim.ObjID, errHandle func(*btrfstree.TreeError), cbs btrfstree.TreeWalkHandler) {
 	tree := bt.RebuiltTree(ctx, treeID)
 	if tree.RootErr != nil {
@@ -370,11 +354,11 @@ func (bt *OldRebuiltForrest) TreeWalk(ctx context.Context, treeID btrfsprim.ObjI
 		})
 		return
 	}
-	tree.treeWalk(ctx, errHandle, cbs)
+	tree.treeWalk(ctx, cbs)
 }
 
-func (tree oldRebuiltTree) treeWalk(ctx context.Context, errHandle func(*btrfstree.TreeError), cbs btrfstree.TreeWalkHandler) {
-	if cbs.Item == nil {
+func (tree oldRebuiltTree) treeWalk(ctx context.Context, cbs btrfstree.TreeWalkHandler) {
+	if cbs.Item == nil && cbs.BadItem == nil {
 		return
 	}
 	var node *btrfstree.Node
@@ -407,10 +391,17 @@ func (tree oldRebuiltTree) treeWalk(ctx context.Context, errHandle func(*btrfstr
 				ToMaxKey:     indexItem.Value.Key,
 			},
 		}
-		if err := cbs.Item(itemPath, item); err != nil {
-			errHandle(&btrfstree.TreeError{Path: itemPath, Err: err})
+		switch item.Body.(type) {
+		case *btrfsitem.Error:
+			if cbs.BadItem != nil {
+				cbs.BadItem(itemPath, item)
+			}
+		default:
+			if cbs.Item != nil {
+				cbs.Item(itemPath, item)
+			}
 		}
-		return true
+		return ctx.Err() == nil
 	})
 	node.Free()
 }
diff --git a/lib/btrfsutil/walk.go b/lib/btrfsutil/walk.go
index bbdf992..b05218c 100644
--- a/lib/btrfsutil/walk.go
+++ b/lib/btrfsutil/walk.go
@@ -61,7 +61,7 @@ func WalkAllTrees(ctx context.Context, fs btrfstree.TreeOperator, cbs WalkAllTre
 		},
 	}
 	origItem := cbs.Item
-	cbs.Item = func(path btrfstree.Path, item btrfstree.Item) error {
+	cbs.Item = func(path btrfstree.Path, item btrfstree.Item) {
 		if item.Key.ItemType == btrfsitem.ROOT_ITEM_KEY {
 			trees = append(trees, struct {
 				Name string
@@ -73,9 +73,8 @@ func WalkAllTrees(ctx context.Context, fs btrfstree.TreeOperator, cbs WalkAllTre
 			})
 		}
 		if origItem != nil {
-			return origItem(path, item)
+			origItem(path, item)
 		}
-		return nil
 	}
 
 	for i := 0; i < len(trees); i++ {