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

["Darrick J. Wong" <djwong@kernel.org>]

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