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

Bill O'Donnell
2016-06-23T14:17:54Z

On Thu, Jun 23, 2016 at 11:42:17AM +1000, Dave Chinner wrote:
> 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?

FWIW, I tried it on a RHEL7 box and it works fine with Damien's test case.

Thanks-
Bill

>
> -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 ) {
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs