On Fri, Jun 23, 2023 at 04:29:18PM +1000, Donald Douwsma wrote:
> If we're going to have a fix for false root problems its a good idea to
> let people know that there's a way to recover, error out with a useful
> message that mentions the `-x` option rather than just assert.
>
> Before
>
> xfsrestore: searching media for directory dump
> xfsrestore: reading directories
> xfsrestore: tree.c:757: tree_begindir: Assertion `ino != persp->p_rootino || hardh == persp->p_rooth' failed.
> Aborted
>
> After
>
> xfsrestore: ERROR: tree.c:791: tree_begindir: Assertion `ino != persp->p_rootino || hardh == persp->p_rooth` failed.
> xfsrestore: ERROR: False root detected. Recovery may be possible using the `-x` option
> Aborted
>
> Fixes: d7cba74 ("xfsrestore: fix rootdir due to xfsdump bulkstat misuse")
> Signed-off-by: Donald Douwsma <ddouwsma@xxxxxxxxxx>
> ---
> restore/tree.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/restore/tree.c b/restore/tree.c
> index bfa07fe..0b65d0f 100644
> --- a/restore/tree.c
> +++ b/restore/tree.c
> @@ -783,8 +783,17 @@ tree_begindir(filehdr_t *fhdrp, dah_t *dahp)
> /* lookup head of hardlink list
> */
> hardh = link_hardh(ino, gen);
> - if (need_fixrootdir == BOOL_FALSE)
> - assert(ino != persp->p_rootino || hardh == persp->p_rooth);
> + if (need_fixrootdir == BOOL_FALSE
> + && !(ino != persp->p_rootino || hardh == persp->p_rooth)) {
Usually we put the && operator on the line above. But ignore that
comment if that's actually the predominant style in xfsrestore.
if (need_fixrootdir == BOOL_FALSE &&
!(ino != persp->p_rootino || hardh == persp->p_rooth)) {
> + mlog(MLOG_ERROR | MLOG_TREE,
Trailing space.
> + "%s:%d: %s: Assertion "
> + "`ino != persp->p_rootino || hardh == persp->p_rooth` failed.\n",
Might be nice to put the entire string on one line for greppability?
mlog(MLOG_ERROR | MLOG_TREE,
"%s:%d: %s: Assertion `ino != persp->p_rootino || hardh == persp->p_rooth` failed.\n",
> + __FILE__, __LINE__, __FUNCTION__);
> + mlog(MLOG_ERROR | MLOG_TREE, _(
> + "False root detected. "
> + "Recovery may be possible using the `-x` option\n"));
Same here.
Otherwise this looks like a good improvement.
--D
> + return NH_NULL;
> + };
>
> /* already present
> */
> --
> 2.39.3
>
