| Message ID | 169049626432.922543.2560381879385116722.stgit@frogsfrogsfrogs (mailing list archive) |
|---|---|
| Headers | show |
| Series | xfs: online repair of inodes and forks | expand |
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.... -Dave.
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