Re: [PATCH v2 2/3] xfs_io: allow foreign FSes to show FS_IOC_FSGETXATTR details

[Darrick J. Wong]

On Thu, Dec 19, 2024 at 06:09:14PM +0530, Ojaswin Mujoo wrote:
> Currently with stat we only show FS_IOC_FSGETXATTR details if the
> filesystem is XFS. With extsize support also coming to ext4 and possibly
> other filesystems, make sure to allow foreign FSes to display these details
> when "stat" or "statx" is used.
> 
> (Thanks to Dave for suggesting implementation of print_extended_info())
> 
> Suggested-by: Dave Chinner <david@xxxxxxxxxxxxx>
> Signed-off-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>

Looks good to me now,
Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>

--D

> ---
>  io/stat.c | 63 +++++++++++++++++++++++++++++++------------------------
>  1 file changed, 36 insertions(+), 27 deletions(-)
> 
> diff --git a/io/stat.c b/io/stat.c
> index 326f2822e276..3ce3308d0562 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -98,30 +98,45 @@ print_file_info(void)
>  }
>  
>  static void
> -print_xfs_info(int verbose)
> +print_extended_info(int verbose)
>  {
> -	struct dioattr	dio;
> -	struct fsxattr	fsx, fsxa;
> -
> -	if ((xfsctl(file->name, file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0 ||
> -	    (xfsctl(file->name, file->fd, XFS_IOC_FSGETXATTRA, &fsxa)) < 0) {
> -		perror("FS_IOC_FSGETXATTR");
> -	} else {
> -		printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags);
> -		printxattr(fsx.fsx_xflags, verbose, 0, file->name, 1, 1);
> -		printf(_("fsxattr.projid = %u\n"), fsx.fsx_projid);
> -		printf(_("fsxattr.extsize = %u\n"), fsx.fsx_extsize);
> -		printf(_("fsxattr.cowextsize = %u\n"), fsx.fsx_cowextsize);
> -		printf(_("fsxattr.nextents = %u\n"), fsx.fsx_nextents);
> -		printf(_("fsxattr.naextents = %u\n"), fsxa.fsx_nextents);
> +	struct dioattr dio = {};
> +	struct fsxattr fsx = {}, fsxa = {};
> +
> +	if ((ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) {
> +		perror("FS_IOC_GETXATTR");
> +		exitcode = 1;
> +		return;
>  	}
> +
> +	printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags);
> +	printxattr(fsx.fsx_xflags, verbose, 0, file->name, 1, 1);
> +	printf(_("fsxattr.projid = %u\n"), fsx.fsx_projid);
> +	printf(_("fsxattr.extsize = %u\n"), fsx.fsx_extsize);
> +	printf(_("fsxattr.cowextsize = %u\n"), fsx.fsx_cowextsize);
> +	printf(_("fsxattr.nextents = %u\n"), fsx.fsx_nextents);
> +
> +	/* Only XFS supports FS_IOC_FSGETXATTRA and XFS_IOC_DIOINFO */
> +	if (file->flags & IO_FOREIGN)
> +		return;
> +
> +	if ((ioctl(file->fd, XFS_IOC_FSGETXATTRA, &fsxa)) < 0) {
> +		perror("XFS_IOC_GETXATTRA");
> +		exitcode = 1;
> +		return;
> +	}
> +
> +	printf(_("fsxattr.naextents = %u\n"), fsxa.fsx_nextents);
> +
>  	if ((xfsctl(file->name, file->fd, XFS_IOC_DIOINFO, &dio)) < 0) {
>  		perror("XFS_IOC_DIOINFO");
> -	} else {
> -		printf(_("dioattr.mem = 0x%x\n"), dio.d_mem);
> -		printf(_("dioattr.miniosz = %u\n"), dio.d_miniosz);
> -		printf(_("dioattr.maxiosz = %u\n"), dio.d_maxiosz);
> +		exitcode = 1;
> +		return;
>  	}
> +
> +	printf(_("dioattr.mem = 0x%x\n"), dio.d_mem);
> +	printf(_("dioattr.miniosz = %u\n"), dio.d_miniosz);
> +	printf(_("dioattr.maxiosz = %u\n"), dio.d_maxiosz);
>  }
>  
>  int
> @@ -167,10 +182,7 @@ stat_f(
>  		printf(_("stat.ctime = %s"), ctime(&st.st_ctime));
>  	}
>  
> -	if (file->flags & IO_FOREIGN)
> -		return 0;
> -
> -	print_xfs_info(verbose);
> +	print_extended_info(verbose);
>  
>  	return 0;
>  }
> @@ -440,10 +452,7 @@ statx_f(
>  				ctime((time_t *)&stx.stx_btime.tv_sec));
>  	}
>  
> -	if (file->flags & IO_FOREIGN)
> -		return 0;
> -
> -	print_xfs_info(verbose);
> +	print_extended_info(verbose);
>  
>  	return 0;
>  }
> -- 
> 2.43.5
> 
>