Re: [PATCH 1/2] xfs_db rdump: remove sole_path logic

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Nov 12, 2025 at 06:19:31PM +0300, Alexander Monakov wrote:
> Eliminate special handling of the case where rdump'ing one directory
> does not create the corresponding directory in the destination, but
> instead modifies the destination's attributes and creates children
> alongside the pre-existing children.

It sounds like what happened is that you ran rdump with a non-empty
destdir, only to have rdump mutate the existing children in that
directory.  Is that correct?

If so, then I think what you really wanted was for rdump to check for
that and error out, unless explicit --overwrite permission had been
granted.  Because...

> This can be a trap for the unwary (the effect on attributes can be
> particularly surprising and non-trivial to undo), and, in general, fewer
> special cases in such a low-level tool should be desirable.

...I use this "special case" and don't understand why you decided that
removing functionality was the solution here.  This is a filesystem
debugger, there are weird functions and sharp edges everywhere.

--D

> Signed-off-by: Alexander Monakov <amonakov@ispras.ru>
> ---
>  db/rdump.c        | 19 ++-----------------
>  man/man8/xfs_db.8 |  8 +-------
>  2 files changed, 3 insertions(+), 24 deletions(-)
> 
> diff --git a/db/rdump.c b/db/rdump.c
> index 9ff83355..73295dbe 100644
> --- a/db/rdump.c
> +++ b/db/rdump.c
> @@ -852,7 +852,6 @@ rdump_file(
>  static int
>  rdump_path(
>  	struct xfs_mount	*mp,
> -	bool			sole_path,
>  	const char		*path,
>  	const struct destdir	*destdir)
>  {
> @@ -890,20 +889,6 @@ rdump_path(
>  			dbprintf(_("%s: %s\n"), path, strerror(ret));
>  			return 1;
>  		}
> -
> -		if (sole_path) {
> -			struct xfs_dinode	*dip = iocur_top->data;
> -
> -			/*
> -			 * If this is the only path to copy out and it's a dir,
> -			 * then we can copy the children directly into the
> -			 * target.
> -			 */
> -			if (S_ISDIR(be16_to_cpu(dip->di_mode))) {
> -				pbuf->len = 0;
> -				pbuf->path[0] = 0;
> -			}
> -		}
>  	} else {
>  		set_cur_inode(mp->m_sb.sb_rootino);
>  	}
> @@ -980,7 +965,7 @@ rdump_f(
>  	if (optind == argc - 1) {
>  		/* no dirs given, just do the whole fs */
>  		push_cur();
> -		ret = rdump_path(mp, false, "", &destdir);
> +		ret = rdump_path(mp, "", &destdir);
>  		pop_cur();
>  		if (ret)
>  			exitcode = 1;
> @@ -996,7 +981,7 @@ rdump_f(
>  		argv[i][len] = 0;
>  
>  		push_cur();
> -		ret = rdump_path(mp, argc == optind + 2, argv[i], &destdir);
> +		ret = rdump_path(mp, argv[i], &destdir);
>  		pop_cur();
>  
>  		if (ret) {
> diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8
> index 1e85aebb..920b2b3e 100644
> --- a/man/man8/xfs_db.8
> +++ b/man/man8/xfs_db.8
> @@ -1147,13 +1147,7 @@ the filesystem as possible.
>  If zero
>  .B paths
>  are specified, the entire filesystem is dumped.
> -If only one
> -.B path
> -is specified and it is a directory, the children of that directory will be
> -copied directly to the destination.
> -If multiple
> -.B paths
> -are specified, each file is copied into the directory as a new child.
> +Otherwise, each entry is copied into the destination as a new child.
>  
>  If possible, sparse holes, xfs file attributes, and extended attributes will be
>  preserved.
> -- 
> 2.51.0
> 
>