Re: [PATCH 4/4] xfs_db: use file_setattr to copy attributes on special files with rdump

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

On Fri, Aug 08, 2025 at 09:30:19PM +0200, Andrey Albershteyn wrote:
> rdump just skipped file attributes on special files as copying wasn't
> possible. Let's use new file_getattr/file_setattr syscalls to copy
> attributes even for special files.
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
>  db/rdump.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/db/rdump.c b/db/rdump.c
> index 9ff833553ccb..5b9458e6bc94 100644
> --- a/db/rdump.c
> +++ b/db/rdump.c
> @@ -17,6 +17,7 @@
>  #include "field.h"
>  #include "inode.h"
>  #include "listxattr.h"
> +#include "libfrog/file_attr.h"
>  #include <sys/xattr.h>
>  #include <linux/xattr.h>
>  
> @@ -152,10 +153,17 @@ rdump_fileattrs_path(
>  	const struct destdir	*destdir,
>  	const struct pathbuf	*pbuf)
>  {
> +	struct file_attr	fa = {
> +		.fa_extsize	= ip->i_extsize,
> +		.fa_projid	= ip->i_projid,
> +		.fa_cowextsize	= ip->i_cowextsize,
> +		.fa_xflags	= xfs_ip2xflags(ip),
> +	};
>  	int			ret;
> +	int			at_flags = AT_SYMLINK_NOFOLLOW;

Why does this become a mutable variable?  AFAICT it doesn't change?

Otherwise things look good here.

--D

>  
>  	ret = fchmodat(destdir->fd, pbuf->path, VFS_I(ip)->i_mode & ~S_IFMT,
> -			AT_SYMLINK_NOFOLLOW);
> +			at_flags);
>  	if (ret) {
>  		/* fchmodat on a symlink is not supported */
>  		if (errno == EPERM || errno == EOPNOTSUPP)
> @@ -169,7 +177,7 @@ rdump_fileattrs_path(
>  	}
>  
>  	ret = fchownat(destdir->fd, pbuf->path, i_uid_read(VFS_I(ip)),
> -			i_gid_read(VFS_I(ip)), AT_SYMLINK_NOFOLLOW);
> +			i_gid_read(VFS_I(ip)), at_flags);
>  	if (ret) {
>  		if (errno == EPERM)
>  			lost_mask |= LOST_OWNER;
> @@ -181,7 +189,17 @@ rdump_fileattrs_path(
>  			return 1;
>  	}
>  
> -	/* Cannot copy fsxattrs until setfsxattrat gets merged */
> +	ret = file_setattr(destdir->fd, pbuf->path, NULL, &fa, at_flags);
> +	if (ret) {
> +		if (errno == EOPNOTSUPP || errno == EPERM || errno == ENOTTY)
> +			lost_mask |= LOST_FSXATTR;
> +		else
> +			dbprintf(_("%s%s%s: file_setattr %s\n"), destdir->path,
> +					destdir->sep, pbuf->path,
> +					strerror(errno));
> +		if (strict_errors)
> +			return 1;
> +	}
>  
>  	return 0;
>  }
> 
> -- 
> 2.49.0
> 
>