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

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

[Eric Sandeen]

These 2 patches are a second pass to add a statx command
to xfs_io in hopes that it will aid creation of xfstests
statx regression tests.

xfs_io> help statx
statx [-O | -m mask][-FDLA] -- extended information about the currently open file

 Display extended file status.

 Options:
 -m mask -- Specify the field mask for the statx call (default STATX_ALL)
 -A -- Suppress terminal automount traversal
 -D -- Don't sync attributes with the server
 -F -- Force the attributes to be sync'd with the server
 -L -- Follow symlinks (statx link target)
 -O -- Add only basic stats (STATX_BASIC_STATS) to default mask

xfs_io> statx
stx_mask: 0xfff
stx_blksize: 4096
stx_attributes: 0x70
stx_nlink: 1
stx_uid: 0
stx_gid: 0
stx_mode: 0100644
stx_ino: 99
stx_size: 0
stx_blocks: 0
stx_atime.tv_sec: 1490109633
stx_atime.tv_nsec: 676550238
stx_btime.tv_sec: 1490109633
stx_btime.tv_nsec: 675550234
stx_ctime.tv_sec: 1490109725
stx_ctime.tv_nsec: 69966839
stx_mtime.tv_sec: 1490109633
stx_mtime.tv_nsec: 676550238
stx_rdev_major: 0
stx_rdev_minor: 0
stx_dev_major: 7
stx_dev_minor: 0

patch 2 includes a header file with lots of the new #defines
so this can build on older systems; eventually that can be
removed perhaps.

An interesting limitation of all of the xfs_io stat
variants is that they currently can't stat any file that
they can't open, which may or may not be a limitation that
we care about....

-Eric

[PATCH 1/2 V2] xfs_io: move stat functions to new file

[Eric Sandeen]

Adding statx will add a bit of code, so break stat-related
functions out of open.c into their own new file.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: No change

diff --git a/io/Makefile b/io/Makefile
index 32df568..435ccff 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -11,7 +11,7 @@ HFILES = init.h io.h
 CFILES = init.c \
 	attr.c bmap.c cowextsize.c encrypt.c file.c freeze.c fsync.c \
 	getrusage.c imap.c link.c mmap.c open.c parent.c pread.c prealloc.c \
-	pwrite.c reflink.c seek.c shutdown.c sync.c truncate.c utimes.c
+	pwrite.c reflink.c seek.c shutdown.c stat.c sync.c truncate.c utimes.c
 
 LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD)
 LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE)
diff --git a/io/io.h b/io/io.h
index c40aad0..7e95bd5 100644
--- a/io/io.h
+++ b/io/io.h
@@ -53,7 +53,7 @@ extern fileio_t		*filetable;	/* open file table */
 extern int		filecount;	/* number of open files */
 extern fileio_t		*file;		/* active file in file table */
 extern int filelist_f(void);
-
+extern int stat_f(int argc, char **argv);
 /*
  * Memory mapped file regions
  */
diff --git a/io/open.c b/io/open.c
index 941fdc1..2ed55cf 100644
--- a/io/open.c
+++ b/io/open.c
@@ -39,9 +39,7 @@
 #endif
 
 static cmdinfo_t open_cmd;
-static cmdinfo_t stat_cmd;
 static cmdinfo_t close_cmd;
-static cmdinfo_t statfs_cmd;
 static cmdinfo_t chproj_cmd;
 static cmdinfo_t lsproj_cmd;
 static cmdinfo_t extsize_cmd;
@@ -49,96 +47,6 @@ static cmdinfo_t inode_cmd;
 static prid_t prid;
 static long extsize;
 
