REVIEW: Don't reset dirty inode flag in xfs_repair

REVIEW: Don't reset dirty inode flag in xfs_repair

[Barry Naujok]

If an inode is dirtied due to some error in an inode, the very last
check (nlink version) in process_dinode_int() in xfs_repair sets the
dirty flag rather than just bumping it if it dirtied the inode.

So, if something earlier dirtied the inode without marking it bad
(eg. resetting the inode's next unlinked field in the example that
detected this issue), that dirty state will be lost if the nlink
version checks out fine.


===========================================================================
xfsprogs/db/check.c
===========================================================================

--- a/xfsprogs/db/check.c       2008-09-24 18:30:53.000000000 +1000
+++ b/xfsprogs/db/check.c       2008-09-24 18:04:48.215871460 +1000
@@ -2689,6 +2689,12 @@ process_inode(
                 }
                 return;
         }
+       if (be32_to_cpu(dip->di_next_unlinked) != NULLAGINO) {
+               if (!sflag || isfree || id->ilist || CHECK_BLIST(bno))
+                       dbprintf("bad next unlinked %#x for inode %lld\n",
+                               be32_to_cpu(dip->di_next_unlinked), ino);
+               error++;
+       }
         /*
          * di_mode is a 16-bit uint so no need to check the < 0 case
          */

===========================================================================
xfsprogs/repair/dinode.c
===========================================================================

--- a/xfsprogs/repair/dinode.c  2008-09-24 18:30:53.000000000 +1000
+++ b/xfsprogs/repair/dinode.c  2008-09-24 18:29:19.477386207 +1000
@@ -2694,7 +2694,7 @@ process_dinode_int(xfs_mount_t *mp,
          * just leave nlinks alone.  even if it's set wrong,
          * it'll be reset when read in.
          */
-       *dirty = process_check_inode_nlink_version(dinoc, lino);
+       *dirty += process_check_inode_nlink_version(dinoc, lino);

         return retval;

Re: REVIEW: Don't reset dirty inode flag in xfs_repair

[Christoph Hellwig]

On Wed, Sep 24, 2008 at 06:47:35PM +1000, Barry Naujok wrote:
> If an inode is dirtied due to some error in an inode, the very last
> check (nlink version) in process_dinode_int() in xfs_repair sets the
> dirty flag rather than just bumping it if it dirtied the inode.
>
> So, if something earlier dirtied the inode without marking it bad
> (eg. resetting the inode's next unlinked field in the example that
> detected this issue), that dirty state will be lost if the nlink
> version checks out fine.

Looks good, but the diff you posted also includes a hunk in db/check.c
that introduces another sanity check in process_inode() which seems
unrelated.