[PATCH] xfsrestore: only process subtrees that are selected

[PATCH] xfsrestore: only process subtrees that are selected

[Donald Douwsma]

We are getting reports from the field where xfsrestore is aborting due
to the failed assertion:

  # xfsrestore -r -f /tmp/l0.dump -s somedir /mnt/scratch
  ...
  # xfsrestore -r -f /tmp/l2.dump /mnt/scratch
  ...
  xfsrestore: 8 directories and 7 entries processed
  xfsrestore: directory post-processing
  xfsrestore: tree.c:1369: noref_elim_recurse: Assertion `isrealpr' failed.

This occurs for cumulative restores where the initial restore has used
the subtree option to limit the trees being restored. If a subsequent
restore encounters a rename for a node outside of the selected trees it
aborts when It cannot find the directory to rename.

Make sure we skip processing for directories outside the selected trees
when eliminating unreferenced nodes in tree post processing.

Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
---
 restore/tree.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/restore/tree.c b/restore/tree.c
index 4e7cf7b..91c5e9d 100644
--- a/restore/tree.c
+++ b/restore/tree.c
@@ -1321,6 +1321,7 @@ noref_elim_recurse(nh_t parh,
 		gen_t gen;
 		bool_t inorphanagepr;
 		bool_t isdirpr;
+		bool_t isselpr;
 		bool_t isrealpr;
 		bool_t isrefpr;
 		bool_t isrenamepr;
@@ -1337,6 +1338,7 @@ noref_elim_recurse(nh_t parh,
 		isdirpr = (cldp->n_flags & NF_ISDIR);
 		isrealpr = (cldp->n_flags & NF_REAL);
 		isrefpr = (cldp->n_flags & NF_REFED);
+		isselpr = (cldp->n_flags & NF_SUBTREE);
 		isrenamepr = (isdirpr && cldp->n_lnkh != NH_NULL);
 		renameh = cldp->n_lnkh;
 		grandcldh = cldp->n_cldh;
@@ -1352,6 +1354,11 @@ noref_elim_recurse(nh_t parh,
 
 		Node_unmap(cldh, &cldp);
 
+		if (!isselpr) {
+			cldh = nextcldh;
+			continue;
+		}
+
 		if (isdirpr) {
 			bool_t ok;
 
-- 
2.47.3

[PATCH] xfs: test xfsdump subtree restores

[Donald Douwsma]

Regression test for cumulative restores where a directory has been
renamed outside of the subtree being restored triggering the assert:

  xfsrestore: tree.c:1421: noref_elim_recurse: Assertion 'isrealpr' failed

Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
---
 tests/xfs/995     | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/995.out |  2 ++
 2 files changed, 53 insertions(+)
 create mode 100755 tests/xfs/995
 create mode 100644 tests/xfs/995.out

diff --git a/tests/xfs/995 b/tests/xfs/995
new file mode 100755
index 00000000..54172572
--- /dev/null
+++ b/tests/xfs/995
@@ -0,0 +1,51 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 Red Hat.  All Rights Reserved.
+#
+# FS QA Test 995
+#
+# Regression test for cumulative restores where a directory has been
+# renamed outside of the subtree being restored resulting in
+#
+#   xfsrestore: tree.c:1421: noref_elim_recurse: Assertion 'isrealpr' failed
+#
+. ./common/preamble
+_begin_fstest auto dump
+
+_fixed_by_git_commit xfsdump XXXXXXX \
+	"xfsrestore: only process subtrees that are selected"
+
+# Override the default cleanup function.
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -r -f $tmp.*
+}
+
+# Import common functions.
+. ./common/dump
+
+# Modify as appropriate.
+_require_scratch
+_scratch_mkfs_xfs >>$seqres.full
+_scratch_mount
+
+mkdir -p $SCRATCH_MNT/a/b/c/d/e/f/g
+mkdir -p $SCRATCH_MNT/restore_me/B/C/D/E/F/G
+
+_do "$XFSDUMP_PROG -L "lab_l0" -M "test" -l0 -f $tmp.dump.l0 $SCRATCH_MNT"
+
+mv $SCRATCH_MNT/a/b/c $SCRATCH_MNT/1
+
+_do "$XFSDUMP_PROG -L "lab_l2" -M "test" -l2 -f $tmp.dump.l2 $SCRATCH_MNT"
+
+dir=$(mktemp -d $SCRATCH_MNT/restore_XXX)
+_do "$XFSRESTORE_PROG -f $tmp.dump.l0 -r -s restore_me $dir"
+_do "$XFSRESTORE_PROG -f $tmp.dump.l2 -r $dir"
+
+test $? -ne 0 && exit
+
+echo Silence is golden
+# success, all done
+_exit 0
diff --git a/tests/xfs/995.out b/tests/xfs/995.out
new file mode 100644
index 00000000..776fa6f2
--- /dev/null
+++ b/tests/xfs/995.out
@@ -0,0 +1,2 @@
+QA output created by 995
+Silence is golden
-- 
2.47.3

Re: [PATCH] xfs: test xfsdump subtree restores

[Andrey Albershteyn]

On 2026-03-26 14:00:12, Donald Douwsma wrote:
> Regression test for cumulative restores where a directory has been
> renamed outside of the subtree being restored triggering the assert:
> 
>   xfsrestore: tree.c:1421: noref_elim_recurse: Assertion 'isrealpr' failed
> 
> Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>

Looks good to me
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>

-- 
- Andrey

Re: [PATCH] xfs: test xfsdump subtree restores

[Zorro Lang]

On Thu, Mar 26, 2026 at 02:00:12PM +1100, Donald Douwsma wrote:
> Regression test for cumulative restores where a directory has been
> renamed outside of the subtree being restored triggering the assert:
> 
>   xfsrestore: tree.c:1421: noref_elim_recurse: Assertion 'isrealpr' failed
> 
> Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
> ---
>  tests/xfs/995     | 51 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/995.out |  2 ++
>  2 files changed, 53 insertions(+)
>  create mode 100755 tests/xfs/995
>  create mode 100644 tests/xfs/995.out
> 
> diff --git a/tests/xfs/995 b/tests/xfs/995
> new file mode 100755
> index 00000000..54172572
> --- /dev/null
> +++ b/tests/xfs/995
> @@ -0,0 +1,51 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2026 Red Hat.  All Rights Reserved.
> +#
> +# FS QA Test 995
> +#
> +# Regression test for cumulative restores where a directory has been
> +# renamed outside of the subtree being restored resulting in
> +#
> +#   xfsrestore: tree.c:1421: noref_elim_recurse: Assertion 'isrealpr' failed
> +#
> +. ./common/preamble
> +_begin_fstest auto dump
> +
> +_fixed_by_git_commit xfsdump XXXXXXX \
> +	"xfsrestore: only process subtrees that are selected"
> +
> +# Override the default cleanup function.
> +_cleanup()
> +{
> +	_cleanup_dump
> +	cd /
> +	rm -r -f $tmp.*
> +}
> +
> +# Import common functions.
> +. ./common/dump
> +
> +# Modify as appropriate.
> +_require_scratch
> +_scratch_mkfs_xfs >>$seqres.full
> +_scratch_mount
> +
> +mkdir -p $SCRATCH_MNT/a/b/c/d/e/f/g
> +mkdir -p $SCRATCH_MNT/restore_me/B/C/D/E/F/G
> +
> +_do "$XFSDUMP_PROG -L "lab_l0" -M "test" -l0 -f $tmp.dump.l0 $SCRATCH_MNT"
> +
> +mv $SCRATCH_MNT/a/b/c $SCRATCH_MNT/1
> +
> +_do "$XFSDUMP_PROG -L "lab_l2" -M "test" -l2 -f $tmp.dump.l2 $SCRATCH_MNT"
> +
> +dir=$(mktemp -d $SCRATCH_MNT/restore_XXX)
> +_do "$XFSRESTORE_PROG -f $tmp.dump.l0 -r -s restore_me $dir"
> +_do "$XFSRESTORE_PROG -f $tmp.dump.l2 -r $dir"

Hi Donald,

I almost forgot we had this _do helper :)

I think your test target is the 2nd xfsrestore command line (about incremental
changes), a comment might be good.

I saw used four _do calls here. I just checked the logic of the _do helper, I
suspect your current usage might prevent the test from catching command failures,
allowing it to 'successfully' continue...

A simple fix would be adding a "note" as a first argument to each _do helper call,
e.g:
  _do "Full dump" "$XFSDUMP_PROG ..."
Once all four calls are updated this way, we can remove the "test $? -ne 0 && exit"
lines too, and rely on golden image matching to detect any failures. What do you
think?

Thanks,
Zorro

> +
> +test $? -ne 0 && exit
> +
> +echo Silence is golden
> +# success, all done
> +_exit 0
> diff --git a/tests/xfs/995.out b/tests/xfs/995.out
> new file mode 100644
> index 00000000..776fa6f2
> --- /dev/null
> +++ b/tests/xfs/995.out
> @@ -0,0 +1,2 @@
> +QA output created by 995
> +Silence is golden
> -- 
> 2.47.3
> 
> 

Re: [PATCH] xfsrestore: only process subtrees that are selected

[Andrey Albershteyn]

On 2026-03-26 13:54:43, Donald Douwsma wrote:
> We are getting reports from the field where xfsrestore is aborting due
> to the failed assertion:
> 
>   # xfsrestore -r -f /tmp/l0.dump -s somedir /mnt/scratch
>   ...
>   # xfsrestore -r -f /tmp/l2.dump /mnt/scratch
>   ...
>   xfsrestore: 8 directories and 7 entries processed
>   xfsrestore: directory post-processing
>   xfsrestore: tree.c:1369: noref_elim_recurse: Assertion `isrealpr' failed.
> 
> This occurs for cumulative restores where the initial restore has used
> the subtree option to limit the trees being restored. If a subsequent
> restore encounters a rename for a node outside of the selected trees it
> aborts when It cannot find the directory to rename.
> 
> Make sure we skip processing for directories outside the selected trees
> when eliminating unreferenced nodes in tree post processing.
> 
> Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>

Looks good to me
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>

-- 
- Andrey