-off64_t
-filesize(void)
-{
-	struct stat	st;
-
-	if (fstat(file->fd, &st) < 0) {
-		perror("fstat");
-		return -1;
-	}
-	return st.st_size;
-}
-
-static char *
-filetype(mode_t mode)
-{
-	switch (mode & S_IFMT) {
-	case S_IFSOCK:
-		return _("socket");
-	case S_IFDIR:
-		return _("directory");
-	case S_IFCHR:
-		return _("char device");
-	case S_IFBLK:
-		return _("block device");
-	case S_IFREG:
-		return _("regular file");
-	case S_IFLNK:
-		return _("symbolic link");
-	case S_IFIFO:
-		return _("fifo");
-	}
-	return NULL;
-}
-
-static int
-stat_f(
-	int		argc,
-	char		**argv)
-{
-	struct dioattr	dio;
-	struct fsxattr	fsx, fsxa;
-	struct stat	st;
-	int		verbose = (argc == 2 && !strcmp(argv[1], "-v"));
-
-	printf(_("fd.path = \"%s\"\n"), file->name);
-	printf(_("fd.flags = %s,%s,%s%s%s%s%s\n"),
-		file->flags & IO_OSYNC ? _("sync") : _("non-sync"),
-		file->flags & IO_DIRECT ? _("direct") : _("non-direct"),
-		file->flags & IO_READONLY ? _("read-only") : _("read-write"),
-		file->flags & IO_REALTIME ? _(",real-time") : "",
-		file->flags & IO_APPEND ? _(",append-only") : "",
-		file->flags & IO_NONBLOCK ? _(",non-block") : "",
-		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
-	if (fstat(file->fd, &st) < 0) {
-		perror("fstat");
-	} else {
-		printf(_("stat.ino = %lld\n"), (long long)st.st_ino);
-		printf(_("stat.type = %s\n"), filetype(st.st_mode));
-		printf(_("stat.size = %lld\n"), (long long)st.st_size);
-		printf(_("stat.blocks = %lld\n"), (long long)st.st_blocks);
-		if (verbose) {
-			printf(_("stat.atime = %s"), ctime(&st.st_atime));
-			printf(_("stat.mtime = %s"), ctime(&st.st_mtime));
-			printf(_("stat.ctime = %s"), ctime(&st.st_ctime));
-		}
-	}
-	if (file->flags & IO_FOREIGN)
-		return 0;
-	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);
-	}
-	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);
-	}
-	return 0;
-}
-
 int
 openfile(
 	char		*path,
@@ -697,58 +605,6 @@ extsize_f(
 	return 0;
 }
 
-static int
-statfs_f(
-	int			argc,
-	char			**argv)
-{
-	struct xfs_fsop_counts	fscounts;
-	struct xfs_fsop_geom	fsgeo;
-	struct statfs		st;
-
-	printf(_("fd.path = \"%s\"\n"), file->name);
-	if (platform_fstatfs(file->fd, &st) < 0) {
-		perror("fstatfs");
-	} else {
-		printf(_("statfs.f_bsize = %lld\n"), (long long) st.f_bsize);
-		printf(_("statfs.f_blocks = %lld\n"), (long long) st.f_blocks);
-		printf(_("statfs.f_bavail = %lld\n"), (long long) st.f_bavail);
-		printf(_("statfs.f_files = %lld\n"), (long long) st.f_files);
-		printf(_("statfs.f_ffree = %lld\n"), (long long) st.f_ffree);
-	}
-	if (file->flags & IO_FOREIGN)
-		return 0;
-	if ((xfsctl(file->name, file->fd, XFS_IOC_FSGEOMETRY_V1, &fsgeo)) < 0) {
-		perror("XFS_IOC_FSGEOMETRY_V1");
-	} else {
-		printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
-		printf(_("geom.agcount = %u\n"), fsgeo.agcount);
-		printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
-		printf(_("geom.datablocks = %llu\n"),
-			(unsigned long long) fsgeo.datablocks);
-		printf(_("geom.rtblocks = %llu\n"),
-			(unsigned long long) fsgeo.rtblocks);
-		printf(_("geom.rtextents = %llu\n"),
-			(unsigned long long) fsgeo.rtextents);
-		printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
-		printf(_("geom.sunit = %u\n"), fsgeo.sunit);
-		printf(_("geom.swidth = %u\n"), fsgeo.swidth);
-	}
-	if ((xfsctl(file->name, file->fd, XFS_IOC_FSCOUNTS, &fscounts)) < 0) {
-		perror("XFS_IOC_FSCOUNTS");
-	} else {
-		printf(_("counts.freedata = %llu\n"),
-			(unsigned long long) fscounts.freedata);
-		printf(_("counts.freertx = %llu\n"),
-			(unsigned long long) fscounts.freertx);
-		printf(_("counts.freeino = %llu\n"),
-			(unsigned long long) fscounts.freeino);
-		printf(_("counts.allocino = %llu\n"),
-			(unsigned long long) fscounts.allocino);
-	}
-	return 0;
-}
-
 static void
 inode_help(void)
 {
@@ -920,14 +776,6 @@ open_init(void)
 	open_cmd.oneline = _("open the file specified by path");
 	open_cmd.help = open_help;
 
-	stat_cmd.name = "stat";
-	stat_cmd.cfunc = stat_f;
-	stat_cmd.argmin = 0;
-	stat_cmd.argmax = 1;
-	stat_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	stat_cmd.args = _("[-v]");
-	stat_cmd.oneline = _("statistics on the currently open file");
-
 	close_cmd.name = "close";
 	close_cmd.altname = "c";
 	close_cmd.cfunc = close_f;
@@ -936,12 +784,6 @@ open_init(void)
 	close_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	close_cmd.oneline = _("close the current open file");
 
-	statfs_cmd.name = "statfs";
-	statfs_cmd.cfunc = statfs_f;
-	statfs_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	statfs_cmd.oneline =
-		_("statistics on the filesystem of the currently open file");
-
 	chproj_cmd.name = "chproj";
 	chproj_cmd.cfunc = chproj_f;
 	chproj_cmd.args = _("[-D | -R] projid");
@@ -983,9 +825,7 @@ open_init(void)
 	inode_cmd.help = inode_help;
 
 	add_command(&open_cmd);
-	add_command(&stat_cmd);
 	add_command(&close_cmd);
-	add_command(&statfs_cmd);
 	add_command(&chproj_cmd);
 	add_command(&lsproj_cmd);
 	add_command(&extsize_cmd);
diff --git a/io/stat.c b/io/stat.c
new file mode 100644
index 0000000..3ae9903
--- /dev/null
+++ b/io/stat.c
@@ -0,0 +1,189 @@
+/*
+ * Copyright (c) 2003-2005 Silicon Graphics, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include "command.h"
+#include "input.h"
+#include "init.h"
+#include "io.h"
+#include "libxfs.h"
+
+static cmdinfo_t stat_cmd;
+static cmdinfo_t statfs_cmd;
+
+off64_t
+filesize(void)
+{
+	struct stat	st;
+
+	if (fstat(file->fd, &st) < 0) {
+		perror("fstat");
+		return -1;
+	}
+	return st.st_size;
+}
+
+static char *
+filetype(mode_t mode)
+{
+	switch (mode & S_IFMT) {
+	case S_IFSOCK:
+		return _("socket");
+	case S_IFDIR:
+		return _("directory");
+	case S_IFCHR:
+		return _("char device");
+	case S_IFBLK:
+		return _("block device");
+	case S_IFREG:
+		return _("regular file");
+	case S_IFLNK:
+		return _("symbolic link");
+	case S_IFIFO:
+		return _("fifo");
+	}
+	return NULL;
+}
+
+int
+stat_f(
+	int		argc,
+	char		**argv)
+{
+	struct dioattr	dio;
+	struct fsxattr	fsx, fsxa;
+	struct stat	st;
+	int		verbose = (argc == 2 && !strcmp(argv[1], "-v"));
+
+	printf(_("fd.path = \"%s\"\n"), file->name);
+	printf(_("fd.flags = %s,%s,%s%s%s%s%s\n"),
+		file->flags & IO_OSYNC ? _("sync") : _("non-sync"),
+		file->flags & IO_DIRECT ? _("direct") : _("non-direct"),
+		file->flags & IO_READONLY ? _("read-only") : _("read-write"),
+		file->flags & IO_REALTIME ? _(",real-time") : "",
+		file->flags & IO_APPEND ? _(",append-only") : "",
+		file->flags & IO_NONBLOCK ? _(",non-block") : "",
+		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
+	if (fstat(file->fd, &st) < 0) {
+		perror("fstat");
+	} else {
+		printf(_("stat.ino = %lld\n"), (long long)st.st_ino);
+		printf(_("stat.type = %s\n"), filetype(st.st_mode));
+		printf(_("stat.size = %lld\n"), (long long)st.st_size);
+		printf(_("stat.blocks = %lld\n"), (long long)st.st_blocks);
+		if (verbose) {
+			printf(_("stat.atime = %s"), ctime(&st.st_atime));
+			printf(_("stat.mtime = %s"), ctime(&st.st_mtime));
+			printf(_("stat.ctime = %s"), ctime(&st.st_ctime));
+		}
+	}
+	if (file->flags & IO_FOREIGN)
+		return 0;
+	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);
+	}
+	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);
+	}
+	return 0;
+}
+
+static int
+statfs_f(
+	int			argc,
+	char			**argv)
+{
+	struct xfs_fsop_counts	fscounts;
+	struct xfs_fsop_geom	fsgeo;
+	struct statfs		st;
+
+	printf(_("fd.path = \"%s\"\n"), file->name);
+	if (platform_fstatfs(file->fd, &st) < 0) {
+		perror("fstatfs");
+	} else {
+		printf(_("statfs.f_bsize = %lld\n"), (long long) st.f_bsize);
+		printf(_("statfs.f_blocks = %lld\n"), (long long) st.f_blocks);
+		printf(_("statfs.f_bavail = %lld\n"), (long long) st.f_bavail);
+		printf(_("statfs.f_files = %lld\n"), (long long) st.f_files);
+		printf(_("statfs.f_ffree = %lld\n"), (long long) st.f_ffree);
+	}
+	if (file->flags & IO_FOREIGN)
+		return 0;
+	if ((xfsctl(file->name, file->fd, XFS_IOC_FSGEOMETRY_V1, &fsgeo)) < 0) {
+		perror("XFS_IOC_FSGEOMETRY_V1");
+	} else {
+		printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
+		printf(_("geom.agcount = %u\n"), fsgeo.agcount);
+		printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
+		printf(_("geom.datablocks = %llu\n"),
+			(unsigned long long) fsgeo.datablocks);
+		printf(_("geom.rtblocks = %llu\n"),
+			(unsigned long long) fsgeo.rtblocks);
+		printf(_("geom.rtextents = %llu\n"),
+			(unsigned long long) fsgeo.rtextents);
+		printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
+		printf(_("geom.sunit = %u\n"), fsgeo.sunit);
+		printf(_("geom.swidth = %u\n"), fsgeo.swidth);
+	}
+	if ((xfsctl(file->name, file->fd, XFS_IOC_FSCOUNTS, &fscounts)) < 0) {
+		perror("XFS_IOC_FSCOUNTS");
+	} else {
+		printf(_("counts.freedata = %llu\n"),
+			(unsigned long long) fscounts.freedata);
+		printf(_("counts.freertx = %llu\n"),
+			(unsigned long long) fscounts.freertx);
+		printf(_("counts.freeino = %llu\n"),
+			(unsigned long long) fscounts.freeino);
+		printf(_("counts.allocino = %llu\n"),
+			(unsigned long long) fscounts.allocino);
+	}
+	return 0;
+}
+
+void
+stat_init(void)
+{
+	stat_cmd.name = "stat";
+	stat_cmd.cfunc = stat_f;
+	stat_cmd.argmin = 0;
+	stat_cmd.argmax = 1;
+	stat_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	stat_cmd.args = _("[-v]");
+	stat_cmd.oneline = _("statistics on the currently open file");
+
+	statfs_cmd.name = "statfs";
+	statfs_cmd.cfunc = statfs_f;
+	statfs_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	statfs_cmd.oneline =
+		_("statistics on the filesystem of the currently open file");
+
+	add_command(&stat_cmd);
+	add_command(&statfs_cmd);
+}

[PATCH 1/2 V2] xfs_io: move stat functions to new file

[Eric Sandeen]

Adding statx will add a bit of code, so break stat-related
functions out of open.c into their own new file.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: No change

diff --git a/io/Makefile b/io/Makefile
index 32df568..435ccff 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -11,7 +11,7 @@ HFILES = init.h io.h
 CFILES = init.c \
 	attr.c bmap.c cowextsize.c encrypt.c file.c freeze.c fsync.c \
 	getrusage.c imap.c link.c mmap.c open.c parent.c pread.c prealloc.c \
-	pwrite.c reflink.c seek.c shutdown.c sync.c truncate.c utimes.c
+	pwrite.c reflink.c seek.c shutdown.c stat.c sync.c truncate.c utimes.c
 
 LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD)
 LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE)
diff --git a/io/io.h b/io/io.h
index c40aad0..7e95bd5 100644
--- a/io/io.h
+++ b/io/io.h
@@ -53,7 +53,7 @@ extern fileio_t		*filetable;	/* open file table */
 extern int		filecount;	/* number of open files */
 extern fileio_t		*file;		/* active file in file table */
 extern int filelist_f(void);
-
+extern int stat_f(int argc, char **argv);
 /*
  * Memory mapped file regions
  */
diff --git a/io/open.c b/io/open.c
index 941fdc1..2ed55cf 100644
--- a/io/open.c
+++ b/io/open.c
@@ -39,9 +39,7 @@
 #endif
 
 static cmdinfo_t open_cmd;
-static cmdinfo_t stat_cmd;
 static cmdinfo_t close_cmd;
-static cmdinfo_t statfs_cmd;
 static cmdinfo_t chproj_cmd;
 static cmdinfo_t lsproj_cmd;
 static cmdinfo_t extsize_cmd;
@@ -49,96 +47,6 @@ static cmdinfo_t inode_cmd;
 static prid_t prid;
 static long extsize;
 
-off64_t
-filesize(void)
-{
-	struct stat	st;
-
-	if (fstat(file->fd, &st) < 0) {
-		perror("fstat");
-		return -1;
-	}
-	return st.st_size;
-}
-
-static char *
-filetype(mode_t mode)
-{
-	switch (mode & S_IFMT) {
-	case S_IFSOCK:
-		return _("socket");
-	case S_IFDIR:
-		return _("directory");
-	case S_IFCHR:
-		return _("char device");
-	case S_IFBLK:
-		return _("block device");
-	case S_IFREG:
-		return _("regular file");
-	case S_IFLNK:
-		return _("symbolic link");
-	case S_IFIFO:
-		return _("fifo");
-	}
-	return NULL;
-}
-
-static int
-stat_f(
-	int		argc,
-	char		**argv)
-{
-	struct dioattr	dio;
-	struct fsxattr	fsx, fsxa;
-	struct stat	st;
-	int		verbose = (argc == 2 && !strcmp(argv[1], "-v"));
-
-	printf(_("fd.path = \"%s\"\n"), file->name);
-	printf(_("fd.flags = %s,%s,%s%s%s%s%s\n"),
-		file->flags & IO_OSYNC ? _("sync") : _("non-sync"),
-		file->flags & IO_DIRECT ? _("direct") : _("non-direct"),
-		file->flags & IO_READONLY ? _("read-only") : _("read-write"),
-		file->flags & IO_REALTIME ? _(",real-time") : "",
-		file->flags & IO_APPEND ? _(",append-only") : "",
-		file->flags & IO_NONBLOCK ? _(",non-block") : "",
-		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
-	if (fstat(file->fd, &st) < 0) {
-		perror("fstat");
-	} else {
-		printf(_("stat.ino = %lld\n"), (long long)st.st_ino);
-		printf(_("stat.type = %s\n"), filetype(st.st_mode));
-		printf(_("stat.size = %lld\n"), (long long)st.st_size);
-		printf(_("stat.blocks = %lld\n"), (long long)st.st_blocks);
-		if (verbose) {
-			printf(_("stat.atime = %s"), ctime(&st.st_atime));
-			printf(_("stat.mtime = %s"), ctime(&st.st_mtime));
-			printf(_("stat.ctime = %s"), ctime(&st.st_ctime));
-		}
-	}
-	if (file->flags & IO_FOREIGN)
-		return 0;
-	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);
-	}
-	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);
-	}
-	return 0;
-}
-
 int
 openfile(
 	char		*path,
@@ -697,58 +605,6 @@ extsize_f(
 	return 0;
 }
 
-static int
-statfs_f(
-	int			argc,
-	char			**argv)
-{
-	struct xfs_fsop_counts	fscounts;
-	struct xfs_fsop_geom	fsgeo;
-	struct statfs		st;
-
-	printf(_("fd.path = \"%s\"\n"), file->name);
-	if (platform_fstatfs(file->fd, &st) < 0) {
-		perror("fstatfs");
-	} else {
-		printf(_("statfs.f_bsize = %lld\n"), (long long) st.f_bsize);
-		printf(_("statfs.f_blocks = %lld\n"), (long long) st.f_blocks);
-		printf(_("statfs.f_bavail = %lld\n"), (long long) st.f_bavail);
-		printf(_("statfs.f_files = %lld\n"), (long long) st.f_files);
-		printf(_("statfs.f_ffree = %lld\n"), (long long) st.f_ffree);
-	}
-	if (file->flags & IO_FOREIGN)
-		return 0;
-	if ((xfsctl(file->name, file->fd, XFS_IOC_FSGEOMETRY_V1, &fsgeo)) < 0) {
-		perror("XFS_IOC_FSGEOMETRY_V1");
-	} else {
-		printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
-		printf(_("geom.agcount = %u\n"), fsgeo.agcount);
-		printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
-		printf(_("geom.datablocks = %llu\n"),
-			(unsigned long long) fsgeo.datablocks);
-		printf(_("geom.rtblocks = %llu\n"),
-			(unsigned long long) fsgeo.rtblocks);
-		printf(_("geom.rtextents = %llu\n"),
-			(unsigned long long) fsgeo.rtextents);
-		printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
-		printf(_("geom.sunit = %u\n"), fsgeo.sunit);
-		printf(_("geom.swidth = %u\n"), fsgeo.swidth);
-	}
-	if ((xfsctl(file->name, file->fd, XFS_IOC_FSCOUNTS, &fscounts)) < 0) {
-		perror("XFS_IOC_FSCOUNTS");
-	} else {
-		printf(_("counts.freedata = %llu\n"),
-			(unsigned long long) fscounts.freedata);
-		printf(_("counts.freertx = %llu\n"),
-			(unsigned long long) fscounts.freertx);
-		printf(_("counts.freeino = %llu\n"),
-			(unsigned long long) fscounts.freeino);
-		printf(_("counts.allocino = %llu\n"),
-			(unsigned long long) fscounts.allocino);
-	}
-	return 0;
-}
-
 static void
 inode_help(void)
 {
@@ -920,14 +776,6 @@ open_init(void)
 	open_cmd.oneline = _("open the file specified by path");
 	open_cmd.help = open_help;
 
-	stat_cmd.name = "stat";
-	stat_cmd.cfunc = stat_f;
-	stat_cmd.argmin = 0;
-	stat_cmd.argmax = 1;
-	stat_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	stat_cmd.args = _("[-v]");
-	stat_cmd.oneline = _("statistics on the currently open file");
-
 	close_cmd.name = "close";
 	close_cmd.altname = "c";
 	close_cmd.cfunc = close_f;
@@ -936,12 +784,6 @@ open_init(void)
 	close_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	close_cmd.oneline = _("close the current open file");
 
-	statfs_cmd.name = "statfs";
-	statfs_cmd.cfunc = statfs_f;
-	statfs_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	statfs_cmd.oneline =
-		_("statistics on the filesystem of the currently open file");
-
 	chproj_cmd.name = "chproj";
 	chproj_cmd.cfunc = chproj_f;
 	chproj_cmd.args = _("[-D | -R] projid");
@@ -983,9 +825,7 @@ open_init(void)
 	inode_cmd.help = inode_help;
 
 	add_command(&open_cmd);
-	add_command(&stat_cmd);
 	add_command(&close_cmd);
-	add_command(&statfs_cmd);
 	add_command(&chproj_cmd);
 	add_command(&lsproj_cmd);
 	add_command(&extsize_cmd);
diff --git a/io/stat.c b/io/stat.c
new file mode 100644
index 0000000..3ae9903
--- /dev/null
+++ b/io/stat.c
@@ -0,0 +1,189 @@
+/*
+ * Copyright (c) 2003-2005 Silicon Graphics, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include "command.h"
+#include "input.h"
+#include "init.h"
+#include "io.h"
+#include "libxfs.h"
+
+static cmdinfo_t stat_cmd;
+static cmdinfo_t statfs_cmd;
+
+off64_t
+filesize(void)
+{
+	struct stat	st;
+
+	if (fstat(file->fd, &st) < 0) {
+		perror("fstat");
+		return -1;
+	}
+	return st.st_size;
+}
+
+static char *
+filetype(mode_t mode)
+{
+	switch (mode & S_IFMT) {
+	case S_IFSOCK:
+		return _("socket");
+	case S_IFDIR:
+		return _("directory");
+	case S_IFCHR:
+		return _("char device");
+	case S_IFBLK:
+		return _("block device");
+	case S_IFREG:
+		return _("regular file");
+	case S_IFLNK:
+		return _("symbolic link");
+	case S_IFIFO:
+		return _("fifo");
+	}
+	return NULL;
+}
+
+int
+stat_f(
+	int		argc,
+	char		**argv)
+{
+	struct dioattr	dio;
+	struct fsxattr	fsx, fsxa;
+	struct stat	st;
+	int		verbose = (argc == 2 && !strcmp(argv[1], "-v"));
+
+	printf(_("fd.path = \"%s\"\n"), file->name);
+	printf(_("fd.flags = %s,%s,%s%s%s%s%s\n"),
+		file->flags & IO_OSYNC ? _("sync") : _("non-sync"),
+		file->flags & IO_DIRECT ? _("direct") : _("non-direct"),
+		file->flags & IO_READONLY ? _("read-only") : _("read-write"),
+		file->flags & IO_REALTIME ? _(",real-time") : "",
+		file->flags & IO_APPEND ? _(",append-only") : "",
+		file->flags & IO_NONBLOCK ? _(",non-block") : "",
+		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
+	if (fstat(file->fd, &st) < 0) {
+		perror("fstat");
+	} else {
+		printf(_("stat.ino = %lld\n"), (long long)st.st_ino);
+		printf(_("stat.type = %s\n"), filetype(st.st_mode));
+		printf(_("stat.size = %lld\n"), (long long)st.st_size);
+		printf(_("stat.blocks = %lld\n"), (long long)st.st_blocks);
+		if (verbose) {
+			printf(_("stat.atime = %s"), ctime(&st.st_atime));
+			printf(_("stat.mtime = %s"), ctime(&st.st_mtime));
+			printf(_("stat.ctime = %s"), ctime(&st.st_ctime));
+		}
+	}
+	if (file->flags & IO_FOREIGN)
+		return 0;
+	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);
+	}
+	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);
+	}
+	return 0;
+}
+
+static int
+statfs_f(
+	int			argc,
+	char			**argv)
+{
+	struct xfs_fsop_counts	fscounts;
+	struct xfs_fsop_geom	fsgeo;
+	struct statfs		st;
+
+	printf(_("fd.path = \"%s\"\n"), file->name);
+	if (platform_fstatfs(file->fd, &st) < 0) {
+		perror("fstatfs");
+	} else {
+		printf(_("statfs.f_bsize = %lld\n"), (long long) st.f_bsize);
+		printf(_("statfs.f_blocks = %lld\n"), (long long) st.f_blocks);
+		printf(_("statfs.f_bavail = %lld\n"), (long long) st.f_bavail);
+		printf(_("statfs.f_files = %lld\n"), (long long) st.f_files);
+		printf(_("statfs.f_ffree = %lld\n"), (long long) st.f_ffree);
+	}
+	if (file->flags & IO_FOREIGN)
+		return 0;
+	if ((xfsctl(file->name, file->fd, XFS_IOC_FSGEOMETRY_V1, &fsgeo)) < 0) {
+		perror("XFS_IOC_FSGEOMETRY_V1");
+	} else {
+		printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
+		printf(_("geom.agcount = %u\n"), fsgeo.agcount);
+		printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
+		printf(_("geom.datablocks = %llu\n"),
+			(unsigned long long) fsgeo.datablocks);
+		printf(_("geom.rtblocks = %llu\n"),
+			(unsigned long long) fsgeo.rtblocks);
+		printf(_("geom.rtextents = %llu\n"),
+			(unsigned long long) fsgeo.rtextents);
+		printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
+		printf(_("geom.sunit = %u\n"), fsgeo.sunit);
+		printf(_("geom.swidth = %u\n"), fsgeo.swidth);
+	}
+	if ((xfsctl(file->name, file->fd, XFS_IOC_FSCOUNTS, &fscounts)) < 0) {
+		perror("XFS_IOC_FSCOUNTS");
+	} else {
+		printf(_("counts.freedata = %llu\n"),
+			(unsigned long long) fscounts.freedata);
+		printf(_("counts.freertx = %llu\n"),
+			(unsigned long long) fscounts.freertx);
+		printf(_("counts.freeino = %llu\n"),
+			(unsigned long long) fscounts.freeino);
+		printf(_("counts.allocino = %llu\n"),
+			(unsigned long long) fscounts.allocino);
+	}
+	return 0;
+}
+
+void
+stat_init(void)
+{
+	stat_cmd.name = "stat";
+	stat_cmd.cfunc = stat_f;
+	stat_cmd.argmin = 0;
+	stat_cmd.argmax = 1;
+	stat_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	stat_cmd.args = _("[-v]");
+	stat_cmd.oneline = _("statistics on the currently open file");
+
+	statfs_cmd.name = "statfs";
+	statfs_cmd.cfunc = statfs_f;
+	statfs_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	statfs_cmd.oneline =
+		_("statistics on the filesystem of the currently open file");
+
+	add_command(&stat_cmd);
+	add_command(&statfs_cmd);
+}

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2 V2] xfs_io: move stat functions to new file

[Darrick J. Wong]

On Thu, Mar 23, 2017 at 11:34:23PM -0500, Eric Sandeen wrote:
> Adding statx will add a bit of code, so break stat-related
> functions out of open.c into their own new file.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> V2: No change
> 
> diff --git a/io/Makefile b/io/Makefile
> index 32df568..435ccff 100644
> --- a/io/Makefile
> +++ b/io/Makefile
> @@ -11,7 +11,7 @@ HFILES = init.h io.h
>  CFILES = init.c \
>  	attr.c bmap.c cowextsize.c encrypt.c file.c freeze.c fsync.c \
>  	getrusage.c imap.c link.c mmap.c open.c parent.c pread.c prealloc.c \
> -	pwrite.c reflink.c seek.c shutdown.c sync.c truncate.c utimes.c
> +	pwrite.c reflink.c seek.c shutdown.c stat.c sync.c truncate.c utimes.c
>  
>  LLDLIBS = $(LIBXCMD) $(LIBHANDLE) $(LIBPTHREAD)
>  LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE)
> diff --git a/io/io.h b/io/io.h
> index c40aad0..7e95bd5 100644
> --- a/io/io.h
> +++ b/io/io.h
> @@ -53,7 +53,7 @@ extern fileio_t		*filetable;	/* open file table */
>  extern int		filecount;	/* number of open files */
>  extern fileio_t		*file;		/* active file in file table */
>  extern int filelist_f(void);
> -
> +extern int stat_f(int argc, char **argv);
>  /*
>   * Memory mapped file regions
>   */
> diff --git a/io/open.c b/io/open.c
> index 941fdc1..2ed55cf 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -39,9 +39,7 @@
>  #endif
>  
>  static cmdinfo_t open_cmd;
> -static cmdinfo_t stat_cmd;
>  static cmdinfo_t close_cmd;
> -static cmdinfo_t statfs_cmd;
>  static cmdinfo_t chproj_cmd;
>  static cmdinfo_t lsproj_cmd;
>  static cmdinfo_t extsize_cmd;
> @@ -49,96 +47,6 @@ static cmdinfo_t inode_cmd;
>  static prid_t prid;
>  static long extsize;
>  
> -off64_t
> -filesize(void)
> -{
> -	struct stat	st;
> -
> -	if (fstat(file->fd, &st) < 0) {
> -		perror("fstat");
> -		return -1;
> -	}
> -	return st.st_size;
> -}
> -
> -static char *
> -filetype(mode_t mode)
> -{
> -	switch (mode & S_IFMT) {
> -	case S_IFSOCK:
> -		return _("socket");
> -	case S_IFDIR:
> -		return _("directory");
> -	case S_IFCHR:
> -		return _("char device");
> -	case S_IFBLK:
> -		return _("block device");
> -	case S_IFREG:
> -		return _("regular file");
> -	case S_IFLNK:
> -		return _("symbolic link");
> -	case S_IFIFO:
> -		return _("fifo");
> -	}
> -	return NULL;
> -}
> -
> -static int
> -stat_f(
> -	int		argc,
> -	char		**argv)
> -{
> -	struct dioattr	dio;
> -	struct fsxattr	fsx, fsxa;
> -	struct stat	st;
> -	int		verbose = (argc == 2 && !strcmp(argv[1], "-v"));
> -
> -	printf(_("fd.path = \"%s\"\n"), file->name);
> -	printf(_("fd.flags = %s,%s,%s%s%s%s%s\n"),
> -		file->flags & IO_OSYNC ? _("sync") : _("non-sync"),
> -		file->flags & IO_DIRECT ? _("direct") : _("non-direct"),
> -		file->flags & IO_READONLY ? _("read-only") : _("read-write"),
> -		file->flags & IO_REALTIME ? _(",real-time") : "",
> -		file->flags & IO_APPEND ? _(",append-only") : "",
> -		file->flags & IO_NONBLOCK ? _(",non-block") : "",
> -		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
> -	if (fstat(file->fd, &st) < 0) {
> -		perror("fstat");
> -	} else {
> -		printf(_("stat.ino = %lld\n"), (long long)st.st_ino);
> -		printf(_("stat.type = %s\n"), filetype(st.st_mode));
> -		printf(_("stat.size = %lld\n"), (long long)st.st_size);
> -		printf(_("stat.blocks = %lld\n"), (long long)st.st_blocks);
> -		if (verbose) {
> -			printf(_("stat.atime = %s"), ctime(&st.st_atime));
> -			printf(_("stat.mtime = %s"), ctime(&st.st_mtime));
> -			printf(_("stat.ctime = %s"), ctime(&st.st_ctime));
> -		}
> -	}
> -	if (file->flags & IO_FOREIGN)
> -		return 0;
> -	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);
> -	}
> -	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);
> -	}
> -	return 0;
> -}
> -
>  int
>  openfile(
>  	char		*path,
> @@ -697,58 +605,6 @@ extsize_f(
>  	return 0;
>  }
>  
> -static int
> -statfs_f(
> -	int			argc,
> -	char			**argv)
> -{
> -	struct xfs_fsop_counts	fscounts;
> -	struct xfs_fsop_geom	fsgeo;
> -	struct statfs		st;
> -
> -	printf(_("fd.path = \"%s\"\n"), file->name);
> -	if (platform_fstatfs(file->fd, &st) < 0) {
> -		perror("fstatfs");
> -	} else {
> -		printf(_("statfs.f_bsize = %lld\n"), (long long) st.f_bsize);
> -		printf(_("statfs.f_blocks = %lld\n"), (long long) st.f_blocks);
> -		printf(_("statfs.f_bavail = %lld\n"), (long long) st.f_bavail);
> -		printf(_("statfs.f_files = %lld\n"), (long long) st.f_files);
> -		printf(_("statfs.f_ffree = %lld\n"), (long long) st.f_ffree);
> -	}
> -	if (file->flags & IO_FOREIGN)
> -		return 0;
> -	if ((xfsctl(file->name, file->fd, XFS_IOC_FSGEOMETRY_V1, &fsgeo)) < 0) {
> -		perror("XFS_IOC_FSGEOMETRY_V1");
> -	} else {
> -		printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
> -		printf(_("geom.agcount = %u\n"), fsgeo.agcount);
> -		printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
> -		printf(_("geom.datablocks = %llu\n"),
> -			(unsigned long long) fsgeo.datablocks);
> -		printf(_("geom.rtblocks = %llu\n"),
> -			(unsigned long long) fsgeo.rtblocks);
> -		printf(_("geom.rtextents = %llu\n"),
> -			(unsigned long long) fsgeo.rtextents);
> -		printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
> -		printf(_("geom.sunit = %u\n"), fsgeo.sunit);
> -		printf(_("geom.swidth = %u\n"), fsgeo.swidth);
> -	}
> -	if ((xfsctl(file->name, file->fd, XFS_IOC_FSCOUNTS, &fscounts)) < 0) {
> -		perror("XFS_IOC_FSCOUNTS");
> -	} else {
> -		printf(_("counts.freedata = %llu\n"),
> -			(unsigned long long) fscounts.freedata);
> -		printf(_("counts.freertx = %llu\n"),
> -			(unsigned long long) fscounts.freertx);
> -		printf(_("counts.freeino = %llu\n"),
> -			(unsigned long long) fscounts.freeino);
> -		printf(_("counts.allocino = %llu\n"),
> -			(unsigned long long) fscounts.allocino);
> -	}
> -	return 0;
> -}
> -
>  static void
>  inode_help(void)
>  {
> @@ -920,14 +776,6 @@ open_init(void)
>  	open_cmd.oneline = _("open the file specified by path");
>  	open_cmd.help = open_help;
>  
> -	stat_cmd.name = "stat";
> -	stat_cmd.cfunc = stat_f;
> -	stat_cmd.argmin = 0;
> -	stat_cmd.argmax = 1;
> -	stat_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> -	stat_cmd.args = _("[-v]");
> -	stat_cmd.oneline = _("statistics on the currently open file");
> -
>  	close_cmd.name = "close";
>  	close_cmd.altname = "c";
>  	close_cmd.cfunc = close_f;
> @@ -936,12 +784,6 @@ open_init(void)
>  	close_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
>  	close_cmd.oneline = _("close the current open file");
>  
> -	statfs_cmd.name = "statfs";
> -	statfs_cmd.cfunc = statfs_f;
> -	statfs_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> -	statfs_cmd.oneline =
> -		_("statistics on the filesystem of the currently open file");
> -
>  	chproj_cmd.name = "chproj";
>  	chproj_cmd.cfunc = chproj_f;
>  	chproj_cmd.args = _("[-D | -R] projid");
> @@ -983,9 +825,7 @@ open_init(void)
>  	inode_cmd.help = inode_help;
>  
>  	add_command(&open_cmd);
> -	add_command(&stat_cmd);
>  	add_command(&close_cmd);
> -	add_command(&statfs_cmd);
>  	add_command(&chproj_cmd);
>  	add_command(&lsproj_cmd);
>  	add_command(&extsize_cmd);
> diff --git a/io/stat.c b/io/stat.c
> new file mode 100644
> index 0000000..3ae9903
> --- /dev/null
> +++ b/io/stat.c
> @@ -0,0 +1,189 @@
> +/*
> + * Copyright (c) 2003-2005 Silicon Graphics, Inc.
> + * All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write the Free Software Foundation,
> + * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + */
> +
> +#include "command.h"
> +#include "input.h"
> +#include "init.h"
> +#include "io.h"
> +#include "libxfs.h"
> +
> +static cmdinfo_t stat_cmd;
> +static cmdinfo_t statfs_cmd;
> +
> +off64_t
> +filesize(void)
> +{
> +	struct stat	st;
> +
> +	if (fstat(file->fd, &st) < 0) {
> +		perror("fstat");
> +		return -1;
> +	}
> +	return st.st_size;
> +}
> +
> +static char *
> +filetype(mode_t mode)
> +{
> +	switch (mode & S_IFMT) {
> +	case S_IFSOCK:
> +		return _("socket");
> +	case S_IFDIR:
> +		return _("directory");
> +	case S_IFCHR:
> +		return _("char device");
> +	case S_IFBLK:
> +		return _("block device");
> +	case S_IFREG:
> +		return _("regular file");
> +	case S_IFLNK:
> +		return _("symbolic link");
> +	case S_IFIFO:
> +		return _("fifo");
> +	}
> +	return NULL;
> +}
> +
> +int
> +stat_f(
> +	int		argc,
> +	char		**argv)
> +{
> +	struct dioattr	dio;
> +	struct fsxattr	fsx, fsxa;
> +	struct stat	st;
> +	int		verbose = (argc == 2 && !strcmp(argv[1], "-v"));
> +
> +	printf(_("fd.path = \"%s\"\n"), file->name);
> +	printf(_("fd.flags = %s,%s,%s%s%s%s%s\n"),
> +		file->flags & IO_OSYNC ? _("sync") : _("non-sync"),
> +		file->flags & IO_DIRECT ? _("direct") : _("non-direct"),
> +		file->flags & IO_READONLY ? _("read-only") : _("read-write"),
> +		file->flags & IO_REALTIME ? _(",real-time") : "",
> +		file->flags & IO_APPEND ? _(",append-only") : "",
> +		file->flags & IO_NONBLOCK ? _(",non-block") : "",
> +		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
> +	if (fstat(file->fd, &st) < 0) {
> +		perror("fstat");
> +	} else {
> +		printf(_("stat.ino = %lld\n"), (long long)st.st_ino);
> +		printf(_("stat.type = %s\n"), filetype(st.st_mode));
> +		printf(_("stat.size = %lld\n"), (long long)st.st_size);
> +		printf(_("stat.blocks = %lld\n"), (long long)st.st_blocks);
> +		if (verbose) {
> +			printf(_("stat.atime = %s"), ctime(&st.st_atime));
> +			printf(_("stat.mtime = %s"), ctime(&st.st_mtime));
> +			printf(_("stat.ctime = %s"), ctime(&st.st_ctime));
> +		}
> +	}
> +	if (file->flags & IO_FOREIGN)
> +		return 0;
> +	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);
> +	}
> +	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);
> +	}
> +	return 0;
> +}
> +
> +static int
> +statfs_f(
> +	int			argc,
> +	char			**argv)
> +{
> +	struct xfs_fsop_counts	fscounts;
> +	struct xfs_fsop_geom	fsgeo;
> +	struct statfs		st;
> +
> +	printf(_("fd.path = \"%s\"\n"), file->name);
> +	if (platform_fstatfs(file->fd, &st) < 0) {
> +		perror("fstatfs");
> +	} else {
> +		printf(_("statfs.f_bsize = %lld\n"), (long long) st.f_bsize);
> +		printf(_("statfs.f_blocks = %lld\n"), (long long) st.f_blocks);
> +		printf(_("statfs.f_bavail = %lld\n"), (long long) st.f_bavail);
> +		printf(_("statfs.f_files = %lld\n"), (long long) st.f_files);
> +		printf(_("statfs.f_ffree = %lld\n"), (long long) st.f_ffree);
> +	}
> +	if (file->flags & IO_FOREIGN)
> +		return 0;
> +	if ((xfsctl(file->name, file->fd, XFS_IOC_FSGEOMETRY_V1, &fsgeo)) < 0) {
> +		perror("XFS_IOC_FSGEOMETRY_V1");
> +	} else {
> +		printf(_("geom.bsize = %u\n"), fsgeo.blocksize);
> +		printf(_("geom.agcount = %u\n"), fsgeo.agcount);
> +		printf(_("geom.agblocks = %u\n"), fsgeo.agblocks);
> +		printf(_("geom.datablocks = %llu\n"),
> +			(unsigned long long) fsgeo.datablocks);
> +		printf(_("geom.rtblocks = %llu\n"),
> +			(unsigned long long) fsgeo.rtblocks);
> +		printf(_("geom.rtextents = %llu\n"),
> +			(unsigned long long) fsgeo.rtextents);
> +		printf(_("geom.rtextsize = %u\n"), fsgeo.rtextsize);
> +		printf(_("geom.sunit = %u\n"), fsgeo.sunit);
> +		printf(_("geom.swidth = %u\n"), fsgeo.swidth);
> +	}
> +	if ((xfsctl(file->name, file->fd, XFS_IOC_FSCOUNTS, &fscounts)) < 0) {
> +		perror("XFS_IOC_FSCOUNTS");
> +	} else {
> +		printf(_("counts.freedata = %llu\n"),
> +			(unsigned long long) fscounts.freedata);
> +		printf(_("counts.freertx = %llu\n"),
> +			(unsigned long long) fscounts.freertx);
> +		printf(_("counts.freeino = %llu\n"),
> +			(unsigned long long) fscounts.freeino);
> +		printf(_("counts.allocino = %llu\n"),
> +			(unsigned long long) fscounts.allocino);
> +	}
> +	return 0;
> +}
> +
> +void
> +stat_init(void)
> +{
> +	stat_cmd.name = "stat";
> +	stat_cmd.cfunc = stat_f;
> +	stat_cmd.argmin = 0;
> +	stat_cmd.argmax = 1;
> +	stat_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> +	stat_cmd.args = _("[-v]");
> +	stat_cmd.oneline = _("statistics on the currently open file");
> +
> +	statfs_cmd.name = "statfs";
> +	statfs_cmd.cfunc = statfs_f;
> +	statfs_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> +	statfs_cmd.oneline =
> +		_("statistics on the filesystem of the currently open file");
> +
> +	add_command(&stat_cmd);
> +	add_command(&statfs_cmd);
> +}
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Eric Sandeen]

