Re: [PATCHSET v26.0 0/6] xfs: online repair of inodes and forks

Darrick J. Wong
2023-08-10T00:45:41Z

On Wed, Aug 09, 2023 at 07:44:02PM +1000, Dave Chinner wrote:
> On Thu, Jul 27, 2023 at 03:21:08PM -0700, Darrick J. Wong wrote:
> > Hi all,
> >
> > In this series, online repair gains the ability to repair inode records.
> > To do this, we must repair the ondisk inode and fork information enough
> > to pass the iget verifiers and hence make the inode igettable again.
> > Once that's done, we can perform higher level repairs on the incore
> > inode. The fstests counterpart of this patchset implements stress
> > testing of repair.
> >
> > If you're going to start using this mess, you probably ought to just
> > pull from my git trees, which are linked below.
>
> Most of this makes sense. I think the main thing I'd suggest is
> documenting the repair decisions being made and how things that get
> zapped are then rebuilt - it seems like there is a lot of dependency
> on running other parts of repair after zapping for things to be
> rebuilt, but it's not immediately clear how the bits are supposed to
> go together so a little bit of documentation for that would go a
> long way....

Ok. The comment for inode_repair.c now reads:

/*
* Inode Record Repair
* ===================
*
* Roughly speaking, inode problems can be classified based on whether
* or not they trip the dinode verifiers. If those trip, then we won't
* be able to xfs_iget ourselves the inode.
*
* Therefore, the xrep_dinode_* functions fix anything that will cause
* the inode buffer verifier or the dinode verifier. The xrep_inode_*
* functions fix things on live incore inodes. The inode repair
* functions make decisions with security and usability implications
* when reviving a file:
*
* - Files with zero di_mode or a garbage di_mode are converted to a
* file that only root can read. If the immediate data fork area or
* block 0 of the data fork look like a directory, the file type will be
* set to a directory. If the immediate data fork area has no nulls, it
* will be turned into a symbolic link. Otherwise, it is turned into a
* regular file. This file may not actually contain user data, if the
* file was not previously a regular file. Setuid and setgid bits are
* cleared.
*
* - Zero-size directories can be truncated to look empty. It is
* necessary to run the bmapbtd and directory repair functions to fully
* rebuild the directory.
*
* - Zero-size symbolic link targets can be truncated to '.'. It is
* necessary to run the bmapbtd and symlink repair functions to salvage
* the symlink.
*
* - Invalid extent size hints will be removed.
*
* - Quotacheck will be scheduled if we repaired an inode that was so
* badly damaged that the ondisk inode had to be rebuilt.
*
* - Invalid user, group, or project IDs (aka -1U) will be reset to
* zero. Setuid and setgid bits are cleared.
*
* - Data and attr forks are reset to extents format with zero extents
* if the fork data is inconsistent. It is necessary to run the bmapbtd
* or bmapbta repair functions to recover the space mapping.
*
* - ACLs will not be recovered if the attr fork is zapped or the
* extended attribute structure itself requires salvaging.
*
* - If the attr fork is zapped, the user and group ids are reset to
* root and the setuid and setgid bits are removed.
*/

--D

> -Dave.
>
> --
> Dave Chinner
> david@fromorbit.com