Re: [RFC 2/3] xfs_io: Add ext4 support to show FS_IOC_FSGETXATTR details

[Darrick J. Wong]

On Wed, Dec 11, 2024 at 01:24:03PM +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 make sure to show these details when -c "stat" or "statx"
> is used.
> 
> No functional changes for filesystems other than ext4.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>
> ---
>  io/stat.c | 38 +++++++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/io/stat.c b/io/stat.c
> index 326f2822e276..d06c2186cde4 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -97,14 +97,14 @@ print_file_info(void)
>  		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
>  }
>  
> -static void
> -print_xfs_info(int verbose)
> +static void print_extended_info(int verbose)
>  {
> -	struct dioattr	dio;
> -	struct fsxattr	fsx, fsxa;
> +	struct dioattr dio;
> +	struct fsxattr fsx, fsxa;
> +	bool is_xfs_fd = platform_test_xfs_fd(file->fd);
>  
> -	if ((xfsctl(file->name, file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0 ||
> -	    (xfsctl(file->name, file->fd, XFS_IOC_FSGETXATTRA, &fsxa)) < 0) {
> +	if ((ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0 ||
> +		(is_xfs_fd && (xfsctl(file->name, file->fd, XFS_IOC_FSGETXATTRA, &fsxa) < 0))) {

Urgh... perhaps we should call FS_IOC_FSGETXATTR and if it returns zero
print whatever is returned, no matter what filesystem we think is
feeding us information?

e.g.

	if (ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) {
		if (is_xfs_fd || (errno != EOPNOTSUPP &&
				  errno != ENOTTY))
			perror("FS_IOC_GETXATTR");
	} else {
		printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags);
		...
	}

	if (ioctl(file->fd, XFS_IOC_FSGETXATTRA, &fsxa)) < 0) {
		if (is_xfs_fd || (errno != EOPNOTSUPP &&
				  errno != ENOTTY))
			perror("XFS_IOC_FSGETXATTRA");
	} else {
		printf(_("fsxattr.naextents = %u\n"), fsxa.fsx_nextents);
	}

That way we don't have to specialcase platform_test_*_fd() for every
other filesystem that might want to return real fsxattr results?
Same idea for DIOINFO.

--D

>  		perror("FS_IOC_FSGETXATTR");
>  	} else {
>  		printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags);
> @@ -113,14 +113,18 @@ print_xfs_info(int verbose)
>  		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);
> +		if (is_xfs_fd)
> +			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);
> +
> +	if (is_xfs_fd) {
> +		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);
> +		}
>  	}
>  }
>  
> @@ -167,10 +171,10 @@ stat_f(
>  		printf(_("stat.ctime = %s"), ctime(&st.st_ctime));
>  	}
>  
> -	if (file->flags & IO_FOREIGN)
> +	if (file->flags & IO_FOREIGN && !platform_test_ext4_fd(file->fd))
>  		return 0;
>  
> -	print_xfs_info(verbose);
> +	print_extended_info(verbose);
>  
>  	return 0;
>  }
> @@ -440,10 +444,10 @@ statx_f(
>  				ctime((time_t *)&stx.stx_btime.tv_sec));
>  	}
>  
> -	if (file->flags & IO_FOREIGN)
> +	if (file->flags & IO_FOREIGN && !platform_test_ext4_fd(file->fd))
>  		return 0;
>  
> -	print_xfs_info(verbose);
> +	print_extended_info(verbose);
>  
>  	return 0;
>  }
> -- 
> 2.43.5
> 
>