Wire up the statx syscall to xfs_io.

xfs_io> help statx
statx [-O | -m mask][-FDLA] -- extended information about the currently open file

 Display extended file status.

 Options:
 -m mask -- Specify the field mask for the statx call (default STATX_ALL)
 -A -- Suppress terminal automount traversal
 -D -- Don't sync attributes with the server
 -F -- Force the attributes to be sync'd with the server
 -L -- Follow symlinks (statx link target)
 -O -- Add only basic stats (STATX_BASIC_STATS) to default mask

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: Fix dumb typos, add usage for bad options, change the -O meaning,
remove pretty print code, move all compat #defines to statx.h

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();
 	sync_range_init();
 	truncate_init();
diff --git a/io/io.h b/io/io.h
index 7e95bd5..952bdb8 100644
--- a/io/io.h
+++ b/io/io.h
@@ -112,6 +112,7 @@ extern void		pwrite_init(void);
 extern void		quit_init(void);
 extern void		seek_init(void);
 extern void		shutdown_init(void);
+extern void		stat_init(void);
 extern void		sync_init(void);
 extern void		truncate_init(void);
 extern void		utimes_init(void);
diff --git a/io/stat.c b/io/stat.c
index 3ae9903..a7aebcd 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -2,6 +2,9 @@
  * Copyright (c) 2003-2005 Silicon Graphics, Inc.
  * All Rights Reserved.
  *
+ * Copyright (C) 2015, 2017 Red Hat, Inc.
+ * Portions of statx support written by David Howells (dhowells@redhat.com)
+ *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
  * published by the Free Software Foundation.
@@ -20,10 +23,14 @@
 #include "input.h"
 #include "init.h"
 #include "io.h"
+#include "statx.h"
 #include "libxfs.h"
 
+#include <fcntl.h>
+
 static cmdinfo_t stat_cmd;
 static cmdinfo_t statfs_cmd;
+static cmdinfo_t statx_cmd;
 
 off64_t
 filesize(void)
@@ -167,6 +174,125 @@ statfs_f(
 	return 0;
 }
 
+static ssize_t
+statx(int dfd, const char *filename, unsigned flags,
+      unsigned int mask, struct statx *buffer)
+{
+	return syscall(__NR_statx, dfd, filename, flags, mask, buffer);
+}
+
+static void
+statx_help(void)
+{
+        printf(_(
+"\n"
+" Display extended file status.\n"
+"\n"
+" Options:\n"
+" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
+" -A -- Suppress terminal automount traversal\n"
+" -D -- Don't sync attributes with the server\n"
+" -F -- Force the attributes to be sync'd with the server\n"
+" -L -- Follow symlinks (statx link target)\n"
+" -O -- Add only basic stats (STATX_BASIC_STATS) to default mask\n"
+"\n"));
+}
+
+/* statx helper */
+static void
+dump_statx(struct statx *stx)
+{
+	printf("stx_mask: 0x%x\n", stx->stx_mask);
+	printf("stx_blksize: %u\n", stx->stx_blksize);
+	printf("stx_attributes: 0x%llx\n", stx->stx_attributes);
+	printf("stx_nlink: %u\n", stx->stx_nlink);
+	printf("stx_uid: %u\n", stx->stx_uid);
+	printf("stx_gid: %u\n", stx->stx_gid);
+	printf("stx_mode: 0%o\n", stx->stx_mode);
+	printf("stx_ino: %llu\n", stx->stx_ino);
+	printf("stx_size: %llu\n", stx->stx_size);
+	printf("stx_blocks: %llu\n", stx->stx_blocks);
+	printf("stx_atime.tv_sec: %lld\n", stx->stx_atime.tv_sec);
+	printf("stx_atime.tv_nsec: %d\n", stx->stx_atime.tv_nsec);
+	printf("stx_btime.tv_sec: %lld\n", stx->stx_btime.tv_sec);
+	printf("stx_btime.tv_nsec: %d\n", stx->stx_btime.tv_nsec);
+	printf("stx_ctime.tv_sec: %lld\n", stx->stx_ctime.tv_sec);
+	printf("stx_ctime.tv_nsec: %d\n", stx->stx_ctime.tv_nsec);
+	printf("stx_mtime.tv_sec: %lld\n", stx->stx_mtime.tv_sec);
+	printf("stx_mtime.tv_nsec: %d\n", stx->stx_mtime.tv_nsec);
+	printf("stx_rdev_major: %u\n", stx->stx_rdev_major);
+	printf("stx_rdev_minor: %u\n", stx->stx_rdev_minor);
+	printf("stx_dev_major: %u\n", stx->stx_dev_major);
+	printf("stx_dev_minor: %u\n", stx->stx_dev_minor);
+}
+
+/*
+ * options:
+ * 	- input flags - query type
+ * 	- output style for flags (and all else?) (chars vs. hex?)
+ * 	- output - mask out incidental flag or not?
+ */
+int
+statx_f(
+	int		argc,
+	char		**argv)
+{
+	int		c;
+	struct statx	stx;
+	int		atflag = AT_SYMLINK_NOFOLLOW;
+	unsigned int	m_mask = 0;	/* mask requested with -m */
+	int		Oflag = 0, mflag = 0;	/* -O or -m was used */
+	unsigned int	mask = STATX_ALL;
+
+	while ((c = getopt(argc, argv, "m:FDLOA")) != EOF) {
+		switch (c) {
+		case 'm':
+			m_mask = atoi(optarg);
+			mflag = 1;
+			break;
+		case 'F':
+			atflag &= ~AT_STATX_SYNC_TYPE;
+			atflag |= AT_STATX_FORCE_SYNC;
+			break;
+		case 'D':
+			atflag &= ~AT_STATX_SYNC_TYPE;
+			atflag |= AT_STATX_DONT_SYNC;
+			break;
+		case 'L':
+			atflag &= ~AT_SYMLINK_NOFOLLOW;
+			break;
+		case 'O':
+			mask = STATX_BASIC_STATS;
+			Oflag = 1;
+			break;
+		case 'A':
+			atflag |= AT_NO_AUTOMOUNT;
+			break;
+		default:
+			return command_usage(&statx_cmd);
+		}
+	}
+
+	if (Oflag && mflag) {
+		printf("Cannot specify both -m mask and -O\n");
+		return 0;
+	}
+
+	/* -m overrides any other mask options */
+	if (mflag)
+		mask = m_mask;
+
+	memset(&stx, 0xbf, sizeof(stx));
+	if (statx(AT_FDCWD, file->name, atflag, mask, &stx) < 0) {
+		perror("statx");
+		return 0;
+	}
+
+	dump_statx(&stx);
+
+	return 0;
+}
+
 void
 stat_init(void)
 {
@@ -176,14 +302,25 @@ stat_init(void)
 	stat_cmd.argmax = 1;
 	stat_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
 	stat_cmd.args = _("[-v]");
-	stat_cmd.oneline = _("statistics on the currently open file");
+	stat_cmd.oneline = _("information about the currently open file");
 
 	statfs_cmd.name = "statfs";
 	statfs_cmd.cfunc = statfs_f;
 	statfs_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
 	statfs_cmd.oneline =
-		_("statistics on the filesystem of the currently open file");
+ _("information about the filesystem of the currently open file");
+
+	statx_cmd.name = "statx";
+	statx_cmd.cfunc = statx_f;
+	statx_cmd.argmin = 0;
+	statx_cmd.argmax = -1;
+	statx_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	statx_cmd.args = _("[-O | -m mask][-FDLAP]");
+	statx_cmd.oneline =
+ _("extended information about the currently open file");
+	statx_cmd.help = statx_help;
 
 	add_command(&stat_cmd);
 	add_command(&statfs_cmd);
+	add_command(&statx_cmd);
 }
diff --git a/io/statx.h b/io/statx.h
new file mode 100644
index 0000000..729a147
--- /dev/null
+++ b/io/statx.h
@@ -0,0 +1,158 @@
+#ifndef XFS_IO_STATX_H
+#define XFS_IO_STATX_H
+
+#include <unistd.h>
+
+#ifndef AT_STATX_SYNC_TYPE
+#define AT_STATX_SYNC_TYPE      0x6000  /* Type of synchronisation required from statx() */
+#define AT_STATX_SYNC_AS_STAT   0x0000  /* - Do whatever stat() does */
+#define AT_STATX_FORCE_SYNC     0x2000  /* - Force the attributes to be sync'd with the server */
+#define AT_STATX_DONT_SYNC      0x4000  /* - Don't sync attributes with the server */
+#endif
+
+#ifndef AT_NO_AUTOMOUNT
+#define AT_NO_AUTOMOUNT		0x800	/* Suppress terminal automount traversal */
+#endif
+
+#ifdef __i386__
+#define __NR_statx 383
+#elif defined (__ILP32__)
+#define __NR_statx (__X32_SYSCALL_BIT + 332)
+#else
+#define __NR_statx 332
+#endif
+
+#ifndef STATX_TYPE
+
+/*
+ * Timestamp structure for the timestamps in struct statx.
+ *
+ * tv_sec holds the number of seconds before (negative) or after (positive)
+ * 00:00:00 1st January 1970 UTC.
+ *
+ * tv_nsec holds a number of nanoseconds before (0..-999,999,999 if tv_sec is
+ * negative) or after (0..999,999,999 if tv_sec is positive) the tv_sec time.
+ *
+ * Note that if both tv_sec and tv_nsec are non-zero, then the two values must
+ * either be both positive or both negative.
+ *
+ * __reserved is held in case we need a yet finer resolution.
+ */
+struct statx_timestamp {
+	__s64	tv_sec;
+	__s32	tv_nsec;
+	__s32	__reserved;
+};
+
+/*
+ * Structures for the extended file attribute retrieval system call
+ * (statx()).
+ *
+ * The caller passes a mask of what they're specifically interested in as a
+ * parameter to statx().  What statx() actually got will be indicated in
+ * st_mask upon return.
+ *
+ * For each bit in the mask argument:
+ *
+ * - if the datum is not supported:
+ *
+ *   - the bit will be cleared, and
+ *
+ *   - the datum will be set to an appropriate fabricated value if one is
+ *     available (eg. CIFS can take a default uid and gid), otherwise
+ *
+ *   - the field will be cleared;
+ *
+ * - otherwise, if explicitly requested:
+ *
+ *   - the datum will be synchronised to the server if AT_STATX_FORCE_SYNC is
+ *     set or if the datum is considered out of date, and
+ *
+ *   - the field will be filled in and the bit will be set;
+ *
+ * - otherwise, if not requested, but available in approximate form without any
+ *   effort, it will be filled in anyway, and the bit will be set upon return
+ *   (it might not be up to date, however, and no attempt will be made to
+ *   synchronise the internal state first);
+ *
+ * - otherwise the field and the bit will be cleared before returning.
+ *
+ * Items in STATX_BASIC_STATS may be marked unavailable on return, but they
+ * will have values installed for compatibility purposes so that stat() and
+ * co. can be emulated in userspace.
+ */
+struct statx {
+	/* 0x00 */
+	__u32	stx_mask;	/* What results were written [uncond] */
+	__u32	stx_blksize;	/* Preferred general I/O size [uncond] */
+	__u64	stx_attributes;	/* Flags conveying information about the file [uncond] */
+	/* 0x10 */
+	__u32	stx_nlink;	/* Number of hard links */
+	__u32	stx_uid;	/* User ID of owner */
+	__u32	stx_gid;	/* Group ID of owner */
+	__u16	stx_mode;	/* File mode */
+	__u16	__spare0[1];
+	/* 0x20 */
+	__u64	stx_ino;	/* Inode number */
+	__u64	stx_size;	/* File size */
+	__u64	stx_blocks;	/* Number of 512-byte blocks allocated */
+	__u64	__spare1[1];
+	/* 0x40 */
+	struct statx_timestamp	stx_atime;	/* Last access time */
+	struct statx_timestamp	stx_btime;	/* File creation time */
+	struct statx_timestamp	stx_ctime;	/* Last attribute change time */
+	struct statx_timestamp	stx_mtime;	/* Last data modification time */
+	/* 0x80 */
+	__u32	stx_rdev_major;	/* Device ID of special file [if bdev/cdev] */
+	__u32	stx_rdev_minor;
+	__u32	stx_dev_major;	/* ID of device containing file [uncond] */
+	__u32	stx_dev_minor;
+	/* 0x90 */
+	__u64	__spare2[14];	/* Spare space for future expansion */
+	/* 0x100 */
+};
+
+/*
+ * Flags to be stx_mask
+ *
+ * Query request/result mask for statx() and struct statx::stx_mask.
+ *
+ * These bits should be set in the mask argument of statx() to request
+ * particular items when calling statx().
+ */
+#define STATX_TYPE		0x00000001U	/* Want/got stx_mode & S_IFMT */
+#define STATX_MODE		0x00000002U	/* Want/got stx_mode & ~S_IFMT */
+#define STATX_NLINK		0x00000004U	/* Want/got stx_nlink */
+#define STATX_UID		0x00000008U	/* Want/got stx_uid */
+#define STATX_GID		0x00000010U	/* Want/got stx_gid */
+#define STATX_ATIME		0x00000020U	/* Want/got stx_atime */
+#define STATX_MTIME		0x00000040U	/* Want/got stx_mtime */
+#define STATX_CTIME		0x00000080U	/* Want/got stx_ctime */
+#define STATX_INO		0x00000100U	/* Want/got stx_ino */
+#define STATX_SIZE		0x00000200U	/* Want/got stx_size */
+#define STATX_BLOCKS		0x00000400U	/* Want/got stx_blocks */
+#define STATX_BASIC_STATS	0x000007ffU	/* The stuff in the normal stat struct */
+#define STATX_BTIME		0x00000800U	/* Want/got stx_btime */
+#define STATX_ALL		0x00000fffU	/* All currently supported flags */
+
+/*
+ * Attributes to be found in stx_attributes
+ *
+ * These give information about the features or the state of a file that might
+ * be of use to ordinary userspace programs such as GUIs or ls rather than
+ * specialised tools.
+ *
+ * Note that the flags marked [I] correspond to generic FS_IOC_FLAGS
+ * semantically.  Where possible, the numerical value is picked to correspond
+ * also.
+ */
+#define STATX_ATTR_COMPRESSED		0x00000004 /* [I] File is compressed by the fs */
+#define STATX_ATTR_IMMUTABLE		0x00000010 /* [I] File is marked immutable */
+#define STATX_ATTR_APPEND		0x00000020 /* [I] File is append-only */
+#define STATX_ATTR_NODUMP		0x00000040 /* [I] File is not to be dumped */
+#define STATX_ATTR_ENCRYPTED		0x00000800 /* [I] File requires key to decrypt in fs */
+
+#define STATX_ATTR_AUTOMOUNT		0x00001000 /* Dir: Automount trigger */
+
+#endif /* STATX_TYPE */
+#endif /* XFS_IO_STATX_H */
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)

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

[Darrick J. Wong]

