Re: [PATCH v2] xfs: test xfsdump subtree restores

[Donald Douwsma <ddouwsma@redhat.com>]

On 11/8/26 06:12, Zorro Lang wrote:
> On Fri, Aug 07, 2026 at 05:48:57PM +1000, Donald Douwsma wrote:
>> On 28/7/26 20:39, Zorro Lang wrote:
>>> On Thu, Jul 23, 2026 at 04:26:17PM +1000, 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>
>>>> ---
>>>> Changes since v1
>>>> - Fix use of _do, including label quoting
>>>> - Update test output
>>>> - Add tests for additional edge cases
>>>> ---
>>>>  tests/xfs/995     | 58 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>  tests/xfs/995.out |  8 +++++++
>>>>  2 files changed, 66 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 000000000..bba0bcd52
>>>> --- /dev/null
>>>> +++ b/tests/xfs/995
>>>> @@ -0,0 +1,58 @@
>>>> +#! /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
>>>> +_do_die_on_error="always"
>>>
>>> I think this line is useless now, right? I'll remove it.
>>>
>>
>> Yes, thanks.
>>
>> I did have a general question about how _do_die_on_error is used
>>
>> $ git grep _do_die_on_error
>> common/rc:# second argument. If the command fails and the variable _do_die_on_error
>> common/rc:# is set to "always" or the two argument form is used and _do_die_on_error
>> common/rc:      && [ "$_do_die_on_error" = "always" \
>> common/rc:          -o \( $# -eq 2 -a "$_do_die_on_error" = "message_only" \) ]
> 
> Oh, my bad. I confused this parameter with the "enable_error" parameter you
> tried to introduce in your previous patch.

You mean the attr one that ended up as tests/xfs/649"
lol, no. Though that was helpful when creating/debugging a test that panics a box.

> Since this variable is used by the current _do helper, setting it to "always" or
> "message_only" here is good. I will keep this line.

Cool, I think i added _do_die_on_error when working on the v2 because I was testing
it on an older xfstests where the coredump functionality wasn't available.

The v1 of this test only had a 'Silence is Golden', so the test would pass which was
really confusing, I fixed that by adding a better 995.out, so this should work with
or without it now.

> 
>> tests/generic/017:_do_die_on_error=y
>> tests/generic/053:_do_die_on_error=y
>> tests/xfs/041:_do_die_on_error=message_only
>> tests/xfs/042:_do_die_on_error=message_only
>> tests/xfs/596:_do_die_on_error=message_only
>> tests/xfs/635:_do_die_on_error=message_only
>>
>> _do_die_on_error=y doesn't seem valid.
> 
> I think you're right, "y" isn't valid. The _do() helper is quite old and rarely used.
> If you have any optimization suggestions, feel free to send a patch to improve it.
> For this patch, I'll merge your current version directly.

I'll have a think about that, it seemed handy for this test, but I don't understand
how the other tests use it.

Cheers,
Don

>> generic/017 doesn't use _do, and I think generic/053 wants _do_die_on_error=always
> 
>>
>> Thoughts?
>>
>> Don
>>