Re: Bug (?) : cumulative xfsrestore does not restore files and folders in a directory which was renamed

Dave Chinner
2016-06-23T01:42:25Z

On Thu, Jun 23, 2016 at 08:23:28AM +1000, Dave Chinner wrote:
> On Thu, Jun 23, 2016 at 08:09:59AM +1000, Dave Chinner wrote:
> > But it seems that dirb hasn't been created before the file in it
> > is being restored. THis can happen because the inventory is not
> > correct, whichmay in fact be a problem with dump rather than
> > restore...
> >
> > I'll have a bit of a play around here, see if I can reproduce it.
>
> Yes, i can reproduce it, so I'll have a deeper look.

Can you try this patch?

-Dave
--
Dave Chinner
david@fromorbit.com

restore: make new directories after renames

From: Dave Chinner <dchinner@redhat.com>

Damien Gombault reported that restores of cumulative dumps with
renamed directories were throwing an error and were incomplete:

xfsrestore: file 0 in stream, file 0 in dump 0 on object
xfsrestore: restoring dirA/dirb/fileb (526337 1283006502)
xfsrestore: restoring regular file ino 526337 dirA/dirb/fileb
xfsrestore: WARNING: open of dirA/dirb/fileb failed: Aucun fichier ou dossier de
ce type: discarding ino 526337

This was triggered by a level 1 dump containing a directory rename
and a new directory being created inside the renamed directory. i.e:

$ mv dira dirA
$ mkdir dirA/dirb
$ echo foo > dirA/dirb/fileb

xfs_restore handles directory renames by first moving the old
directory to the orphanage, then renaming it from the orphanage to
it's new location. This, in itself is fine.

The problem is that restore creates the new directories between
these two steps. Hence any new directory created in a renamed
directory cannot be restored from a cumulative dump because when
restore tries to create the new directory neither the old directory
path nor the new directory path exists. Hence it silently drops the
new directory, resulting in subsequent errors tryin gto restore
files within that new directory.

The simple fix - just change the order of operations
in tree_post() so that new directories are created after all the
renames are processed - is not useful. All that does is break the
case of renames into newly created directories.

However, because the making of directories that already exist or
can't be made silently fails, and the create process does not modify
the internal directory tree, we can run the directory creation
function multiple times. Hence we can run directory creation both
before and after the directory rename step, hence ensuring both
new parents and new child directories are created appropriately.

This still may not be sufficient for complex directory
reorganisations, but it does address the reported problem in a
manner that is unlikely to cause regresssions. This is important,
because this code has not changed at all since it was first publicly
released in early 2001. Hence the minimum change we can make to fix
the reported problem is the least risky approach we can take.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
restore/tree.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 51 insertions(+), 10 deletions(-)

diff --git a/restore/tree.c b/restore/tree.c
index 0336e77..8e6fab1 100644
--- a/restore/tree.c
+++ b/restore/tree.c
@@ -1163,15 +1163,35 @@ tree_subtree_parse( bool_t sensepr, char *path )
return BOOL_TRUE;
}

-/* tree_post - called after the dirdump has been applied.
- * first phase is to eliminate all unreferenced dirents.
- * done by recursive depth-wise descent of the tree. on the way
- * up, unlink or orphan unreferenced nondirs, unlink unreferenced
- * dirs, orphan dirs to be renamed. if a dir is unreferenced, but
+/* tree_post - called after the dirdump has been applied. first phase is to
+ * eliminate all unreferenced dirents. done by recursive depth-wise descent of
+ * the tree. on the way up, unlink or orphan unreferenced nondirs, unlink
+ * unreferenced dirs, orphan dirs to be renamed. if a dir is unreferenced, but
* contains referenced dirents, orphan those dirents. orphan unreferenced
* nondirs if they are the last link to an inode referenced but not real
* somewhere else in the tree. next, make new directories. then rename
* directories. finally, create hardlinks from orphanage.
+ *
+ * Note: the way renamed directories are handled by first orphaning them leads
+ * to a chicken and egg problem - the directory does not exist when we try to
+ * make a new directory inside the renamed directory destination. This fails
+ * silently, leaving us with ENOENT errors when trying to restore files within
+ * that new directory.
+ *
+ * To prevent this from happening, we need to create new subdirectories *after*
+ * we have processed all the renamed directories. However, so that the renames
+ * succeed, we also have to create any new parent directories that the renames
+ * depend on.
+ *
+ * Hence we have to do two mkdir passes: one before the renames to create new
+ * ancestors for rename destinations and one after the rename to create new
+ * children in rename destinations.
+ *
+ * NOTE: a simple before/after creation as done below may be too simple for
+ * complex directory structure manipulations. e.g. rename into a new child in
+ * the destination of another rename. This may have to become an iterative loop
+ * that runs until all renames and mkdirs are resolved. We'll cross that bridge
+ * when we need to, not now.
*/
static bool_t noref_elim_recurse( nh_t parh,
nh_t cldh,
@@ -1216,7 +1236,14 @@ tree_post( char *path1, char *path2 )
}
}

- /* make new directories
+#ifdef TREE_CHK
+ assert( tree_chk( ));
+#endif /* TREE_CHK */
+
+ /*
+ * make new directories to ensure rename destination ancestors are
+ * present before attempting the renames. This will silently skip all
+ * the creations that are in rename destinations.
*/
mlog( MLOG_DEBUG | MLOG_TREE,
"making new directories\n" );
@@ -1228,10 +1255,6 @@ tree_post( char *path1, char *path2 )
return BOOL_FALSE;
}

-#ifdef TREE_CHK
- assert( tree_chk( ));
-#endif /* TREE_CHK */
-
/* rename directories
*/
if ( ! persp->p_fullpr ) {
@@ -1250,6 +1273,24 @@ tree_post( char *path1, char *path2 )
assert( tree_chk( ));
#endif /* TREE_CHK */

+ /*
+ * Repeat making new directories to create directories in rename
+ * destinations that were skipped in the first pass.
+ */
+ mlog( MLOG_DEBUG | MLOG_TREE,
+ "making new directories in renamed ancestors\n" );
+ rootp = Node_map( persp->p_rooth );
+ cldh = rootp->n_cldh;
+ Node_unmap( persp->p_rooth, &rootp );
+ ok = mkdirs_recurse( persp->p_rooth, cldh, path1 );
+ if ( ! ok ) {
+ return BOOL_FALSE;
+ }
+
+#ifdef TREE_CHK
+ assert( tree_chk( ));
+#endif /* TREE_CHK */
+
/* process hard links
*/
if ( ! persp->p_fullpr ) {