On Thu, Mar 23, 2017 at 11:45:45PM -0500, Eric Sandeen wrote:
> Wire up the statx syscall to xfs_io.
> 
> xfs_io> help statx
> statx [-O | -m mask][-FDLA] -- extended information about the currently open file
> 
>  Display extended file status.
> 
>  Options:
>  -m mask -- Specify the field mask for the statx call (default STATX_ALL)
>  -A -- Suppress terminal automount traversal
>  -D -- Don't sync attributes with the server
>  -F -- Force the attributes to be sync'd with the server
>  -L -- Follow symlinks (statx link target)
>  -O -- Add only basic stats (STATX_BASIC_STATS) to default mask
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> V2: Fix dumb typos, add usage for bad options, change the -O meaning,
> remove pretty print code, move all compat #defines to statx.h
> 
> 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();
>  	sync_range_init();
>  	truncate_init();
> diff --git a/io/io.h b/io/io.h
> index 7e95bd5..952bdb8 100644
> --- a/io/io.h
> +++ b/io/io.h
> @@ -112,6 +112,7 @@ extern void		pwrite_init(void);
>  extern void		quit_init(void);
>  extern void		seek_init(void);
>  extern void		shutdown_init(void);
> +extern void		stat_init(void);
>  extern void		sync_init(void);
>  extern void		truncate_init(void);
>  extern void		utimes_init(void);
> diff --git a/io/stat.c b/io/stat.c
> index 3ae9903..a7aebcd 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -2,6 +2,9 @@
>   * Copyright (c) 2003-2005 Silicon Graphics, Inc.
>   * All Rights Reserved.
>   *
> + * Copyright (C) 2015, 2017 Red Hat, Inc.
> + * Portions of statx support written by David Howells (dhowells@redhat.com)
> + *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License as
>   * published by the Free Software Foundation.
> @@ -20,10 +23,14 @@
>  #include "input.h"
>  #include "init.h"
>  #include "io.h"
> +#include "statx.h"
>  #include "libxfs.h"
>  
> +#include <fcntl.h>
> +
>  static cmdinfo_t stat_cmd;
>  static cmdinfo_t statfs_cmd;
> +static cmdinfo_t statx_cmd;
>  
>  off64_t
>  filesize(void)
> @@ -167,6 +174,125 @@ statfs_f(
>  	return 0;
>  }
>  
> +static ssize_t
> +statx(int dfd, const char *filename, unsigned flags,
> +      unsigned int mask, struct statx *buffer)
> +{
> +	return syscall(__NR_statx, dfd, filename, flags, mask, buffer);
> +}
> +
> +static void
> +statx_help(void)
> +{
> +        printf(_(
> +"\n"
> +" Display extended file status.\n"
> +"\n"
> +" Options:\n"
> +" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
> +" -A -- Suppress terminal automount traversal\n"
> +" -D -- Don't sync attributes with the server\n"
> +" -F -- Force the attributes to be sync'd with the server\n"
> +" -L -- Follow symlinks (statx link target)\n"
> +" -O -- Add only basic stats (STATX_BASIC_STATS) to default mask\n"
> +"\n"));
> +}
> +
> +/* statx helper */
> +static void
> +dump_statx(struct statx *stx)
> +{
> +	printf("stx_mask: 0x%x\n", stx->stx_mask);
> +	printf("stx_blksize: %u\n", stx->stx_blksize);
> +	printf("stx_attributes: 0x%llx\n", stx->stx_attributes);
> +	printf("stx_nlink: %u\n", stx->stx_nlink);
> +	printf("stx_uid: %u\n", stx->stx_uid);
> +	printf("stx_gid: %u\n", stx->stx_gid);
> +	printf("stx_mode: 0%o\n", stx->stx_mode);
> +	printf("stx_ino: %llu\n", stx->stx_ino);
> +	printf("stx_size: %llu\n", stx->stx_size);
> +	printf("stx_blocks: %llu\n", stx->stx_blocks);
> +	printf("stx_atime.tv_sec: %lld\n", stx->stx_atime.tv_sec);
> +	printf("stx_atime.tv_nsec: %d\n", stx->stx_atime.tv_nsec);
> +	printf("stx_btime.tv_sec: %lld\n", stx->stx_btime.tv_sec);
> +	printf("stx_btime.tv_nsec: %d\n", stx->stx_btime.tv_nsec);
> +	printf("stx_ctime.tv_sec: %lld\n", stx->stx_ctime.tv_sec);
> +	printf("stx_ctime.tv_nsec: %d\n", stx->stx_ctime.tv_nsec);
> +	printf("stx_mtime.tv_sec: %lld\n", stx->stx_mtime.tv_sec);
> +	printf("stx_mtime.tv_nsec: %d\n", stx->stx_mtime.tv_nsec);
> +	printf("stx_rdev_major: %u\n", stx->stx_rdev_major);
> +	printf("stx_rdev_minor: %u\n", stx->stx_rdev_minor);
> +	printf("stx_dev_major: %u\n", stx->stx_dev_major);
> +	printf("stx_dev_minor: %u\n", stx->stx_dev_minor);
> +}
> +
> +/*
> + * options:
> + * 	- input flags - query type
> + * 	- output style for flags (and all else?) (chars vs. hex?)
> + * 	- output - mask out incidental flag or not?
> + */
> +int
> +statx_f(
> +	int		argc,
> +	char		**argv)
> +{
> +	int		c;
> +	struct statx	stx;
> +	int		atflag = AT_SYMLINK_NOFOLLOW;
> +	unsigned int	m_mask = 0;	/* mask requested with -m */
> +	int		Oflag = 0, mflag = 0;	/* -O or -m was used */
> +	unsigned int	mask = STATX_ALL;
> +
> +	while ((c = getopt(argc, argv, "m:FDLOA")) != EOF) {
> +		switch (c) {
> +		case 'm':
> +			m_mask = atoi(optarg);
> +			mflag = 1;
> +			break;
> +		case 'F':
> +			atflag &= ~AT_STATX_SYNC_TYPE;
> +			atflag |= AT_STATX_FORCE_SYNC;
> +			break;
> +		case 'D':
> +			atflag &= ~AT_STATX_SYNC_TYPE;
> +			atflag |= AT_STATX_DONT_SYNC;
> +			break;
> +		case 'L':
> +			atflag &= ~AT_SYMLINK_NOFOLLOW;
> +			break;
> +		case 'O':
> +			mask = STATX_BASIC_STATS;
> +			Oflag = 1;
> +			break;
> +		case 'A':
> +			atflag |= AT_NO_AUTOMOUNT;
> +			break;
> +		default:
> +			return command_usage(&statx_cmd);
> +		}
> +	}
> +
> +	if (Oflag && mflag) {
> +		printf("Cannot specify both -m mask and -O\n");
> +		return 0;
> +	}
> +
> +	/* -m overrides any other mask options */
> +	if (mflag)
> +		mask = m_mask;
> +
> +	memset(&stx, 0xbf, sizeof(stx));
> +	if (statx(AT_FDCWD, file->name, atflag, mask, &stx) < 0) {
> +		perror("statx");
> +		return 0;
> +	}
> +
> +	dump_statx(&stx);
> +
> +	return 0;
> +}
> +
>  void
>  stat_init(void)
>  {
> @@ -176,14 +302,25 @@ stat_init(void)
>  	stat_cmd.argmax = 1;
>  	stat_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
>  	stat_cmd.args = _("[-v]");
> -	stat_cmd.oneline = _("statistics on the currently open file");
> +	stat_cmd.oneline = _("information about the currently open file");
>  
>  	statfs_cmd.name = "statfs";
>  	statfs_cmd.cfunc = statfs_f;
>  	statfs_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
>  	statfs_cmd.oneline =
> -		_("statistics on the filesystem of the currently open file");
> + _("information about the filesystem of the currently open file");
> +
> +	statx_cmd.name = "statx";
> +	statx_cmd.cfunc = statx_f;
> +	statx_cmd.argmin = 0;
> +	statx_cmd.argmax = -1;
> +	statx_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> +	statx_cmd.args = _("[-O | -m mask][-FDLAP]");
> +	statx_cmd.oneline =
> + _("extended information about the currently open file");
> +	statx_cmd.help = statx_help;
>  
>  	add_command(&stat_cmd);
>  	add_command(&statfs_cmd);
> +	add_command(&statx_cmd);
>  }
> diff --git a/io/statx.h b/io/statx.h
> new file mode 100644
> index 0000000..729a147
> --- /dev/null
> +++ b/io/statx.h
> @@ -0,0 +1,158 @@
> +#ifndef XFS_IO_STATX_H
> +#define XFS_IO_STATX_H
> +
> +#include <unistd.h>
> +
> +#ifndef AT_STATX_SYNC_TYPE
> +#define AT_STATX_SYNC_TYPE      0x6000  /* Type of synchronisation required from statx() */
> +#define AT_STATX_SYNC_AS_STAT   0x0000  /* - Do whatever stat() does */
> +#define AT_STATX_FORCE_SYNC     0x2000  /* - Force the attributes to be sync'd with the server */
> +#define AT_STATX_DONT_SYNC      0x4000  /* - Don't sync attributes with the server */
> +#endif
> +
> +#ifndef AT_NO_AUTOMOUNT
> +#define AT_NO_AUTOMOUNT		0x800	/* Suppress terminal automount traversal */
> +#endif
> +
> +#ifdef __i386__
> +#define __NR_statx 383
> +#elif defined (__ILP32__)
> +#define __NR_statx (__X32_SYSCALL_BIT + 332)
> +#else
> +#define __NR_statx 332
> +#endif
> +
> +#ifndef STATX_TYPE
> +
> +/*
> + * Timestamp structure for the timestamps in struct statx.
> + *
> + * tv_sec holds the number of seconds before (negative) or after (positive)
> + * 00:00:00 1st January 1970 UTC.
> + *
> + * tv_nsec holds a number of nanoseconds before (0..-999,999,999 if tv_sec is
> + * negative) or after (0..999,999,999 if tv_sec is positive) the tv_sec time.
> + *
> + * Note that if both tv_sec and tv_nsec are non-zero, then the two values must
> + * either be both positive or both negative.
> + *
> + * __reserved is held in case we need a yet finer resolution.
> + */
> +struct statx_timestamp {
> +	__s64	tv_sec;
> +	__s32	tv_nsec;
> +	__s32	__reserved;
> +};
> +
> +/*
> + * Structures for the extended file attribute retrieval system call
> + * (statx()).
> + *
> + * The caller passes a mask of what they're specifically interested in as a
> + * parameter to statx().  What statx() actually got will be indicated in
> + * st_mask upon return.
> + *
> + * For each bit in the mask argument:
> + *
> + * - if the datum is not supported:
> + *
> + *   - the bit will be cleared, and
> + *
> + *   - the datum will be set to an appropriate fabricated value if one is
> + *     available (eg. CIFS can take a default uid and gid), otherwise
> + *
> + *   - the field will be cleared;
> + *
> + * - otherwise, if explicitly requested:
> + *
> + *   - the datum will be synchronised to the server if AT_STATX_FORCE_SYNC is
> + *     set or if the datum is considered out of date, and
> + *
> + *   - the field will be filled in and the bit will be set;
> + *
> + * - otherwise, if not requested, but available in approximate form without any
> + *   effort, it will be filled in anyway, and the bit will be set upon return
> + *   (it might not be up to date, however, and no attempt will be made to
> + *   synchronise the internal state first);
> + *
> + * - otherwise the field and the bit will be cleared before returning.
> + *
> + * Items in STATX_BASIC_STATS may be marked unavailable on return, but they
> + * will have values installed for compatibility purposes so that stat() and
> + * co. can be emulated in userspace.
> + */
> +struct statx {
> +	/* 0x00 */
> +	__u32	stx_mask;	/* What results were written [uncond] */
> +	__u32	stx_blksize;	/* Preferred general I/O size [uncond] */
> +	__u64	stx_attributes;	/* Flags conveying information about the file [uncond] */
> +	/* 0x10 */
> +	__u32	stx_nlink;	/* Number of hard links */
> +	__u32	stx_uid;	/* User ID of owner */
> +	__u32	stx_gid;	/* Group ID of owner */
> +	__u16	stx_mode;	/* File mode */
> +	__u16	__spare0[1];
> +	/* 0x20 */
> +	__u64	stx_ino;	/* Inode number */
> +	__u64	stx_size;	/* File size */
> +	__u64	stx_blocks;	/* Number of 512-byte blocks allocated */
> +	__u64	__spare1[1];
> +	/* 0x40 */
> +	struct statx_timestamp	stx_atime;	/* Last access time */
> +	struct statx_timestamp	stx_btime;	/* File creation time */
> +	struct statx_timestamp	stx_ctime;	/* Last attribute change time */
> +	struct statx_timestamp	stx_mtime;	/* Last data modification time */
> +	/* 0x80 */
> +	__u32	stx_rdev_major;	/* Device ID of special file [if bdev/cdev] */
> +	__u32	stx_rdev_minor;
> +	__u32	stx_dev_major;	/* ID of device containing file [uncond] */
> +	__u32	stx_dev_minor;
> +	/* 0x90 */
> +	__u64	__spare2[14];	/* Spare space for future expansion */
> +	/* 0x100 */
> +};
> +
> +/*
> + * Flags to be stx_mask
> + *
> + * Query request/result mask for statx() and struct statx::stx_mask.
> + *
> + * These bits should be set in the mask argument of statx() to request
> + * particular items when calling statx().
> + */
> +#define STATX_TYPE		0x00000001U	/* Want/got stx_mode & S_IFMT */
> +#define STATX_MODE		0x00000002U	/* Want/got stx_mode & ~S_IFMT */
> +#define STATX_NLINK		0x00000004U	/* Want/got stx_nlink */
> +#define STATX_UID		0x00000008U	/* Want/got stx_uid */
> +#define STATX_GID		0x00000010U	/* Want/got stx_gid */
> +#define STATX_ATIME		0x00000020U	/* Want/got stx_atime */
> +#define STATX_MTIME		0x00000040U	/* Want/got stx_mtime */
> +#define STATX_CTIME		0x00000080U	/* Want/got stx_ctime */
> +#define STATX_INO		0x00000100U	/* Want/got stx_ino */
> +#define STATX_SIZE		0x00000200U	/* Want/got stx_size */
> +#define STATX_BLOCKS		0x00000400U	/* Want/got stx_blocks */
> +#define STATX_BASIC_STATS	0x000007ffU	/* The stuff in the normal stat struct */
> +#define STATX_BTIME		0x00000800U	/* Want/got stx_btime */
> +#define STATX_ALL		0x00000fffU	/* All currently supported flags */
> +
> +/*
> + * Attributes to be found in stx_attributes
> + *
> + * These give information about the features or the state of a file that might
> + * be of use to ordinary userspace programs such as GUIs or ls rather than
> + * specialised tools.
> + *
> + * Note that the flags marked [I] correspond to generic FS_IOC_FLAGS
> + * semantically.  Where possible, the numerical value is picked to correspond
> + * also.
> + */
> +#define STATX_ATTR_COMPRESSED		0x00000004 /* [I] File is compressed by the fs */
> +#define STATX_ATTR_IMMUTABLE		0x00000010 /* [I] File is marked immutable */
> +#define STATX_ATTR_APPEND		0x00000020 /* [I] File is append-only */
> +#define STATX_ATTR_NODUMP		0x00000040 /* [I] File is not to be dumped */
> +#define STATX_ATTR_ENCRYPTED		0x00000800 /* [I] File requires key to decrypt in fs */
> +
> +#define STATX_ATTR_AUTOMOUNT		0x00001000 /* Dir: Automount trigger */
> +
> +#endif /* STATX_TYPE */
> +#endif /* XFS_IO_STATX_H */
> 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)
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Eric Biggers]

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

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

[Eric Sandeen]

On 3/27/17 3:58 PM, Eric Biggers wrote:
> 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?

Yeah, good point.  Not sure why I did what I did ;)

> 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.)

*nod*

> 
>> 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.

Yup!

>> 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:

Ugh, thanks.  I'm not good at man pages.

Will fix all these, thanks!

-Eric

> .TP
> .B \-A
> Suppress terminal automount traversal
> .RE
> .TP
> .B statfs
> 
> 
> - Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[David Howells]

Eric Sandeen <sandeen@sandeen.net> wrote:

> These 2 patches are a second pass to add a statx command
> to xfs_io in hopes that it will aid creation of xfstests
> statx regression tests.

Would it be possible to redirect all the stat() calls made by xfstests to
statx() with translation of the output buffer back to struct stat?

David

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

[Eric Sandeen]

On 3/27/17 5:00 AM, David Howells wrote:
> Eric Sandeen <sandeen@sandeen.net> wrote:
> 
>> These 2 patches are a second pass to add a statx command
>> to xfs_io in hopes that it will aid creation of xfstests
>> statx regression tests.
> 
> Would it be possible to redirect all the stat() calls made by xfstests to
> statx() with translation of the output buffer back to struct stat?

I'm sure LD_PRELOAD tricks could do that, but it'd be a little unusual
for xfstests, at least - kind of a side hack that's not really expected
and therefore wouldn't get consistently run.

Honestly, just write a bash script for xfstests which tests expected
statx behavior for various inputs, verifies what it can against stat,
and it's all good...

If you have xfstests-specific questions or problems we'll be happy
to help.

Thanks,
-Eric

> David
> 

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

[Eric Sandeen]

On 3/27/17 12:47 PM, Eric Sandeen wrote:
> On 3/27/17 5:00 AM, David Howells wrote:
>> Eric Sandeen <sandeen@sandeen.net> wrote:
>>
>>> These 2 patches are a second pass to add a statx command
>>> to xfs_io in hopes that it will aid creation of xfstests
>>> statx regression tests.
>> Would it be possible to redirect all the stat() calls made by xfstests to
>> statx() with translation of the output buffer back to struct stat?
> I'm sure LD_PRELOAD tricks could do that, but it'd be a little unusual
> for xfstests, at least - kind of a side hack that's not really expected
> and therefore wouldn't get consistently run.

(and thinking about it a bit more, s/stat/statx/ would be the barest
of bare minimum testing - it leaves out verification of all the
interesting behaviors of the new interface...)

-Eric

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

[Dave Chinner]

On Mon, Mar 27, 2017 at 12:47:33PM -0500, Eric Sandeen wrote:
> On 3/27/17 5:00 AM, David Howells wrote:
> > Eric Sandeen <sandeen@sandeen.net> wrote:
> > 
> >> These 2 patches are a second pass to add a statx command
> >> to xfs_io in hopes that it will aid creation of xfstests
> >> statx regression tests.
> > 
> > Would it be possible to redirect all the stat() calls made by xfstests to
> > statx() with translation of the output buffer back to struct stat?
> 
> I'm sure LD_PRELOAD tricks could do that, but it'd be a little unusual
> for xfstests, at least - kind of a side hack that's not really expected
> and therefore wouldn't get consistently run.
> 
> Honestly, just write a bash script for xfstests which tests expected
> statx behavior for various inputs, verifies what it can against stat,
> and it's all good...

Don't forget to add it to fsstress and randomise the flag values it
uses...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

[Amir Goldstein]

On Mon, Mar 27, 2017 at 6:00 AM, David Howells <dhowells@redhat.com> wrote:
> Eric Sandeen <sandeen@sandeen.net> wrote:
>
>> These 2 patches are a second pass to add a statx command
>> to xfs_io in hopes that it will aid creation of xfstests
>> statx regression tests.
>
> Would it be possible to redirect all the stat() calls made by xfstests to
> statx() with translation of the output buffer back to struct stat?
>

David,

xfstests is all 99% bash scripts and most of the uses of stat(2) are
by executing
stat(1), so all you have to do in order to exercise a statx() with existing
tests is install a stat executable in your path that uses statx()
instead of stat().

For that purpose, the xfs_io patch does not really help you because it does
not provide the stat -c output formatting options, which the tests expect.

This may not be enough long term, but it could be a nice validation.

xfs_io -c statx would be useful to write new xfstests that exercise
statx specifically.

It sounds like you are new to xfstests?
What you probably want to run is ./check -g quick.
xfstests is not extensively documented, but it quite simple, so it
usually "just works".
Let us know if you have troubles staring up.

Cheers,
Amir.

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

[David Howells]

Amir Goldstein <amir73il@gmail.com> wrote:

> xfstests is all 99% bash scripts and most of the uses of stat(2) are
> by executing
> stat(1), so all you have to do in order to exercise a statx() with existing
> tests is install a stat executable in your path that uses statx()
> instead of stat().

Actually, it would be easier to write the test in C as:

 (1) How do you feed statx bad arguments from shell scripts (e.g. a kernel
     address for the buffer pointer or the name pointer) to make sure it gives
     appropriate errors?

 (2) I want to do a comparison of the struct statx to a struct stat almost
     every time statx is called.

 (3) There's going to be a bunch of timestamp comparisons.

I grant that (2) and (3) can be done in shell scripts - I've not tried bash's
associative arrays, but they ought to make it easier.  (1), however, is
something that requires C support and is something that I have in the LTP
tests as I copied that stuff from the stat/fstat/lstat tests there.

> It sounds like you are new to xfstests?

I've not tried to modify it before.

> xfstests is not extensively documented, but it quite simple, so it
> usually "just works".

I noticed the paucity of documentation.  doc/ contains nothing more than an
old CHANGES file.  "Quite simple" does not translate to "easy to use",
especially when you have to make changes outside the package to get it to work
- and nowhere in the docs that there are is this mentioned.

David

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

[David Howells]

Eric Sandeen <sandeen@sandeen.net> wrote:

> +			m_mask = atoi(optarg);

strtoul() would probably be more useful as you can then specify a hex number.

David

Re: [PATCH 1/2 V2] xfs_io: move stat functions to new file

[David Howells]

Eric Sandeen <sandeen@sandeen.net> wrote:

> diff --git a/io/Makefile b/io/Makefile

What do I apply this to?  I don't see any directory or file called "io" in
xfstests-dev/master.

David

Re: [PATCH 1/2 V2] xfs_io: move stat functions to new file

[Darrick J. Wong]

On Mon, Mar 27, 2017 at 09:26:07PM +0100, David Howells wrote:
> Eric Sandeen <sandeen@sandeen.net> wrote:
> 
> > diff --git a/io/Makefile b/io/Makefile
> 
> What do I apply this to?  I don't see any directory or file called "io" in
> xfstests-dev/master.

