Re: [PATCH 2/2 V2] xfs_io: hook up statx

[Eric Biggers <ebiggers3@gmail.com>]

Hi Eric,

On Thu, Mar 23, 2017 at 11:45:45PM -0500, Eric Sandeen wrote:
> Wire up the statx syscall to xfs_io.
> 

I haven't reviewed these patches in detail, but just a few nits:

Given that the command operates on an open file, wouldn't it make more sense to
call statx() with the file descriptor and a NULL path?  It wouldn't be ideal for
test coverage, but it seems xfs_io isn't currently designed to work otherwise.
And note that the 'stat' xfs_io command already calls fstat(), not stat().

(In particular think about what should happen if you open a file through a
symlink, then execute both the 'stat' and 'statx' commands... with the current
proposal 'stat' would operate on the file, while 'statx' would operate on the
symlink by default.  Seems inconsistent.)

> diff --git a/io/init.c b/io/init.c
> index 06002e6..c15a1e1 100644
> --- a/io/init.c
> +++ b/io/init.c
> @@ -86,6 +86,7 @@ init_commands(void)
>  	seek_init();
>  	sendfile_init();
>  	shutdown_init();
> +	stat_init();
>  	sync_init();

The call to stat_init() should be added in patch 1 instead of patch 2, since
otherwise the 'stat' command is unavailable after applying just patch 1.

> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 19e1ae4..022f0ea 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -880,6 +880,32 @@ and the XFS_IOC_GETXATTR system call on the current file. If the
>  option is specified, the atime (last access), mtime
>  (last modify), and ctime (last change) timestamps are also displayed.
>  .TP
> +.BR statx " [ " \-O " | " "\-m mask" " ][ \-" FDLA " ]"
> +Extended information from the statx syscall.
> +.RS 1.0i
> +.PD 0
> +.TP 0.4i
> +.B \-m mask
> +Specify the field mask for the statx call (default STATX_ALL)
> +.TP
> +.B \-O
> +Add only basic stats (STATX_BASIC_STATS) to default mask
> +.TP
> +.B \-F
> +Force the attributes to be sync'd with the server
> +.TP
> +.B \-D
> +Don't sync attributes with the server
> +.TP
> +.B \-L
> +Follow symlinks (statx link target)
> +.TP
> +.B \-A
> +Suppress terminal automount traversal
> +.TP
> +.RE
> +.IP
> +.TP
>  .B statfs
>  Selected statistics from
>  .BR statfs (2)

This re-introduces the issue where the later text in the formatted man page is
indented too much.  Removing the extra .TP and .IP, so that the source looks
like the following, appears to fix it:

.TP
.B \-A
Suppress terminal automount traversal
.RE
.TP
.B statfs


- Eric