On 3/20/25 3:25 PM, bodonnel@xxxxxxxxxx wrote:
> From: Bill O'Donnell <bodonnel@xxxxxxxxxx>
>
> In certain cases, if a block is so messed up that crc, uuid and magic number are all
> bad, we need to not only detect in phase3 but fix it properly in phase6. In the
> current code, the mechanism doesn't work in that it only pays attention to one of the
> parameters.
>
> Note: in this case, the nlink inode link count drops to 1, but re-running xfs_repair
> fixes it back to 2. This is a side effect that should probably be handled in
> update_inode_nlinks() with separate patch. Regardless, running xfs_repair twice
> fixes the issue. Also, this patch fixes the issue with v5, but not v4 xfs.
>
> Signed-off-by: Bill O'Donnell <bodonnel@xxxxxxxxxx>
> ---
> repair/phase6.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 4064a84b2450..677505d45357 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -2336,6 +2336,7 @@ longform_dir2_entry_check(
> int fixit = 0;
> struct xfs_da_args args;
> int error;
> + int wantmagic;
>
> *need_dot = 1;
> freetab = malloc(FREETAB_SIZE(ip->i_disk_size / mp->m_dir_geo->blksize));
> @@ -2364,7 +2365,6 @@ longform_dir2_entry_check(
> da_bno = (xfs_dablk_t)next_da_bno) {
> const struct xfs_buf_ops *ops;
> int error;
> - struct xfs_dir2_data_hdr *d;
>
> next_da_bno = da_bno + mp->m_dir_geo->fsbcount - 1;
> if (bmap_next_offset(ip, &next_da_bno)) {
> @@ -2404,9 +2404,11 @@ longform_dir2_entry_check(
> }
>
> /* check v5 metadata */
> - d = bp->b_addr;
> - if (be32_to_cpu(d->magic) == XFS_DIR3_BLOCK_MAGIC ||
> - be32_to_cpu(d->magic) == XFS_DIR3_DATA_MAGIC) {
> + if (xfs_has_crc(mp))
> + wantmagic = XFS_DIR3_BLOCK_MAGIC;
> + else
> + wantmagic = XFS_DIR2_BLOCK_MAGIC;
> + if (wantmagic == XFS_DIR3_BLOCK_MAGIC) {
So I guess the prior 5 lines are equivalent to:
/* check v5 metadata */
if (xfs_has_crc(mp)) {
...
and that will force it to check the header, below. And then I think we hit
the goto out_fix; line, and it moves the contents of this directory to
lost+found (at least on my custom repro image.)
Curious to see what others think is the right path through all this.
-Eric
> error = check_dir3_header(mp, bp, ino);
> if (error) {
> fixit++;