This is a patch series against xfsprogs to add a statx command to
xfs_io(8).  Eric wired up statx into a command line tool so that you can
write more easily the xfstests cases to exercise the new syscall.

--D

> 
> David
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] xfs_io: changes to statx interface

[David Howells]

Eric Sandeen <sandeen@sandeen.net> wrote:

> Wire up the statx syscall to xfs_io.
> 
> xfs_io> help statx
> statx [-O | -m mask][-FDLA] -- extended information about the currently open file
> ...

I would like to make the attached changes, to make it more capable, except
that xfs_io seems to precheck the "-m mask" argument somewhere:

	[root@andromeda ~]# xfs_io -c statx -m all /dev/null
	non-numeric mode -- all

and interprets "-c" for itself on the command line:

	[root@andromeda ~]# xfs_io -c statx -c /dev/null
	command "/dev/null" not found

though both these seem to work when used from xfs_io's command prompt.  I
guess I should switch -c to -C and also -m to -M since it's not a file mode as
I think the core is expecting.

Also, how do you get xfs_io to tell you what fd it has opened from its own
command prompt?  I would need to pass that to the -d flag as a dir fd.

David
---
commit 03e4ca1cdc59aaa362fdd51f079493a1f0da254c
Author: David Howells <dhowells@redhat.com>
Date:   Tue Mar 28 10:42:23 2017 +0100

    changes

diff --git a/io/stat.c b/io/stat.c
index a7aebcd..0b05ec9 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -189,12 +189,14 @@ statx_help(void)
 " Display extended file status.\n"
 "\n"
 " Options:\n"
-" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
+" -c -- Compare against fstat/fstatat on the same file/fd\n"
+" -d dirfd -- Use a specific directory fd\n"
+" -f -- Do fstat equivalent and operate on fd\n"
+" -m mask -- Specify the field mask for the statx call (can also be 'basic' or 'all'; default STATX_ALL)\n"
 " -A -- Suppress terminal automount traversal\n"
 " -D -- Don't sync attributes with the server\n"
 " -F -- Force the attributes to be sync'd with the server\n"
 " -L -- Follow symlinks (statx link target)\n"
-" -O -- Add only basic stats (STATX_BASIC_STATS) to default mask\n"
 "\n"));
 }
 
