xfs
[Top] [All Lists]

Re: REVIEW: xfs_repair fixes for bad directories

To: "Christoph Hellwig" <hch@xxxxxxxxxxxxx>
Subject: Re: REVIEW: xfs_repair fixes for bad directories
From: "Barry Naujok" <bnaujok@xxxxxxx>
Date: Wed, 27 Aug 2008 14:21:34 +1000
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
In-reply-to: <20080826193205.GA31105@xxxxxxxxxxxxx>
Organization: SGI
References: <op.udlsirmx3jf8g2@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20080826193205.GA31105@xxxxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Opera Mail/9.51 (Win32)
On Wed, 27 Aug 2008 05:32:05 +1000, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:

On Tue, Jul 01, 2008 at 06:00:17PM +1000, Barry Naujok wrote:

2. An update a while ago to xfs_repair attempts to fix invalid
   ".." entries for subdirectories where there is a valid parent
   with the appropriate entry. It was a partial fix that never
   did the full job, especially if the subdirectory was short-
   form or it has already been processed.

   Patch fix_dir_rebuild_without_dotdot_entry.patch creates a
   post-processing queue after the main scan to update any
   directories with an invalid ".." entry.

Where is the existing attemp?  I can't find code doing anything like
that removed in the patch.  But the actual patch looks good, while
I had this mess with the tons of different boolean flags in repair
converting these to a more descriptive bitmask should be a different
patch.

A while ago, when we found some directories couldn't be deleted
as their "." and ".." entries were in different blocks, code was
added to detect this and fix it up, including restoring ".."
entries that were missing in phase 6:

                } else if (parent == ip->i_ino)  {
                        add_inode_reached(irec, ino_offset);
                        add_inode_ref(current_irec, current_ino_offset);
+               } else if (parent == NULLFSINO) {
+                       /* ".." was missing, but this entry refers to it,
+                          so, set it as the parent and mark for rebuild */
+                       do_warn(_("entry \"%s\" in dir ino %llu doesn't have a"
+                               " .. entry, will set it in ino %llu.\n"),
+                               fname, ip->i_ino, inum);
+                       set_inode_parent(irec, ino_offset, ip->i_ino);
+                       add_inode_reached(irec, ino_offset);
+                       add_inode_ref(current_irec, current_ino_offset);
                } else  {
                        junkit = 1;
                        do_warn(


It didn't quite work in the end depending on the order of inode scanning!

So, this patch added that post-processing queue to make sure it always works.


<Prev in Thread] Current Thread [Next in Thread>