@@ -227,6 +229,65 @@ dump_statx(struct statx *stx)
 }
 
 /*
+ * Compare the contents of a statx struct with that of a stat struct and check
+ * that they're the same.
+ */
+static int
+cmp_statx(const struct statx *stx, const struct stat *st)
+{
+	const char *what = NULL;
+
+#define cmp(x) \
+	do {					\
+		what = #x;		\
+		if (stx->stx_##x != st->st_##x)	\
+			goto mismatch;		\
+	} while (0)
+
+	cmp(blksize);
+	cmp(nlink);
+	cmp(uid);
+	cmp(gid);
+	cmp(mode);
+	cmp(ino);
+	cmp(size);
+	cmp(blocks);
+
+#define devcmp(x) \
+	do {						\
+		what = #x".major";			\
+		if (stx->stx_##x##_major != major(st->st_##x))	\
+			goto mismatch;			\
+		what = #x".minor";			\
+		if (stx->stx_##x##_minor != minor(st->st_##x))	\
+			goto mismatch;			\
+	} while (0)
+
+	devcmp(dev);
+	devcmp(rdev);
+
+#define timecmp(x) \
+	do {						\
+		what = #x".tv_sec";			\
+		if (stx->stx_##x##time.tv_sec != st->st_##x##tim.tv_sec)	\
+			goto mismatch;			\
+		what = #x".tv_nsec";			\
+		if (stx->stx_##x##time.tv_nsec != st->st_##x##tim.tv_nsec)	\
+			goto mismatch;			\
+	} while (0)
+
+	timecmp(a);
+	timecmp(c);
+	timecmp(m);
+
+	return 0;
+
+mismatch:
+	fprintf(stderr, "Mismatch between stat and statx output (%s)\n", what);
+	return -1;
+}
+
+/*
  * options:
  * 	- input flags - query type
  * 	- output style for flags (and all else?) (chars vs. hex?)
@@ -239,16 +300,31 @@ statx_f(
 {
 	int		c;
 	struct statx	stx;
+	struct stat	st;
 	int		atflag = AT_SYMLINK_NOFOLLOW;
-	unsigned int	m_mask = 0;	/* mask requested with -m */
-	int		Oflag = 0, mflag = 0;	/* -O or -m was used */
 	unsigned int	mask = STATX_ALL;
+	int		use_fd = 0;
+	int		dirfd = AT_FDCWD;
+	int		compare = 0;
 
-	while ((c = getopt(argc, argv, "m:FDLOA")) != EOF) {
+	while ((c = getopt(argc, argv, "d:cfm:FDLA")) != EOF) {
 		switch (c) {
+		case 'c':
+			compare = 1;
+			break;
+		case 'd':
+			dirfd = strtoul(optarg, NULL, 0);
+			break;
+		case 'f':
+			use_fd = 1;
+			break;
 		case 'm':
-			m_mask = atoi(optarg);
-			mflag = 1;
+			if (strcmp(optarg, "basic") == 0)
+				mask = STATX_BASIC_STATS;
+			else if (strcmp(optarg, "all") == 0)
+				mask = STATX_ALL;
+			else
+				mask = strtoul(optarg, NULL, 0);
 			break;
 		case 'F':
 			atflag &= ~AT_STATX_SYNC_TYPE;
@@ -261,10 +337,6 @@ statx_f(
 		case 'L':
 			atflag &= ~AT_SYMLINK_NOFOLLOW;
 			break;
-		case 'O':
-			mask = STATX_BASIC_STATS;
-			Oflag = 1;
-			break;
 		case 'A':
 			atflag |= AT_NO_AUTOMOUNT;
 			break;
@@ -273,23 +345,38 @@ statx_f(
 		}
 	}
 
-	if (Oflag && mflag) {
-		printf("Cannot specify both -m mask and -O\n");
-		return 0;
+	memset(&stx, 0xbf, sizeof(stx));
+
+	if (use_fd) {
+		if (statx(file->fd, NULL, atflag, mask, &stx) < 0) {
+			perror("statx");
+			return 0;
+		}
+	} else {
+		if (statx(dirfd, file->name, atflag, mask, &stx) < 0) {
+			perror("statx");
+			return 0;
+		}
 	}
 
-	/* -m overrides any other mask options */
-	if (mflag)
-		mask = m_mask;
+	if (compare) {
+		if (use_fd) {
+			if (fstat(file->fd, &st) < 0) {
+				perror("fstat");
+				return 0;
+			}
+		} else {
+			if (fstatat(dirfd, file->name, &st,
+				    atflag & ~AT_STATX_SYNC_TYPE) < 0) {
+				perror("fstatat");
+				return 0;
+			}
+		}
 
-	memset(&stx, 0xbf, sizeof(stx));
-	if (statx(AT_FDCWD, file->name, atflag, mask, &stx) < 0) {
-		perror("statx");
-		return 0;
+		cmp_statx(&stx, &st);
 	}
-
+	
 	dump_statx(&stx);
-
 	return 0;
 }
 

Re: [PATCH] xfs_io: changes to statx interface

[Amir Goldstein]

On Tue, Mar 28, 2017 at 6:22 AM, David Howells <dhowells@redhat.com> wrote:
> Eric Sandeen <sandeen@sandeen.net> wrote:
>
>> Wire up the statx syscall to xfs_io.
>>
>> xfs_io> help statx
>> statx [-O | -m mask][-FDLA] -- extended information about the currently open file
>> ...
>
> I would like to make the attached changes, to make it more capable, except
> that xfs_io seems to precheck the "-m mask" argument somewhere:
>
>         [root@andromeda ~]# xfs_io -c statx -m all /dev/null
>         non-numeric mode -- all
>
> and interprets "-c" for itself on the command line:
>
>         [root@andromeda ~]# xfs_io -c statx -c /dev/null
>         command "/dev/null" not found

xfs_io -c "statx -c /dev/null"

is what you want

>
> though both these seem to work when used from xfs_io's command prompt.  I
> guess I should switch -c to -C and also -m to -M since it's not a file mode as
> I think the core is expecting.
>
> Also, how do you get xfs_io to tell you what fd it has opened from its own
> command prompt?  I would need to pass that to the -d flag as a dir fd.
>

See the output of 'file' command or the global var filetable.


> David
> ---
> commit 03e4ca1cdc59aaa362fdd51f079493a1f0da254c
> Author: David Howells <dhowells@redhat.com>
> Date:   Tue Mar 28 10:42:23 2017 +0100
>
>     changes
>
> diff --git a/io/stat.c b/io/stat.c
> index a7aebcd..0b05ec9 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -189,12 +189,14 @@ statx_help(void)
>  " Display extended file status.\n"
>  "\n"
>  " Options:\n"
> -" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
> +" -c -- Compare against fstat/fstatat on the same file/fd\n"
> +" -d dirfd -- Use a specific directory fd\n"
> +" -f -- Do fstat equivalent and operate on fd\n"
> +" -m mask -- Specify the field mask for the statx call (can also be 'basic' or 'all'; default STATX_ALL)\n"
>  " -A -- Suppress terminal automount traversal\n"
>  " -D -- Don't sync attributes with the server\n"
>  " -F -- Force the attributes to be sync'd with the server\n"
>  " -L -- Follow symlinks (statx link target)\n"
> -" -O -- Add only basic stats (STATX_BASIC_STATS) to default mask\n"
>  "\n"));
>  }
>
> @@ -227,6 +229,65 @@ dump_statx(struct statx *stx)
>  }
>
>  /*
> + * Compare the contents of a statx struct with that of a stat struct and check
> + * that they're the same.
> + */
> +static int
> +cmp_statx(const struct statx *stx, const struct stat *st)
> +{
> +       const char *what = NULL;
> +
> +#define cmp(x) \
> +       do {                                    \
> +               what = #x;              \
> +               if (stx->stx_##x != st->st_##x) \
> +                       goto mismatch;          \
> +       } while (0)
> +
> +       cmp(blksize);
> +       cmp(nlink);
> +       cmp(uid);
> +       cmp(gid);
> +       cmp(mode);
> +       cmp(ino);
> +       cmp(size);
> +       cmp(blocks);
> +
> +#define devcmp(x) \
> +       do {                                            \
> +               what = #x".major";                      \
> +               if (stx->stx_##x##_major != major(st->st_##x))  \
> +                       goto mismatch;                  \
> +               what = #x".minor";                      \
> +               if (stx->stx_##x##_minor != minor(st->st_##x))  \
> +                       goto mismatch;                  \
> +       } while (0)
> +
> +       devcmp(dev);
> +       devcmp(rdev);
> +
> +#define timecmp(x) \
> +       do {                                            \
> +               what = #x".tv_sec";                     \
> +               if (stx->stx_##x##time.tv_sec != st->st_##x##tim.tv_sec)        \
> +                       goto mismatch;                  \
> +               what = #x".tv_nsec";                    \
> +               if (stx->stx_##x##time.tv_nsec != st->st_##x##tim.tv_nsec)      \
> +                       goto mismatch;                  \
> +       } while (0)
> +
> +       timecmp(a);
> +       timecmp(c);
> +       timecmp(m);
> +
> +       return 0;
> +
> +mismatch:
> +       fprintf(stderr, "Mismatch between stat and statx output (%s)\n", what);
> +       return -1;
> +}
> +
> +/*
>   * options:
>   *     - input flags - query type
>   *     - output style for flags (and all else?) (chars vs. hex?)
> @@ -239,16 +300,31 @@ statx_f(
>  {
>         int             c;
>         struct statx    stx;
> +       struct stat     st;
>         int             atflag = AT_SYMLINK_NOFOLLOW;
> -       unsigned int    m_mask = 0;     /* mask requested with -m */
> -       int             Oflag = 0, mflag = 0;   /* -O or -m was used */
>         unsigned int    mask = STATX_ALL;
> +       int             use_fd = 0;
> +       int             dirfd = AT_FDCWD;
> +       int             compare = 0;
>
> -       while ((c = getopt(argc, argv, "m:FDLOA")) != EOF) {
> +       while ((c = getopt(argc, argv, "d:cfm:FDLA")) != EOF) {
>                 switch (c) {
> +               case 'c':
> +                       compare = 1;
> +                       break;
> +               case 'd':
> +                       dirfd = strtoul(optarg, NULL, 0);
> +                       break;
> +               case 'f':
> +                       use_fd = 1;
> +                       break;
>                 case 'm':
> -                       m_mask = atoi(optarg);
> -                       mflag = 1;
> +                       if (strcmp(optarg, "basic") == 0)
> +                               mask = STATX_BASIC_STATS;
> +                       else if (strcmp(optarg, "all") == 0)
> +                               mask = STATX_ALL;
> +                       else
> +                               mask = strtoul(optarg, NULL, 0);
>                         break;
>                 case 'F':
>                         atflag &= ~AT_STATX_SYNC_TYPE;
> @@ -261,10 +337,6 @@ statx_f(
>                 case 'L':
>                         atflag &= ~AT_SYMLINK_NOFOLLOW;
>                         break;
> -               case 'O':
> -                       mask = STATX_BASIC_STATS;
> -                       Oflag = 1;
> -                       break;
>                 case 'A':
>                         atflag |= AT_NO_AUTOMOUNT;
>                         break;
> @@ -273,23 +345,38 @@ statx_f(
>                 }
>         }
>
> -       if (Oflag && mflag) {
> -               printf("Cannot specify both -m mask and -O\n");
> -               return 0;
> +       memset(&stx, 0xbf, sizeof(stx));
> +
> +       if (use_fd) {
> +               if (statx(file->fd, NULL, atflag, mask, &stx) < 0) {
> +                       perror("statx");
> +                       return 0;
> +               }
> +       } else {
> +               if (statx(dirfd, file->name, atflag, mask, &stx) < 0) {
> +                       perror("statx");
> +                       return 0;
> +               }
>         }
>
> -       /* -m overrides any other mask options */
> -       if (mflag)
> -               mask = m_mask;
> +       if (compare) {
> +               if (use_fd) {
> +                       if (fstat(file->fd, &st) < 0) {
> +                               perror("fstat");
> +                               return 0;
> +                       }
> +               } else {
> +                       if (fstatat(dirfd, file->name, &st,
> +                                   atflag & ~AT_STATX_SYNC_TYPE) < 0) {
> +                               perror("fstatat");
> +                               return 0;
> +                       }
> +               }
>
> -       memset(&stx, 0xbf, sizeof(stx));
> -       if (statx(AT_FDCWD, file->name, atflag, mask, &stx) < 0) {
> -               perror("statx");
> -               return 0;
> +               cmp_statx(&stx, &st);
>         }
> -
> +
>         dump_statx(&stx);
> -
>         return 0;
>  }
>

Re: [PATCH] xfs_io: changes to statx interface

[David Howells]

Amir Goldstein <amir73il@gmail.com> wrote:

> xfs_io -c "statx -c /dev/null"
> 
> is what you want

That doesn't produce any output.  Did you mean:

	xfs_io -c "statx -c" /dev/null

Further, this should work:

	warthog>xfs_io -c "statx -c" /dev/sda
	/dev/sda: Permission denied

but doesn't.

> See the output of 'file' command or the global var filetable.

So I would have to make "-d dirfd" take an entry in the global var filetable
rather than an actual fd number?

David

Re: [PATCH] xfs_io: changes to statx interface

[Amir Goldstein]

On Tue, Mar 28, 2017 at 8:31 AM, David Howells <dhowells@redhat.com> wrote:
> Amir Goldstein <amir73il@gmail.com> wrote:
>
>> xfs_io -c "statx -c /dev/null"
>>
>> is what you want
>
> That doesn't produce any output.  Did you mean:
>
>         xfs_io -c "statx -c" /dev/null

Yes.

>
> Further, this should work:
>
>         warthog>xfs_io -c "statx -c" /dev/sda
>         /dev/sda: Permission denied
>

This opens /dev/sda for read/write and feeds the fd to the commands
executed.
You may want to try
xfs_io -r -c "statx -c" /dev/sda

> but doesn't.
>
>> See the output of 'file' command or the global var filetable.
>
> So I would have to make "-d dirfd" take an entry in the global var filetable
> rather than an actual fd number?
>

I guess if you want to test statx without -f/-d then you'd
need to provide filename as one of the args to the statx command
(and not as args to xfs_io command line).

-f is the natural operation you can get from xfs_io
and you can use -d on current fd as well.

The only command I see that uses the non current fd is
sendfile -f N
Mostly commands operate on the "current" fd and current fd
is iterated from filetable.
if you use -d N from command line, you must use -C instead of
-c so statx is not performed on all fds in filetable, e.g:

xfs_io -r -C "statx -c -d 0" /a/ /a/foo

But you see that this makes little sense if "foo" is not an argument
of statx.

In my (hopefully unbiased) opinion, there is room for the syscall sanity
tests that you posted to LTP and there is room for file system
functional tests with xfstests, which xfs_io can be used for.

The main advantage you get from writing xfstests is that many of us run
them on a diversity of file systems, so it is good for catching wrong
implementation of statx by specific file systems.

By the time statx() gets to fs specific code it does not matter if you called it
with -d/-f or like stat() does it?

Amir.

Re: [PATCH] xfs_io: changes to statx interface

[David Howells]

Amir Goldstein <amir73il@gmail.com> wrote:

> > Further, this should work:
> >
> >         warthog>xfs_io -c "statx -c" /dev/sda
> >         /dev/sda: Permission denied
> >
> 
> This opens /dev/sda for read/write and feeds the fd to the commands
> executed.
> You may want to try
> xfs_io -r -c "statx -c" /dev/sda

I've added -P to open to supply O_PATH, but I'm having trouble testing the
dirfd usage - I think probably because file->name is an absolute path:

	xfs_io> open /dev
	Opened 0
	xfs_io> open sda
	sda: No such file or directory
	xfs_io> open /dev/sda
	Opened 1

Possibly open should be able to take a base dir and call openat().

(I also made it print the file table index after a successful open, though I
perhaps only want to do this in interactive mode).

> By the time statx() gets to fs specific code it does not matter if you
> called it with -d/-f or like stat() does it?

No.  But all the calling options still have to be tested, as does stuffing bad
values into the syscall args - something xfstests seems to be very poor at.

> In my (hopefully unbiased) opinion, there is room for the syscall sanity
> tests that you posted to LTP and there is room for file system
> functional tests with xfstests, which xfs_io can be used for.

Yes.  I agree.  Christoph may be of the opinion that LTP is a trainwreck, but
it can do some things much more easily than can xfstests, primarily because
its tests are written in C.

Any suggestions on how to do timestamp comparisons?  I really want to be able
to do things like asking if mtime > btime or btime < wall clock time.  I guess
I could add another command for that.

Re: [PATCH] xfs_io: changes to statx interface

[Amir Goldstein]

On Tue, Mar 28, 2017 at 10:04 AM, David Howells <dhowells@redhat.com> wrote:
> Amir Goldstein <amir73il@gmail.com> wrote:
>
>> > Further, this should work:
>> >
>> >         warthog>xfs_io -c "statx -c" /dev/sda
>> >         /dev/sda: Permission denied
>> >
>>
>> This opens /dev/sda for read/write and feeds the fd to the commands
>> executed.
>> You may want to try
>> xfs_io -r -c "statx -c" /dev/sda
>
> I've added -P to open to supply O_PATH, but I'm having trouble testing the
> dirfd usage - I think probably because file->name is an absolute path:
>
>         xfs_io> open /dev
>         Opened 0
>         xfs_io> open sda
>         sda: No such file or directory
>         xfs_io> open /dev/sda
>         Opened 1
>
> Possibly open should be able to take a base dir and call openat().

Yeh, I told you statx -d does not make much sense for xfs_io.
I don't think you wanna go there...

>
> (I also made it print the file table index after a successful open, though I
> perhaps only want to do this in interactive mode).
>
>> By the time statx() gets to fs specific code it does not matter if you
>> called it with -d/-f or like stat() does it?
>
> No.  But all the calling options still have to be tested, as does stuffing bad
> values into the syscall args - something xfstests seems to be very poor at.
>
>> In my (hopefully unbiased) opinion, there is room for the syscall sanity
>> tests that you posted to LTP and there is room for file system
>> functional tests with xfstests, which xfs_io can be used for.
>
> Yes.  I agree.  Christoph may be of the opinion that LTP is a trainwreck, but
> it can do some things much more easily than can xfstests, primarily because
> its tests are written in C.
>

Adding C tests under src/ dir is a viable option.

> Any suggestions on how to do timestamp comparisons?  I really want to be able
> to do things like asking if mtime > btime or btime < wall clock time.  I guess
> I could add another command for that.

Not in xfs_io, don't go there.

You can look at tests/generic/003 (it hurts the eyes a bit)
Perhaps it would be better to write a small C helper (e.g.
t_compare_statx_times)
and then tests could be easily and flexibly extended.

Amir.

[PATCH] xfs_io: changes to statx interface [ver #2]

[David Howells]

Here are my current changes to Eric's statx interface patch.  I've made it
analoguous to the stat command and so it only does statx-of-fd.

I've made the "-m" flag able to take the words "basic" and "all" in
preference to a number and able to take an octal or hex number as an
alternative too.

I've dropped the -A and -L flags since it no longer passes a path over.

Finally, I've added a -c flag that causes an fstat() to be done as well and
the buffers compared as a consistency check.

David
---
diff --git a/io/stat.c b/io/stat.c
index a7aebcd..961b6d1 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -189,12 +189,10 @@ statx_help(void)
 " Display extended file status.\n"
 "\n"
 " Options:\n"
-" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
-" -A -- Suppress terminal automount traversal\n"
+" -c -- Compare against fstat/fstatat on the same file/fd\n"
+" -m mask -- Specify the field mask for the statx call (can also be 'basic' or 'all'; default STATX_ALL)\n"
 " -D -- Don't sync attributes with the server\n"
 " -F -- Force the attributes to be sync'd with the server\n"
-" -L -- Follow symlinks (statx link target)\n"
-" -O -- Add only basic stats (STATX_BASIC_STATS) to default mask\n"
 "\n"));
 }
 
@@ -227,6 +225,65 @@ dump_statx(struct statx *stx)
 }
 
 /*
+ * Compare the contents of a statx struct with that of a stat struct and check
+ * that they're the same.
+ */
+static int
+cmp_statx(const struct statx *stx, const struct stat *st)
+{
+	const char *what = NULL;
+
+#define cmp(x) \
+	do {					\
+		what = #x;		\
+		if (stx->stx_##x != st->st_##x)	\
+			goto mismatch;		\
+	} while (0)
+
+	cmp(blksize);
+	cmp(nlink);
+	cmp(uid);
+	cmp(gid);
+	cmp(mode);
+	cmp(ino);
+	cmp(size);
+	cmp(blocks);
+
+#define devcmp(x) \
+	do {						\
+		what = #x".major";			\
+		if (stx->stx_##x##_major != major(st->st_##x))	\
+			goto mismatch;			\
+		what = #x".minor";			\
+		if (stx->stx_##x##_minor != minor(st->st_##x))	\
+			goto mismatch;			\
+	} while (0)
+
+	devcmp(dev);
+	devcmp(rdev);
+
+#define timecmp(x) \
+	do {						\
+		what = #x".tv_sec";			\
+		if (stx->stx_##x##time.tv_sec != st->st_##x##tim.tv_sec)	\
+			goto mismatch;			\
+		what = #x".tv_nsec";			\
+		if (stx->stx_##x##time.tv_nsec != st->st_##x##tim.tv_nsec)	\
+			goto mismatch;			\
+	} while (0)
+
+	timecmp(a);
+	timecmp(c);
+	timecmp(m);
+
+	return 0;
+
+mismatch:
+	fprintf(stderr, "Mismatch between stat and statx output (%s)\n", what);
+	return -1;
+}
+
+/*
  * options:
  * 	- input flags - query type
  * 	- output style for flags (and all else?) (chars vs. hex?)
@@ -239,16 +296,23 @@ statx_f(
 {
 	int		c;
 	struct statx	stx;
-	int		atflag = AT_SYMLINK_NOFOLLOW;
-	unsigned int	m_mask = 0;	/* mask requested with -m */
-	int		Oflag = 0, mflag = 0;	/* -O or -m was used */
+	struct stat	st;
+	int		atflag = 0;
 	unsigned int	mask = STATX_ALL;
+	int		compare = 0;
 
-	while ((c = getopt(argc, argv, "m:FDLOA")) != EOF) {
+	while ((c = getopt(argc, argv, "cm:FD")) != EOF) {
 		switch (c) {
+		case 'c':
+			compare = 1;
+			break;
 		case 'm':
-			m_mask = atoi(optarg);
-			mflag = 1;
+			if (strcmp(optarg, "basic") == 0)
+				mask = STATX_BASIC_STATS;
+			else if (strcmp(optarg, "all") == 0)
+				mask = STATX_ALL;
+			else
+				mask = strtoul(optarg, NULL, 0);
 			break;
 		case 'F':
 			atflag &= ~AT_STATX_SYNC_TYPE;
@@ -258,38 +322,28 @@ statx_f(
 			atflag &= ~AT_STATX_SYNC_TYPE;
 			atflag |= AT_STATX_DONT_SYNC;
 			break;
-		case 'L':
-			atflag &= ~AT_SYMLINK_NOFOLLOW;
-			break;
-		case 'O':
-			mask = STATX_BASIC_STATS;
-			Oflag = 1;
-			break;
-		case 'A':
-			atflag |= AT_NO_AUTOMOUNT;
-			break;
 		default:
 			return command_usage(&statx_cmd);
 		}
 	}
 
-	if (Oflag && mflag) {
-		printf("Cannot specify both -m mask and -O\n");
-		return 0;
-	}
-
-	/* -m overrides any other mask options */
-	if (mflag)
-		mask = m_mask;
-
 	memset(&stx, 0xbf, sizeof(stx));
-	if (statx(AT_FDCWD, file->name, atflag, mask, &stx) < 0) {
+
+	if (statx(file->fd, NULL, atflag, mask, &stx) < 0) {
 		perror("statx");
 		return 0;
 	}
 
-	dump_statx(&stx);
+	if (compare) {
+		if (fstat(file->fd, &st) < 0) {
+			perror("fstat");
+			return 0;
+		}
 
+		cmp_statx(&stx, &st);
+	}
+	
+	dump_statx(&stx);
 	return 0;
 }
 
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 77ba760..d82f882 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -884,16 +884,23 @@ 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 " ]"
+.BR statx " [ " "\-m mask" " ][ \-" cFD " ]"
 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)
+Specify the field mask for the statx call as an decimal, hex or octal integer
+or
+.RI \" basic "\" or \"" all \"
+to specify the basic stats that
+.IR stat ()
+returns or all the stats known by the header file.  All is the default.
 .TP
-.B \-O
-Add only basic stats (STATX_BASIC_STATS) to default mask
+.B \-c
+Do an
+.IR fstat ()
+call as well and compare the buffers.
 .TP
 .B \-F
 Force the attributes to be sync'd with the server
@@ -901,12 +908,6 @@ Force the attributes to be sync'd with the server
 .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

Re: [PATCH] xfs_io: changes to statx interface [ver #2]

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 6471 bytes --]

On Mar 28, 2017, at 8:38 AM, David Howells <dhowells@redhat.com> wrote:
> 
> Here are my current changes to Eric's statx interface patch.  I've made it
> analoguous to the stat command and so it only does statx-of-fd.
> 
> I've made the "-m" flag able to take the words "basic" and "all" in
> preference to a number and able to take an octal or hex number as an
> alternative too.
> 
> I've dropped the -A and -L flags since it no longer passes a path over.
> 
> Finally, I've added a -c flag that causes an fstat() to be done as well and
> the buffers compared as a consistency check.

It probably makes sense to use "-C" to avoid confusion with xfs_io's "-c"
option, and also the "stat -c" option to dump only specific fields (which
may be useful to add at some point in the future as well).

Cheers, Andreas

> 
> David
> ---
> diff --git a/io/stat.c b/io/stat.c
> index a7aebcd..961b6d1 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -189,12 +189,10 @@ statx_help(void)
> " Display extended file status.\n"
> "\n"
> " Options:\n"
> -" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
> -" -A -- Suppress terminal automount traversal\n"
> +" -c -- Compare against fstat/fstatat on the same file/fd\n"
> +" -m mask -- Specify the field mask for the statx call (can also be 'basic' or 'all'; default STATX_ALL)\n"
> " -D -- Don't sync attributes with the server\n"
> " -F -- Force the attributes to be sync'd with the server\n"
> -" -L -- Follow symlinks (statx link target)\n"
> -" -O -- Add only basic stats (STATX_BASIC_STATS) to default mask\n"
> "\n"));
> }
> 
> @@ -227,6 +225,65 @@ dump_statx(struct statx *stx)
> }
> 
> /*
> + * Compare the contents of a statx struct with that of a stat struct and check
> + * that they're the same.
> + */
> +static int
> +cmp_statx(const struct statx *stx, const struct stat *st)
> +{
> +	const char *what = NULL;
> +
> +#define cmp(x) \
> +	do {					\
> +		what = #x;		\
> +		if (stx->stx_##x != st->st_##x)	\
> +			goto mismatch;		\
> +	} while (0)
> +
> +	cmp(blksize);
> +	cmp(nlink);
> +	cmp(uid);
> +	cmp(gid);
> +	cmp(mode);
> +	cmp(ino);
> +	cmp(size);
> +	cmp(blocks);
> +
> +#define devcmp(x) \
> +	do {						\
> +		what = #x".major";			\
> +		if (stx->stx_##x##_major != major(st->st_##x))	\
> +			goto mismatch;			\
> +		what = #x".minor";			\
> +		if (stx->stx_##x##_minor != minor(st->st_##x))	\
> +			goto mismatch;			\
> +	} while (0)
> +
> +	devcmp(dev);
> +	devcmp(rdev);
> +
> +#define timecmp(x) \
> +	do {						\
> +		what = #x".tv_sec";			\
> +		if (stx->stx_##x##time.tv_sec != st->st_##x##tim.tv_sec)	\
> +			goto mismatch;			\
> +		what = #x".tv_nsec";			\
> +		if (stx->stx_##x##time.tv_nsec != st->st_##x##tim.tv_nsec)	\
> +			goto mismatch;			\
> +	} while (0)
> +
> +	timecmp(a);
> +	timecmp(c);
> +	timecmp(m);
> +
> +	return 0;
> +
> +mismatch:
> +	fprintf(stderr, "Mismatch between stat and statx output (%s)\n", what);
> +	return -1;
> +}
> +
> +/*
>  * options:
>  * 	- input flags - query type
>  * 	- output style for flags (and all else?) (chars vs. hex?)
> @@ -239,16 +296,23 @@ statx_f(
> {
> 	int		c;
> 	struct statx	stx;
> -	int		atflag = AT_SYMLINK_NOFOLLOW;
> -	unsigned int	m_mask = 0;	/* mask requested with -m */
> -	int		Oflag = 0, mflag = 0;	/* -O or -m was used */
> +	struct stat	st;
> +	int		atflag = 0;
> 	unsigned int	mask = STATX_ALL;
> +	int		compare = 0;
> 
> -	while ((c = getopt(argc, argv, "m:FDLOA")) != EOF) {
> +	while ((c = getopt(argc, argv, "cm:FD")) != EOF) {
> 		switch (c) {
> +		case 'c':
> +			compare = 1;
> +			break;
> 		case 'm':
> -			m_mask = atoi(optarg);
> -			mflag = 1;
> +			if (strcmp(optarg, "basic") == 0)
> +				mask = STATX_BASIC_STATS;
> +			else if (strcmp(optarg, "all") == 0)
> +				mask = STATX_ALL;
> +			else
> +				mask = strtoul(optarg, NULL, 0);
> 			break;
> 		case 'F':
> 			atflag &= ~AT_STATX_SYNC_TYPE;
> @@ -258,38 +322,28 @@ statx_f(
> 			atflag &= ~AT_STATX_SYNC_TYPE;
> 			atflag |= AT_STATX_DONT_SYNC;
> 			break;
> -		case 'L':
> -			atflag &= ~AT_SYMLINK_NOFOLLOW;
> -			break;
> -		case 'O':
> -			mask = STATX_BASIC_STATS;
> -			Oflag = 1;
> -			break;
> -		case 'A':
> -			atflag |= AT_NO_AUTOMOUNT;
> -			break;
> 		default:
> 			return command_usage(&statx_cmd);
> 		}
> 	}
> 
> -	if (Oflag && mflag) {
> -		printf("Cannot specify both -m mask and -O\n");
> -		return 0;
> -	}
> -
> -	/* -m overrides any other mask options */
> -	if (mflag)
> -		mask = m_mask;
> -
> 	memset(&stx, 0xbf, sizeof(stx));
> -	if (statx(AT_FDCWD, file->name, atflag, mask, &stx) < 0) {
> +
> +	if (statx(file->fd, NULL, atflag, mask, &stx) < 0) {
> 		perror("statx");
> 		return 0;
> 	}
> 
> -	dump_statx(&stx);
> +	if (compare) {
> +		if (fstat(file->fd, &st) < 0) {
> +			perror("fstat");
> +			return 0;
> +		}
> 
> +		cmp_statx(&stx, &st);
> +	}
> +
> +	dump_statx(&stx);
> 	return 0;
> }
> 
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 77ba760..d82f882 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -884,16 +884,23 @@ 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 " ]"
> +.BR statx " [ " "\-m mask" " ][ \-" cFD " ]"
> 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)
> +Specify the field mask for the statx call as an decimal, hex or octal integer
> +or
> +.RI \" basic "\" or \"" all \"
> +to specify the basic stats that
> +.IR stat ()
> +returns or all the stats known by the header file.  All is the default.
> .TP
> -.B \-O
> -Add only basic stats (STATX_BASIC_STATS) to default mask
> +.B \-c
> +Do an
> +.IR fstat ()
> +call as well and compare the buffers.
> .TP
> .B \-F
> Force the attributes to be sync'd with the server
> @@ -901,12 +908,6 @@ Force the attributes to be sync'd with the server
> .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


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] xfs_io: changes to statx interface [ver #2]

[Amir Goldstein]

On Tue, Mar 28, 2017 at 2:40 PM, Andreas Dilger <adilger@dilger.ca> wrote:
> On Mar 28, 2017, at 8:38 AM, David Howells <dhowells@redhat.com> wrote:
>>
>> Here are my current changes to Eric's statx interface patch.  I've made it
>> analoguous to the stat command and so it only does statx-of-fd.
>>
>> I've made the "-m" flag able to take the words "basic" and "all" in
>> preference to a number and able to take an octal or hex number as an
>> alternative too.
>>
>> I've dropped the -A and -L flags since it no longer passes a path over.
>>
>> Finally, I've added a -c flag that causes an fstat() to be done as well and
>> the buffers compared as a consistency check.
>
> It probably makes sense to use "-C" to avoid confusion with xfs_io's "-c"
> option, and also the "stat -c" option to dump only specific fields (which
> may be useful to add at some point in the future as well).
>

I'm afraid its not better for avoiding confusion.
-C already taken as well for new "oneshot commands"...

Re: [PATCH] xfs_io: changes to statx interface [ver #2]

[Eric Sandeen]

On 3/28/17 2:07 PM, Amir Goldstein wrote:
> On Tue, Mar 28, 2017 at 2:40 PM, Andreas Dilger <adilger@dilger.ca> wrote:
>> On Mar 28, 2017, at 8:38 AM, David Howells <dhowells@redhat.com> wrote:
>>>
>>> Here are my current changes to Eric's statx interface patch.  I've made it
>>> analoguous to the stat command and so it only does statx-of-fd.
>>>
>>> I've made the "-m" flag able to take the words "basic" and "all" in
>>> preference to a number and able to take an octal or hex number as an
>>> alternative too.
>>>
>>> I've dropped the -A and -L flags since it no longer passes a path over.
>>>
>>> Finally, I've added a -c flag that causes an fstat() to be done as well and
>>> the buffers compared as a consistency check.
>>
>> It probably makes sense to use "-C" to avoid confusion with xfs_io's "-c"
>> option, and also the "stat -c" option to dump only specific fields (which
>> may be useful to add at some point in the future as well).
>>
> 
> I'm afraid its not better for avoiding confusion.
> -C already taken as well for new "oneshot commands"...

There's no conflict here AFAICT; the "-C" for oneshot is for
xfs_io invocation; individual commands are free to re-use such
options for their own purposes.

i.e.:

# xfs_io -C "command -C" file

is fine; the two different -C args have different contexts.  Adding a
-C to the statx command poses no problem.

And with that public service announcement let me go read this thread and
catch up... ;)

-Eric

> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] xfs_io: changes to statx interface [ver #2]

[Dave Chinner]

On Tue, Mar 28, 2017 at 03:38:52PM +0100, David Howells wrote:
> Here are my current changes to Eric's statx interface patch.  I've made it
> analoguous to the stat command and so it only does statx-of-fd.
> 
> I've made the "-m" flag able to take the words "basic" and "all" in
> preference to a number and able to take an octal or hex number as an
> alternative too.
> 
> I've dropped the -A and -L flags since it no longer passes a path over.
> 
> Finally, I've added a -c flag that causes an fstat() to be done as well and
> the buffers compared as a consistency check.
> 
> David
> ---
> diff --git a/io/stat.c b/io/stat.c
> index a7aebcd..961b6d1 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -189,12 +189,10 @@ statx_help(void)
>  " Display extended file status.\n"
>  "\n"
>  " Options:\n"
> -" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
> -" -A -- Suppress terminal automount traversal\n"
> +" -c -- Compare against fstat/fstatat on the same file/fd\n"

Running "compare" tests like this is not a function that xfs_io
should perform. Have it run and output the statx information,
and use xfstests to compare that against a golden output and/or
the output of the xfs_io stat command.

i.e. xfs_io is a utility for executing commands, whilst xfstests is
used to test that the executed functionality has the correct output.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs_io: changes to statx interface [ver #2]

[David Howells]

Dave Chinner <david@fromorbit.com> wrote:

> Running "compare" tests like this is not a function that xfs_io
> should perform. Have it run and output the statx information,
> and use xfstests to compare that against a golden output and/or
> the output of the xfs_io stat command.

A golden output that covers timestamps isn't possible.

Also comparing the output of Eric's statx command with the output of the stat
command as it stands isn't practical.  I guess I can add another parameter to
the stat command to produce something that looks the same.

I'm tempted to write C-based python3 module that gives raw access to the
system calls (something I want for the unionmount testsuite) and then script
this in python.

David

Re: [PATCH] xfs_io: changes to statx interface [ver #2]

[David Howells]

David Howells <dhowells@redhat.com> wrote:

> Here are my current changes to Eric's statx interface patch.  I've made it
> analoguous to the stat command and so it only does statx-of-fd.
> 
> I've made the "-m" flag able to take the words "basic" and "all" in
> preference to a number and able to take an octal or hex number as an
> alternative too.
> 
> I've dropped the -A and -L flags since it no longer passes a path over.
> 
> Finally, I've added a -c flag that causes an fstat() to be done as well and
> the buffers compared as a consistency check.

Note that the intention is to put the testing of the syscall parameter
handling into LTP, along with testing of the symlink following and dirfd usage
since xfstests seems unsuitable for this.

David

Re: [PATCH] xfs_io: changes to statx interface [ver #2]

[David Howells]

David Howells <dhowells@redhat.com> wrote:

> Here are my current changes to Eric's statx interface patch.  I've made it
> analoguous to the stat command and so it only does statx-of-fd.
> 
> I've made the "-m" flag able to take the words "basic" and "all" in
> preference to a number and able to take an octal or hex number as an
> alternative too.
> 
> I've dropped the -A and -L flags since it no longer passes a path over.
> 
> Finally, I've added a -c flag that causes an fstat() to be done as well and
> the buffers compared as a consistency check.

Note that the intention is to put the testing of the syscall parameter
handling into LTP, along with testing of the symlink following and dirfd usage
since xfstests seems unsuitable for this.

David

Re: [PATCH] xfs_io: changes to statx interface [ver #2]

[Eric Sandeen]

I am (literally) in the woods this week so can't really comment at length, but generally:

xfs_io is useful to facilitate testing at times, but usually doesn't have tests built into the tool itself.

And I think you are right that it is a poor fit for some of the testing you'd like to do.  There is a src/ dir in xfstests for specialized C tests which get called by the test harness; that might also be a reasonable option.

Thanks, 
Eric

> On Mar 28, 2017, at 9:41 AM, David Howells <dhowells@redhat.com> wrote:
> 
> Note that the intention is to put the testing of the syscall parameter
> handling into LTP, along with testing of the symlink following and dirfd usage
> since xfstests seems unsuitable for this.
> 
> David

Re: [PATCH] xfs_io: changes to statx interface [ver #2]

[Amir Goldstein]

On Tue, Mar 28, 2017 at 12:42 PM, Eric Sandeen <sandeen@sandeen.net> wrote:
> I am (literally) in the woods this week so can't really comment at length, but generally:
>
> xfs_io is useful to facilitate testing at times, but usually doesn't have tests built into the tool itself.
>

David,

Eric being the maintainer, so he gets to decide, but he is being
somewhat subtle.
There is no precedence AFAIK to tests within xfs_io.
Instead of comparing to stat with -c flag, you can make sure that
output of xfs_io -c 'statx -c'
is fully compatible to output of xfs_io -c 'stat -v' and do the test in scripts.

> And I think you are right that it is a poor fit for some of the testing you'd like to do.
> There is a src/ dir in xfstests for specialized C tests which get called by the test harness; that might also be a reasonable option.
>

That's actually the shortest path for you and there are quite a few
tests in xfstests that took
this path. It should be easy for you to copy & paste one of them.

That's not instead of adding xfs_io statx - just for tests that are
not natural to do with xfs_io.

> Thanks,
> Eric
>
>> On Mar 28, 2017, at 9:41 AM, David Howells <dhowells@redhat.com> wrote:
>>
>> Note that the intention is to put the testing of the syscall parameter
>> handling into LTP, along with testing of the symlink following and dirfd usage
>> since xfstests seems unsuitable for this.
>>
>> David
>

Re: [PATCH] xfs_io: changes to statx interface [ver #2]

[Eric Sandeen]

On 3/28/17 12:35 PM, Amir Goldstein wrote:
> On Tue, Mar 28, 2017 at 12:42 PM, Eric Sandeen <sandeen@sandeen.net> wrote:
>> I am (literally) in the woods this week so can't really comment at length, but generally:
>>
>> xfs_io is useful to facilitate testing at times, but usually doesn't have tests built into the tool itself.
>>
> 
> David,
> 
> Eric being the maintainer, so he gets to decide, but he is being
> somewhat subtle.

Sorry about that.  More like: terse, with only a phone to communicate,
and a wife who thinks we're on vacation.  ;)

> There is no precedence AFAIK to tests within xfs_io.
> Instead of comparing to stat with -c flag, you can make sure that
> output of xfs_io -c 'statx -c'
> is fully compatible to output of xfs_io -c 'stat -v' and do the test in scripts.

*nod*

>> And I think you are right that it is a poor fit for some of the testing you'd like to do.
>> There is a src/ dir in xfstests for specialized C tests which get called by the test harness; that might also be a reasonable option.
>>
> 
> That's actually the shortest path for you and there are quite a few
> tests in xfstests that took
> this path. It should be easy for you to copy & paste one of them.
> 
> That's not instead of adding xfs_io statx - just for tests that are
> not natural to do with xfs_io.

Agreed.

I think there's value in having a statx command in xfs_io, but it won't
be able to facilitate testing /everything/ that needs to be tested.

Thanks,
-Eric

>> Thanks,
>> Eric
>>
>>> On Mar 28, 2017, at 9:41 AM, David Howells <dhowells@redhat.com> wrote:
>>>
>>> Note that the intention is to put the testing of the syscall parameter
>>> handling into LTP, along with testing of the symlink following and dirfd usage
>>> since xfstests seems unsuitable for this.
>>>
>>> David
>>
> 

Re: [PATCH] xfs_io: changes to statx interface [ver #2]

[David Howells]

Amir Goldstein <amir73il@gmail.com> wrote:

> That's actually the shortest path for you and there are quite a few
> tests in xfstests that took
> this path. It should be easy for you to copy & paste one of them.

Don't forget that syscall testing for this has to be added to LTP anyway...

David

Re: [PATCH] xfs_io: changes to statx interface [ver #2]

[Amir Goldstein]

On Tue, Mar 28, 2017 at 1:56 PM, David Howells <dhowells@redhat.com> wrote:
> Amir Goldstein <amir73il@gmail.com> wrote:
>
>> That's actually the shortest path for you and there are quite a few
>> tests in xfstests that took
>> this path. It should be easy for you to copy & paste one of them.
>
> Don't forget that syscall testing for this has to be added to LTP anyway...
>

Sure, LTP is good for that and many testers run LTP, but I have no idea how
wide is the file system diversity of their setups.
statx may have bugs in syscall/vfs and it may have bugs in specific
fs, so having
some C tests in LTP and some C tests in xfstest can't hurt.

It's not difficult to harness a C test to xfstest.
See recent example of C only (mostly) test in
 2161614 generic: concurrent non-overlapping direct I/O on the same extents

[PATCH] xfs_io: changes to statx interface [ver #3]

[David Howells]

Here's my third attempt at changing Eric's statx patch.  I've got rid of the
compare option to statx and added a raw output option to stat.  They're still
not directly comparable since the stat output lacks some fields, but it will
hopefully be possible to load them into associative arrays in bash and compare
them that way.

---
diff --git a/io/stat.c b/io/stat.c
index a7aebcd..d2ea854 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -44,6 +44,35 @@ filesize(void)
 	return st.st_size;
 }
 
+static int
+dump_raw_stat(struct stat *st)
+{
+	if (fstat(file->fd, st) < 0) {
+		perror("fstat");
+		return -1;
+	}
+
+	printf("stat.blksize: %lu\n", st->st_blksize);
+	printf("stat.nlink: %lu\n", st->st_nlink);
+	printf("stat.uid: %u\n", st->st_uid);
+	printf("stat.gid: %u\n", st->st_gid);
+	printf("stat.mode: 0%o\n", st->st_mode);
+	printf("stat.ino: %lu\n", st->st_ino);
+	printf("stat.size: %lu\n", st->st_size);
+	printf("stat.blocks: %lu\n", st->st_blocks);
+	printf("stat.atime.tv_sec: %ld\n", st->st_atim.tv_sec);
+	printf("stat.atime.tv_nsec: %ld\n", st->st_atim.tv_nsec);
+	printf("stat.ctime.tv_sec: %ld\n", st->st_ctim.tv_sec);
+	printf("stat.ctime.tv_nsec: %ld\n", st->st_ctim.tv_nsec);
+	printf("stat.mtime.tv_sec: %ld\n", st->st_mtim.tv_sec);
+	printf("stat.mtime.tv_nsec: %ld\n", st->st_mtim.tv_nsec);
+	printf("stat.rdev_major: %u\n", major(st->st_rdev));
+	printf("stat.rdev_minor: %u\n", minor(st->st_rdev));
+	printf("stat.dev_major: %u\n", major(st->st_dev));
+	printf("stat.dev_minor: %u\n", minor(st->st_dev));
+	return 0;
+}
+
 static char *
 filetype(mode_t mode)
 {
@@ -74,7 +103,23 @@ stat_f(
 	struct dioattr	dio;
 	struct fsxattr	fsx, fsxa;
 	struct stat	st;
-	int		verbose = (argc == 2 && !strcmp(argv[1], "-v"));
+	int		c, verbose = 0, raw = 0;
+
+	while ((c = getopt(argc, argv, "rv")) != EOF) {
+		switch (c) {
+		case 'r':
+			raw = 1;
+			break;
+		case 'v':
+			verbose = 1;
+			break;
+		default:
+			return command_usage(&stat_cmd);
+		}
+	}
+
+	if (raw)
+		return dump_raw_stat(&st);
 
 	printf(_("fd.path = \"%s\"\n"), file->name);
 	printf(_("fd.flags = %s,%s,%s%s%s%s%s\n"),
@@ -189,12 +234,9 @@ statx_help(void)
 " Display extended file status.\n"
 "\n"
 " Options:\n"
-" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
-" -A -- Suppress terminal automount traversal\n"
+" -m mask -- Specify the field mask for the statx call (can also be 'basic' or 'all'; default STATX_ALL)\n"
 " -D -- Don't sync attributes with the server\n"
 " -F -- Force the attributes to be sync'd with the server\n"
-" -L -- Follow symlinks (statx link target)\n"
-" -O -- Add only basic stats (STATX_BASIC_STATS) to default mask\n"
 "\n"));
 }
 
@@ -202,28 +244,28 @@ statx_help(void)
 static void
 dump_statx(struct statx *stx)
 {
-	printf("stx_mask: 0x%x\n", stx->stx_mask);
-	printf("stx_blksize: %u\n", stx->stx_blksize);
-	printf("stx_attributes: 0x%llx\n", stx->stx_attributes);
-	printf("stx_nlink: %u\n", stx->stx_nlink);
-	printf("stx_uid: %u\n", stx->stx_uid);
-	printf("stx_gid: %u\n", stx->stx_gid);
-	printf("stx_mode: 0%o\n", stx->stx_mode);
-	printf("stx_ino: %llu\n", stx->stx_ino);
-	printf("stx_size: %llu\n", stx->stx_size);
-	printf("stx_blocks: %llu\n", stx->stx_blocks);
-	printf("stx_atime.tv_sec: %lld\n", stx->stx_atime.tv_sec);
-	printf("stx_atime.tv_nsec: %d\n", stx->stx_atime.tv_nsec);
-	printf("stx_btime.tv_sec: %lld\n", stx->stx_btime.tv_sec);
-	printf("stx_btime.tv_nsec: %d\n", stx->stx_btime.tv_nsec);
-	printf("stx_ctime.tv_sec: %lld\n", stx->stx_ctime.tv_sec);
-	printf("stx_ctime.tv_nsec: %d\n", stx->stx_ctime.tv_nsec);
-	printf("stx_mtime.tv_sec: %lld\n", stx->stx_mtime.tv_sec);
-	printf("stx_mtime.tv_nsec: %d\n", stx->stx_mtime.tv_nsec);
-	printf("stx_rdev_major: %u\n", stx->stx_rdev_major);
-	printf("stx_rdev_minor: %u\n", stx->stx_rdev_minor);
-	printf("stx_dev_major: %u\n", stx->stx_dev_major);
-	printf("stx_dev_minor: %u\n", stx->stx_dev_minor);
+	printf("stat.mask: 0x%x\n", stx->stx_mask);
+	printf("stat.blksize: %u\n", stx->stx_blksize);
+	printf("stat.attributes: 0x%llx\n", stx->stx_attributes);
+	printf("stat.nlink: %u\n", stx->stx_nlink);
+	printf("stat.uid: %u\n", stx->stx_uid);
+	printf("stat.gid: %u\n", stx->stx_gid);
+	printf("stat.mode: 0%o\n", stx->stx_mode);
+	printf("stat.ino: %llu\n", stx->stx_ino);
+	printf("stat.size: %llu\n", stx->stx_size);
+	printf("stat.blocks: %llu\n", stx->stx_blocks);
+	printf("stat.atime.tv_sec: %lld\n", stx->stx_atime.tv_sec);
+	printf("stat.atime.tv_nsec: %d\n", stx->stx_atime.tv_nsec);
+	printf("stat.btime.tv_sec: %lld\n", stx->stx_btime.tv_sec);
+	printf("stat.btime.tv_nsec: %d\n", stx->stx_btime.tv_nsec);
+	printf("stat.ctime.tv_sec: %lld\n", stx->stx_ctime.tv_sec);
+	printf("stat.ctime.tv_nsec: %d\n", stx->stx_ctime.tv_nsec);
+	printf("stat.mtime.tv_sec: %lld\n", stx->stx_mtime.tv_sec);
+	printf("stat.mtime.tv_nsec: %d\n", stx->stx_mtime.tv_nsec);
+	printf("stat.rdev_major: %u\n", stx->stx_rdev_major);
+	printf("stat.rdev_minor: %u\n", stx->stx_rdev_minor);
+	printf("stat.dev_major: %u\n", stx->stx_dev_major);
+	printf("stat.dev_minor: %u\n", stx->stx_dev_minor);
 }
 
 /*
@@ -239,16 +281,18 @@ statx_f(
 {
 	int		c;
 	struct statx	stx;
-	int		atflag = AT_SYMLINK_NOFOLLOW;
-	unsigned int	m_mask = 0;	/* mask requested with -m */
-	int		Oflag = 0, mflag = 0;	/* -O or -m was used */
+	int		atflag = 0;
 	unsigned int	mask = STATX_ALL;
 
-	while ((c = getopt(argc, argv, "m:FDLOA")) != EOF) {
+	while ((c = getopt(argc, argv, "m:FD")) != EOF) {
 		switch (c) {
 		case 'm':
-			m_mask = atoi(optarg);
-			mflag = 1;
+			if (strcmp(optarg, "basic") == 0)
+				mask = STATX_BASIC_STATS;
+			else if (strcmp(optarg, "all") == 0)
+				mask = STATX_ALL;
+			else
+				mask = strtoul(optarg, NULL, 0);
 			break;
 		case 'F':
 			atflag &= ~AT_STATX_SYNC_TYPE;
@@ -258,38 +302,19 @@ statx_f(
 			atflag &= ~AT_STATX_SYNC_TYPE;
 			atflag |= AT_STATX_DONT_SYNC;
 			break;
-		case 'L':
-			atflag &= ~AT_SYMLINK_NOFOLLOW;
-			break;
-		case 'O':
-			mask = STATX_BASIC_STATS;
-			Oflag = 1;
-			break;
-		case 'A':
-			atflag |= AT_NO_AUTOMOUNT;
-			break;
 		default:
 			return command_usage(&statx_cmd);
 		}
 	}
 
-	if (Oflag && mflag) {
-		printf("Cannot specify both -m mask and -O\n");
-		return 0;
-	}
-
-	/* -m overrides any other mask options */
-	if (mflag)
-		mask = m_mask;
-
 	memset(&stx, 0xbf, sizeof(stx));
-	if (statx(AT_FDCWD, file->name, atflag, mask, &stx) < 0) {
+
+	if (statx(file->fd, NULL, atflag, mask, &stx) < 0) {
 		perror("statx");
 		return 0;
 	}
-
+	
 	dump_statx(&stx);
-
 	return 0;
 }
 
@@ -301,7 +326,7 @@ stat_init(void)
 	stat_cmd.argmin = 0;
 	stat_cmd.argmax = 1;
 	stat_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	stat_cmd.args = _("[-v]");
+	stat_cmd.args = _("[-rv]");
 	stat_cmd.oneline = _("information about the currently open file");
 
 	statfs_cmd.name = "statfs";
@@ -315,7 +340,7 @@ stat_init(void)
 	statx_cmd.argmin = 0;
 	statx_cmd.argmax = -1;
 	statx_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	statx_cmd.args = _("[-O | -m mask][-FDLAP]");
+	statx_cmd.args = _("[-m basic | -m all | -m <mask>][-FD]");
 	statx_cmd.oneline =
  _("extended information about the currently open file");
 	statx_cmd.help = statx_help;
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 77ba760..486ad11 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -876,24 +876,29 @@ Only available in expert mode and requires privileges.
 Force the filesystem to shutdown (with or without flushing the log).
 Only available in expert mode and requires privileges.
 .TP
-.BR stat " [ " \-v " ]"
+.BR stat " [ \-" rv " ]"
 Selected statistics from
 .BR stat (2)
 and the XFS_IOC_GETXATTR system call on the current file. If the
 .B \-v
 option is specified, the atime (last access), mtime
-(last modify), and ctime (last change) timestamps are also displayed.
+(last modify), and ctime (last change) timestamps are also displayed. If the
+.B \-r
+option is specified, the raw output will be dumped in the same form as the
+output for the statx command, but with some fields missing.
 .TP
-.BR statx " [ " \-O " | " "\-m mask" " ][ \-" FDLA " ]"
+.BR statx " [ " "\-m mask" " ][ \-" FD " ]"
 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
+Specify the field mask for the statx call as an decimal, hex or octal integer
+or
+.RI \" basic "\" or \"" all \"
+to specify the basic stats that
+.IR stat ()
+returns or all the stats known by the header file.  All is the default.
 .TP
 .B \-F
 Force the attributes to be sync'd with the server
@@ -901,12 +906,6 @@ Force the attributes to be sync'd with the server
 .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

Re: [PATCH] xfs_io: changes to statx interface [ver #3]

[Eric Sandeen]

On 3/29/17 11:49 AM, David Howells wrote:
> Here's my third attempt at changing Eric's statx patch.  I've got rid of the
> compare option to statx and added a raw output option to stat.  They're still
> not directly comparable since the stat output lacks some fields, but it will
> hopefully be possible to load them into associative arrays in bash and compare
> them that way.

Thanks for working on this; comments below.

But - do you plan to send this as (a) formal patch(es) with signed-off-by
etc?  Or do you want me to just fold this into my patch with some
sort of Changes-inspired-by: David Howells <dhowells@redhat.com> tag? ;)

The changes to stat_f should be their own standalone patch, I think.

(meh, it's kind of unfortunate that the current stat_f does more
than just stat, isn't it?)

I may make further changes so that a bare "statx" prints exactly the
same info as a bare "stat", and a new "-r" to each one dumps the raw
stat[x] structure.  Just for some symmetry...

> ---
> diff --git a/io/stat.c b/io/stat.c
> index a7aebcd..d2ea854 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -44,6 +44,35 @@ filesize(void)
>  	return st.st_size;
>  }
>  
> +static int
> +dump_raw_stat(struct stat *st)
> +{
> +	if (fstat(file->fd, st) < 0) {
> +		perror("fstat");
> +		return -1;
> +	}
> +
> +	printf("stat.blksize: %lu\n", st->st_blksize);
> +	printf("stat.nlink: %lu\n", st->st_nlink);
> +	printf("stat.uid: %u\n", st->st_uid);
> +	printf("stat.gid: %u\n", st->st_gid);
> +	printf("stat.mode: 0%o\n", st->st_mode);
> +	printf("stat.ino: %lu\n", st->st_ino);
> +	printf("stat.size: %lu\n", st->st_size);
> +	printf("stat.blocks: %lu\n", st->st_blocks);
> +	printf("stat.atime.tv_sec: %ld\n", st->st_atim.tv_sec);
> +	printf("stat.atime.tv_nsec: %ld\n", st->st_atim.tv_nsec);
> +	printf("stat.ctime.tv_sec: %ld\n", st->st_ctim.tv_sec);
> +	printf("stat.ctime.tv_nsec: %ld\n", st->st_ctim.tv_nsec);
> +	printf("stat.mtime.tv_sec: %ld\n", st->st_mtim.tv_sec);
> +	printf("stat.mtime.tv_nsec: %ld\n", st->st_mtim.tv_nsec);
> +	printf("stat.rdev_major: %u\n", major(st->st_rdev));
> +	printf("stat.rdev_minor: %u\n", minor(st->st_rdev));
> +	printf("stat.dev_major: %u\n", major(st->st_dev));
> +	printf("stat.dev_minor: %u\n", minor(st->st_dev));
> +	return 0;
> +}
> +
>  static char *
>  filetype(mode_t mode)
>  {
> @@ -74,7 +103,23 @@ stat_f(
>  	struct dioattr	dio;
>  	struct fsxattr	fsx, fsxa;
>  	struct stat	st;
> -	int		verbose = (argc == 2 && !strcmp(argv[1], "-v"));
> +	int		c, verbose = 0, raw = 0;
> +
> +	while ((c = getopt(argc, argv, "rv")) != EOF) {
> +		switch (c) {
> +		case 'r':
> +			raw = 1;
> +			break;
> +		case 'v':
> +			verbose = 1;
> +			break;
> +		default:
> +			return command_usage(&stat_cmd);
> +		}
> +	}
> +
> +	if (raw)
> +		return dump_raw_stat(&st);
>  
>  	printf(_("fd.path = \"%s\"\n"), file->name);
>  	printf(_("fd.flags = %s,%s,%s%s%s%s%s\n"),
> @@ -189,12 +234,9 @@ statx_help(void)
>  " Display extended file status.\n"
>  "\n"
>  " Options:\n"
> -" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
> -" -A -- Suppress terminal automount traversal\n"
> +" -m mask -- Specify the field mask for the statx call (can also be 'basic' or 'all'; default STATX_ALL)\n"
>  " -D -- Don't sync attributes with the server\n"
>  " -F -- Force the attributes to be sync'd with the server\n"
> -" -L -- Follow symlinks (statx link target)\n"
> -" -O -- Add only basic stats (STATX_BASIC_STATS) to default mask\n"

ok, I like the basic/all/mask better, yes.

>  "\n"));
>  }
>  
> @@ -202,28 +244,28 @@ statx_help(void)
>  static void
>  dump_statx(struct statx *stx)
>  {
> -	printf("stx_mask: 0x%x\n", stx->stx_mask);
> -	printf("stx_blksize: %u\n", stx->stx_blksize);
> -	printf("stx_attributes: 0x%llx\n", stx->stx_attributes);
> -	printf("stx_nlink: %u\n", stx->stx_nlink);
> -	printf("stx_uid: %u\n", stx->stx_uid);
> -	printf("stx_gid: %u\n", stx->stx_gid);
> -	printf("stx_mode: 0%o\n", stx->stx_mode);
> -	printf("stx_ino: %llu\n", stx->stx_ino);
> -	printf("stx_size: %llu\n", stx->stx_size);
> -	printf("stx_blocks: %llu\n", stx->stx_blocks);
> -	printf("stx_atime.tv_sec: %lld\n", stx->stx_atime.tv_sec);
> -	printf("stx_atime.tv_nsec: %d\n", stx->stx_atime.tv_nsec);
> -	printf("stx_btime.tv_sec: %lld\n", stx->stx_btime.tv_sec);
> -	printf("stx_btime.tv_nsec: %d\n", stx->stx_btime.tv_nsec);
> -	printf("stx_ctime.tv_sec: %lld\n", stx->stx_ctime.tv_sec);
> -	printf("stx_ctime.tv_nsec: %d\n", stx->stx_ctime.tv_nsec);
> -	printf("stx_mtime.tv_sec: %lld\n", stx->stx_mtime.tv_sec);
> -	printf("stx_mtime.tv_nsec: %d\n", stx->stx_mtime.tv_nsec);
> -	printf("stx_rdev_major: %u\n", stx->stx_rdev_major);
> -	printf("stx_rdev_minor: %u\n", stx->stx_rdev_minor);
> -	printf("stx_dev_major: %u\n", stx->stx_dev_major);
> -	printf("stx_dev_minor: %u\n", stx->stx_dev_minor);
> +	printf("stat.mask: 0x%x\n", stx->stx_mask);

Seems a little weird to be printing something that looks
like structure.member, when it's not actually the structure
or member name, but I suppose it facilitates parsing the output
for testing?

> +	printf("stat.blksize: %u\n", stx->stx_blksize);
> +	printf("stat.attributes: 0x%llx\n", stx->stx_attributes);
> +	printf("stat.nlink: %u\n", stx->stx_nlink);
> +	printf("stat.uid: %u\n", stx->stx_uid);
> +	printf("stat.gid: %u\n", stx->stx_gid);
> +	printf("stat.mode: 0%o\n", stx->stx_mode);
> +	printf("stat.ino: %llu\n", stx->stx_ino);
> +	printf("stat.size: %llu\n", stx->stx_size);
> +	printf("stat.blocks: %llu\n", stx->stx_blocks);
> +	printf("stat.atime.tv_sec: %lld\n", stx->stx_atime.tv_sec);
> +	printf("stat.atime.tv_nsec: %d\n", stx->stx_atime.tv_nsec);
> +	printf("stat.btime.tv_sec: %lld\n", stx->stx_btime.tv_sec);
> +	printf("stat.btime.tv_nsec: %d\n", stx->stx_btime.tv_nsec);
> +	printf("stat.ctime.tv_sec: %lld\n", stx->stx_ctime.tv_sec);
> +	printf("stat.ctime.tv_nsec: %d\n", stx->stx_ctime.tv_nsec);
> +	printf("stat.mtime.tv_sec: %lld\n", stx->stx_mtime.tv_sec);
> +	printf("stat.mtime.tv_nsec: %d\n", stx->stx_mtime.tv_nsec);
> +	printf("stat.rdev_major: %u\n", stx->stx_rdev_major);
> +	printf("stat.rdev_minor: %u\n", stx->stx_rdev_minor);
> +	printf("stat.dev_major: %u\n", stx->stx_dev_major);
> +	printf("stat.dev_minor: %u\n", stx->stx_dev_minor);
>  }
>  
>  /*
> @@ -239,16 +281,18 @@ statx_f(
>  {
>  	int		c;
>  	struct statx	stx;
> -	int		atflag = AT_SYMLINK_NOFOLLOW;
> -	unsigned int	m_mask = 0;	/* mask requested with -m */
> -	int		Oflag = 0, mflag = 0;	/* -O or -m was used */
> +	int		atflag = 0;
>  	unsigned int	mask = STATX_ALL;
>  
> -	while ((c = getopt(argc, argv, "m:FDLOA")) != EOF) {
> +	while ((c = getopt(argc, argv, "m:FD")) != EOF) {
>  		switch (c) {
>  		case 'm':
> -			m_mask = atoi(optarg);
> -			mflag = 1;
> +			if (strcmp(optarg, "basic") == 0)
> +				mask = STATX_BASIC_STATS;
> +			else if (strcmp(optarg, "all") == 0)
> +				mask = STATX_ALL;
> +			else
> +				mask = strtoul(optarg, NULL, 0);

best to check for failure here I suppose, but I've already got 
that in my current patch.

>  			break;
>  		case 'F':
>  			atflag &= ~AT_STATX_SYNC_TYPE;
> @@ -258,38 +302,19 @@ statx_f(
>  			atflag &= ~AT_STATX_SYNC_TYPE;
>  			atflag |= AT_STATX_DONT_SYNC;
>  			break;
> -		case 'L':
> -			atflag &= ~AT_SYMLINK_NOFOLLOW;
> -			break;
> -		case 'O':
> -			mask = STATX_BASIC_STATS;
> -			Oflag = 1;
> -			break;
> -		case 'A':
> -			atflag |= AT_NO_AUTOMOUNT;
> -			break;
>  		default:
>  			return command_usage(&statx_cmd);
>  		}
>  	}
>  
> -	if (Oflag && mflag) {
> -		printf("Cannot specify both -m mask and -O\n");
> -		return 0;
> -	}
> -
> -	/* -m overrides any other mask options */
> -	if (mflag)
> -		mask = m_mask;
> -
>  	memset(&stx, 0xbf, sizeof(stx));
> -	if (statx(AT_FDCWD, file->name, atflag, mask, &stx) < 0) {
> +
> +	if (statx(file->fd, NULL, atflag, mask, &stx) < 0) {
>  		perror("statx");
>  		return 0;
>  	}
> -
> +	

whitespace ;)

>  	dump_statx(&stx);
> -
>  	return 0;
>  }
>  
> @@ -301,7 +326,7 @@ stat_init(void)
>  	stat_cmd.argmin = 0;
>  	stat_cmd.argmax = 1;
>  	stat_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> -	stat_cmd.args = _("[-v]");
> +	stat_cmd.args = _("[-rv]");
>  	stat_cmd.oneline = _("information about the currently open file");
>  
>  	statfs_cmd.name = "statfs";
> @@ -315,7 +340,7 @@ stat_init(void)
>  	statx_cmd.argmin = 0;
>  	statx_cmd.argmax = -1;
>  	statx_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> -	statx_cmd.args = _("[-O | -m mask][-FDLAP]");
> +	statx_cmd.args = _("[-m basic | -m all | -m <mask>][-FD]");
>  	statx_cmd.oneline =
>   _("extended information about the currently open file");
>  	statx_cmd.help = statx_help;
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 77ba760..486ad11 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -876,24 +876,29 @@ Only available in expert mode and requires privileges.
>  Force the filesystem to shutdown (with or without flushing the log).
>  Only available in expert mode and requires privileges.
>  .TP
> -.BR stat " [ " \-v " ]"
> +.BR stat " [ \-" rv " ]"
>  Selected statistics from
>  .BR stat (2)
>  and the XFS_IOC_GETXATTR system call on the current file. If the
>  .B \-v
>  option is specified, the atime (last access), mtime
> -(last modify), and ctime (last change) timestamps are also displayed.
> +(last modify), and ctime (last change) timestamps are also displayed. If the
> +.B \-r
> +option is specified, the raw output will be dumped in the same form as the
> +output for the statx command, but with some fields missing.
>  .TP
> -.BR statx " [ " \-O " | " "\-m mask" " ][ \-" FDLA " ]"
> +.BR statx " [ " "\-m mask" " ][ \-" FD " ]"
>  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
> +Specify the field mask for the statx call as an decimal, hex or octal integer
> +or
> +.RI \" basic "\" or \"" all \"
> +to specify the basic stats that
> +.IR stat ()
> +returns or all the stats known by the header file.  All is the default.
>  .TP
>  .B \-F
>  Force the attributes to be sync'd with the server
> @@ -901,12 +906,6 @@ Force the attributes to be sync'd with the server
>  .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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] xfs_io: changes to statx interface [ver #3]

[David Howells]

David Howells <dhowells@redhat.com> wrote:

> Here's my third attempt at changing Eric's statx patch.  I've got rid of the
> compare option to statx and added a raw output option to stat.  They're still
> not directly comparable since the stat output lacks some fields, but it will
> hopefully be possible to load them into associative arrays in bash and compare
> them that way.

Kind of like this:

function cmpstat () {
    file=$1
    declare -A st stx

    # Read the stat output
    coproc { xfs_io -c "open -P $file" -c "stat -r" | sed -e s/://; }
    while read tag val
    do
	st[$tag]=$val
    done <&${COPROC[0]}
    wait || exit $?

    # Read the statx output
    coproc { xfs_io -c "open -P $file" -c "statx" | sed -e s/://; }
    while read tag val
    do
	stx[$tag]=$val
    done <&${COPROC[0]}
    wait || exit $?

    # Compare them
    for key in ${!stx[*]}
    do
	if [ "${stx[$key]}" != "${st[$key]}" ]
	then
	    case $key in
		stat.mask|stat.attributes|stat.btime.tv_*)
		    # Not supported in stat
		    ;;
		*)
		    echo differ $key "${stx[$key]}" != "${st[$key]}"
		    ;;
	    esac
	fi
    done
}

cmpstat /

[PATCH] xfs_io: changes to statx interface [ver #4]

[David Howells]

Here's my fourth attempt at changing Eric's statx patch.  In this one I split
the type out from the mode field.

David
---
diff --git a/io/stat.c b/io/stat.c
index a7aebcd..410d33a 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -44,6 +44,36 @@ filesize(void)
 	return st.st_size;
 }
 
+static int
+dump_raw_stat(struct stat *st)
+{
+	if (fstat(file->fd, st) < 0) {
+		perror("fstat");
+		return -1;
+	}
+
+	printf("stat.blksize: %lu\n", st->st_blksize);
+	printf("stat.nlink: %lu\n", st->st_nlink);
+	printf("stat.uid: %u\n", st->st_uid);
+	printf("stat.gid: %u\n", st->st_gid);
+	printf("stat.type: 0%o\n", st->st_mode & ~07777);
+	printf("stat.mode: 0%o\n", st->st_mode & 07777);
+	printf("stat.ino: %lu\n", st->st_ino);
+	printf("stat.size: %lu\n", st->st_size);
+	printf("stat.blocks: %lu\n", st->st_blocks);
+	printf("stat.atime.tv_sec: %ld\n", st->st_atim.tv_sec);
+	printf("stat.atime.tv_nsec: %ld\n", st->st_atim.tv_nsec);
+	printf("stat.ctime.tv_sec: %ld\n", st->st_ctim.tv_sec);
+	printf("stat.ctime.tv_nsec: %ld\n", st->st_ctim.tv_nsec);
+	printf("stat.mtime.tv_sec: %ld\n", st->st_mtim.tv_sec);
+	printf("stat.mtime.tv_nsec: %ld\n", st->st_mtim.tv_nsec);
+	printf("stat.rdev_major: %u\n", major(st->st_rdev));
+	printf("stat.rdev_minor: %u\n", minor(st->st_rdev));
+	printf("stat.dev_major: %u\n", major(st->st_dev));
+	printf("stat.dev_minor: %u\n", minor(st->st_dev));
+	return 0;
+}
+
 static char *
 filetype(mode_t mode)
 {
@@ -74,7 +104,23 @@ stat_f(
 	struct dioattr	dio;
 	struct fsxattr	fsx, fsxa;
 	struct stat	st;
-	int		verbose = (argc == 2 && !strcmp(argv[1], "-v"));
+	int		c, verbose = 0, raw = 0;
+
+	while ((c = getopt(argc, argv, "rv")) != EOF) {
+		switch (c) {
+		case 'r':
+			raw = 1;
+			break;
+		case 'v':
+			verbose = 1;
+			break;
+		default:
+			return command_usage(&stat_cmd);
+		}
+	}
+
+	if (raw)
+		return dump_raw_stat(&st);
 
 	printf(_("fd.path = \"%s\"\n"), file->name);
 	printf(_("fd.flags = %s,%s,%s%s%s%s%s\n"),
@@ -189,12 +235,9 @@ statx_help(void)
 " Display extended file status.\n"
 "\n"
 " Options:\n"
-" -m mask -- Specify the field mask for the statx call (default STATX_ALL)\n"
-" -A -- Suppress terminal automount traversal\n"
+" -m mask -- Specify the field mask for the statx call (can also be 'basic' or 'all'; default STATX_ALL)\n"
 " -D -- Don't sync attributes with the server\n"
 " -F -- Force the attributes to be sync'd with the server\n"
-" -L -- Follow symlinks (statx link target)\n"
-" -O -- Add only basic stats (STATX_BASIC_STATS) to default mask\n"
 "\n"));
 }
 
@@ -202,28 +245,29 @@ statx_help(void)
 static void
 dump_statx(struct statx *stx)
 {
-	printf("stx_mask: 0x%x\n", stx->stx_mask);
-	printf("stx_blksize: %u\n", stx->stx_blksize);
-	printf("stx_attributes: 0x%llx\n", stx->stx_attributes);
-	printf("stx_nlink: %u\n", stx->stx_nlink);
-	printf("stx_uid: %u\n", stx->stx_uid);
-	printf("stx_gid: %u\n", stx->stx_gid);
-	printf("stx_mode: 0%o\n", stx->stx_mode);
-	printf("stx_ino: %llu\n", stx->stx_ino);
-	printf("stx_size: %llu\n", stx->stx_size);
-	printf("stx_blocks: %llu\n", stx->stx_blocks);
-	printf("stx_atime.tv_sec: %lld\n", stx->stx_atime.tv_sec);
-	printf("stx_atime.tv_nsec: %d\n", stx->stx_atime.tv_nsec);
-	printf("stx_btime.tv_sec: %lld\n", stx->stx_btime.tv_sec);
-	printf("stx_btime.tv_nsec: %d\n", stx->stx_btime.tv_nsec);
-	printf("stx_ctime.tv_sec: %lld\n", stx->stx_ctime.tv_sec);
-	printf("stx_ctime.tv_nsec: %d\n", stx->stx_ctime.tv_nsec);
-	printf("stx_mtime.tv_sec: %lld\n", stx->stx_mtime.tv_sec);
-	printf("stx_mtime.tv_nsec: %d\n", stx->stx_mtime.tv_nsec);
-	printf("stx_rdev_major: %u\n", stx->stx_rdev_major);
-	printf("stx_rdev_minor: %u\n", stx->stx_rdev_minor);
-	printf("stx_dev_major: %u\n", stx->stx_dev_major);
-	printf("stx_dev_minor: %u\n", stx->stx_dev_minor);
+	printf("stat.mask: 0x%x\n", stx->stx_mask);
+	printf("stat.blksize: %u\n", stx->stx_blksize);
+	printf("stat.attributes: 0x%llx\n", stx->stx_attributes);
+	printf("stat.nlink: %u\n", stx->stx_nlink);
+	printf("stat.uid: %u\n", stx->stx_uid);
+	printf("stat.gid: %u\n", stx->stx_gid);
+	printf("stat.type: 0%o\n", stx->stx_mode & ~07777);
+	printf("stat.mode: 0%o\n", stx->stx_mode & 07777);
+	printf("stat.ino: %llu\n", stx->stx_ino);
+	printf("stat.size: %llu\n", stx->stx_size);
+	printf("stat.blocks: %llu\n", stx->stx_blocks);
+	printf("stat.atime.tv_sec: %lld\n", stx->stx_atime.tv_sec);
+	printf("stat.atime.tv_nsec: %d\n", stx->stx_atime.tv_nsec);
+	printf("stat.btime.tv_sec: %lld\n", stx->stx_btime.tv_sec);
+	printf("stat.btime.tv_nsec: %d\n", stx->stx_btime.tv_nsec);
+	printf("stat.ctime.tv_sec: %lld\n", stx->stx_ctime.tv_sec);
+	printf("stat.ctime.tv_nsec: %d\n", stx->stx_ctime.tv_nsec);
+	printf("stat.mtime.tv_sec: %lld\n", stx->stx_mtime.tv_sec);
+	printf("stat.mtime.tv_nsec: %d\n", stx->stx_mtime.tv_nsec);
+	printf("stat.rdev_major: %u\n", stx->stx_rdev_major);
+	printf("stat.rdev_minor: %u\n", stx->stx_rdev_minor);
+	printf("stat.dev_major: %u\n", stx->stx_dev_major);
+	printf("stat.dev_minor: %u\n", stx->stx_dev_minor);
 }
 
 /*
@@ -239,16 +283,18 @@ statx_f(
 {
 	int		c;
 	struct statx	stx;
-	int		atflag = AT_SYMLINK_NOFOLLOW;
-	unsigned int	m_mask = 0;	/* mask requested with -m */
-	int		Oflag = 0, mflag = 0;	/* -O or -m was used */
+	int		atflag = 0;
 	unsigned int	mask = STATX_ALL;
 
-	while ((c = getopt(argc, argv, "m:FDLOA")) != EOF) {
+	while ((c = getopt(argc, argv, "m:FD")) != EOF) {
 		switch (c) {
 		case 'm':
-			m_mask = atoi(optarg);
-			mflag = 1;
+			if (strcmp(optarg, "basic") == 0)
+				mask = STATX_BASIC_STATS;
+			else if (strcmp(optarg, "all") == 0)
+				mask = STATX_ALL;
+			else
+				mask = strtoul(optarg, NULL, 0);
 			break;
 		case 'F':
 			atflag &= ~AT_STATX_SYNC_TYPE;
@@ -258,38 +304,19 @@ statx_f(
 			atflag &= ~AT_STATX_SYNC_TYPE;
 			atflag |= AT_STATX_DONT_SYNC;
 			break;
-		case 'L':
-			atflag &= ~AT_SYMLINK_NOFOLLOW;
-			break;
-		case 'O':
-			mask = STATX_BASIC_STATS;
-			Oflag = 1;
-			break;
-		case 'A':
-			atflag |= AT_NO_AUTOMOUNT;
-			break;
 		default:
 			return command_usage(&statx_cmd);
 		}
 	}
 
-	if (Oflag && mflag) {
-		printf("Cannot specify both -m mask and -O\n");
-		return 0;
-	}
-
-	/* -m overrides any other mask options */
-	if (mflag)
-		mask = m_mask;
-
 	memset(&stx, 0xbf, sizeof(stx));
-	if (statx(AT_FDCWD, file->name, atflag, mask, &stx) < 0) {
+
+	if (statx(file->fd, NULL, atflag, mask, &stx) < 0) {
 		perror("statx");
 		return 0;
 	}
-
+	
 	dump_statx(&stx);
-
 	return 0;
 }
 
@@ -301,7 +328,7 @@ stat_init(void)
 	stat_cmd.argmin = 0;
 	stat_cmd.argmax = 1;
 	stat_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	stat_cmd.args = _("[-v]");
+	stat_cmd.args = _("[-rv]");
 	stat_cmd.oneline = _("information about the currently open file");
 
 	statfs_cmd.name = "statfs";
@@ -315,7 +342,7 @@ stat_init(void)
 	statx_cmd.argmin = 0;
 	statx_cmd.argmax = -1;
 	statx_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	statx_cmd.args = _("[-O | -m mask][-FDLAP]");
+	statx_cmd.args = _("[-m basic | -m all | -m <mask>][-FD]");
 	statx_cmd.oneline =
  _("extended information about the currently open file");
 	statx_cmd.help = statx_help;
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index e77be40..416f718 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -882,24 +882,29 @@ Only available in expert mode and requires privileges.
 Force the filesystem to shutdown (with or without flushing the log).
 Only available in expert mode and requires privileges.
 .TP
-.BR stat " [ " \-v " ]"
+.BR stat " [ \-" rv " ]"
 Selected statistics from
 .BR stat (2)
 and the XFS_IOC_GETXATTR system call on the current file. If the
 .B \-v
 option is specified, the atime (last access), mtime
-(last modify), and ctime (last change) timestamps are also displayed.
+(last modify), and ctime (last change) timestamps are also displayed. If the
+.B \-r
+option is specified, the raw output will be dumped in the same form as the
+output for the statx command, but with some fields missing.
 .TP
-.BR statx " [ " \-O " | " "\-m mask" " ][ \-" FDLA " ]"
+.BR statx " [ " "\-m mask" " ][ \-" FD " ]"
 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
+Specify the field mask for the statx call as an decimal, hex or octal integer
+or
+.RI \" basic "\" or \"" all \"
+to specify the basic stats that
+.IR stat ()
+returns or all the stats known by the header file.  All is the default.
 .TP
 .B \-F
 Force the attributes to be sync'd with the server
@@ -907,12 +912,6 @@ Force the attributes to be sync'd with the server
 .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