[PATCH v2 0/6] xfs_io: fix up command iteration

[PATCH v2 0/6] xfs_io: fix up command iteration

[Dave Chinner]

Hi folks,

V2 of the xfs_io command iteration cleanup patch first posted here:

https://www.spinics.net/lists/linux-xfs/msg02696.html

No major code changes in this version - I've improved the man page
update documenting the iteration behaviour and cleaned up the typos
pointed out, but otherwise it is unchanged.

-Dave.

[PATCH 1/6] libxcmd: check CMD_FLAG_GLOBAL inside args_command()

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Rather than having multiple methods of executing commands from the
CLI, use CMD_FLAG_GLOBAL to indicate a one-shot command rather than
an iterative command from args_command(). This simplifies the main
loop processing.

To make it more obvious what this CMD_FLAG_GLOBAL flag does, rename
it to CMD_FLAG_ONESHOT to indicate that the command should only ever
be executed once and not iterated.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
 include/command.h |  7 ++++++-
 io/file.c         |  2 +-
 io/init.c         |  2 +-
 libxcmd/command.c | 20 +++++++++++++-------
 libxcmd/help.c    |  2 +-
 libxcmd/quit.c    |  2 +-
 quota/path.c      |  4 ++--
 quota/report.c    |  2 +-
 8 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/include/command.h b/include/command.h
index 81d5a4dbb7f3..58bfcaac44a0 100644
--- a/include/command.h
+++ b/include/command.h
@@ -20,7 +20,12 @@
 
 #include <sys/time.h>
 
-#define CMD_FLAG_GLOBAL		(1<<31)	/* don't iterate "args" */
+/*
+ * A "oneshot" command ony runs once per command execution. It does
+ * not iterate the command args function callout and so can be used
+ * for functions like "help" that should only ever be run once.
+ */
+#define CMD_FLAG_ONESHOT	(1<<31)
 #define CMD_FLAG_FOREIGN_OK	(1<<30)	/* command not restricted to XFS */
 
 typedef int (*cfunc_t)(int argc, char **argv);
diff --git a/io/file.c b/io/file.c
index d4bc4f8fc1d5..8e3f07122922 100644
--- a/io/file.c
+++ b/io/file.c
@@ -104,7 +104,7 @@ file_init(void)
 	print_cmd.argmin = 0;
 	print_cmd.argmax = 0;
 	print_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK |
-				CMD_FLAG_GLOBAL;
+				CMD_FLAG_ONESHOT;
 	print_cmd.oneline = _("list current open files and memory mappings");
 
 	add_command(&file_cmd);
diff --git a/io/init.c b/io/init.c
index a9191cfa072d..ab40f3745390 100644
--- a/io/init.c
+++ b/io/init.c
@@ -104,7 +104,7 @@ static int
 init_check_command(
 	const cmdinfo_t	*ct)
 {
-	if (ct->flags & CMD_FLAG_GLOBAL)
+	if (ct->flags & CMD_FLAG_ONESHOT)
 		return 1;
 
 	if (!file && !(ct->flags & CMD_NOFILE_OK)) {
diff --git a/libxcmd/command.c b/libxcmd/command.c
index dd0034cc6d83..dce8361ce3ea 100644
--- a/libxcmd/command.c
+++ b/libxcmd/command.c
@@ -124,10 +124,20 @@ add_user_command(char *optarg)
 	cmdline[ncmdline-1] = optarg;
 }
 
+/*
+ * To detect one-shot commands, they will return a negative index. If we
+ * get a negative index on entry, we've already run the one-shot command,
+ * so we abort straight away.
+ */
 static int
 args_command(
-	int	index)
+	const cmdinfo_t	*ct,
+	int		index)
 {
+	if (index < 0)
+		return 0;
+	if (ct->flags & CMD_FLAG_ONESHOT)
+		return -1;
 	if (args_func)
 		return args_func(index);
 	return 0;
@@ -160,13 +170,9 @@ command_loop(void)
 		if (c) {
 			ct = find_command(v[0]);
 			if (ct) {
-				if (ct->flags & CMD_FLAG_GLOBAL)
+				j = 0;
+				while (!done && (j = args_command(ct, j)))
 					done = command(ct, c, v);
-				else {
-					j = 0;
-					while (!done && (j = args_command(j)))
-						done = command(ct, c, v);
-				}
 			} else
 				fprintf(stderr, _("command \"%s\" not found\n"),
 					v[0]);
diff --git a/libxcmd/help.c b/libxcmd/help.c
index 8894c7931f89..bc31d6df1d8a 100644
--- a/libxcmd/help.c
+++ b/libxcmd/help.c
@@ -89,7 +89,7 @@ help_init(void)
 	help_cmd.cfunc = help_f;
 	help_cmd.argmin = 0;
 	help_cmd.argmax = 1;
-	help_cmd.flags = CMD_FLAG_GLOBAL | CMD_ALL_FSTYPES;
+	help_cmd.flags = CMD_FLAG_ONESHOT | CMD_ALL_FSTYPES;
 	help_cmd.args = _("[command]");
 	help_cmd.oneline = _("help for one or all commands");
 
diff --git a/libxcmd/quit.c b/libxcmd/quit.c
index e0af91629b81..19431015aee2 100644
--- a/libxcmd/quit.c
+++ b/libxcmd/quit.c
@@ -39,7 +39,7 @@ quit_init(void)
 	quit_cmd.cfunc = quit_f;
 	quit_cmd.argmin = -1;
 	quit_cmd.argmax = -1;
-	quit_cmd.flags = CMD_FLAG_GLOBAL | CMD_ALL_FSTYPES;
+	quit_cmd.flags = CMD_FLAG_ONESHOT | CMD_ALL_FSTYPES;
 	quit_cmd.oneline = _("exit the program");
 
 	add_command(&quit_cmd);
diff --git a/quota/path.c b/quota/path.c
index 57d14f0b5511..330a3bef6aa9 100644
--- a/quota/path.c
+++ b/quota/path.c
@@ -141,7 +141,7 @@ path_init(void)
 	path_cmd.cfunc = path_f;
 	path_cmd.argmin = 0;
 	path_cmd.argmax = 1;
-	path_cmd.flags = CMD_FLAG_GLOBAL | CMD_FLAG_FOREIGN_OK;
+	path_cmd.flags = CMD_FLAG_ONESHOT | CMD_FLAG_FOREIGN_OK;
 	path_cmd.oneline = _("set current path, or show the list of paths");
 
 	print_cmd.name = "print";
@@ -149,7 +149,7 @@ path_init(void)
 	print_cmd.cfunc = print_f;
 	print_cmd.argmin = 0;
 	print_cmd.argmax = 0;
-	print_cmd.flags = CMD_FLAG_GLOBAL | CMD_FLAG_FOREIGN_OK;
+	print_cmd.flags = CMD_FLAG_ONESHOT | CMD_FLAG_FOREIGN_OK;
 	print_cmd.oneline = _("list known mount points and projects");
 
 	if (expert)
diff --git a/quota/report.c b/quota/report.c
index 604f50dc6001..ca9d2b2c9564 100644
--- a/quota/report.c
+++ b/quota/report.c
@@ -770,7 +770,7 @@ report_init(void)
 	report_cmd.args = _("[-bir] [-gpu] [-ahnt] [-f file]");
 	report_cmd.oneline = _("report filesystem quota information");
 	report_cmd.help = report_help;
-	report_cmd.flags = CMD_FLAG_GLOBAL | CMD_FLAG_FOREIGN_OK;
+	report_cmd.flags = CMD_FLAG_ONESHOT | CMD_FLAG_FOREIGN_OK;
 
 	if (expert) {
 		add_command(&dump_cmd);
-- 
2.10.2

[PATCH 2/6] libxcmd: rename args_command to command_iterator

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

It is not particularly easy to understand the function of the
args_command abstraction. it's actually a command iterator interface
that allows callers to specify the target of the command and iterate
the command multiple times over different targets. Rename and
document the abstraction to make this functionality clear.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
 include/command.h |  4 ++--
 io/init.c         |  9 +++++++--
 libxcmd/command.c | 16 ++++++++--------
 quota/init.c      |  9 +++++++--
 4 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/include/command.h b/include/command.h
index 58bfcaac44a0..637ee06e6e9a 100644
--- a/include/command.h
+++ b/include/command.h
@@ -50,12 +50,12 @@ extern int		ncmds;
 extern void		help_init(void);
 extern void		quit_init(void);
 
-typedef int (*argsfunc_t)(int index);
+typedef int (*iterfunc_t)(int index);
 typedef int (*checkfunc_t)(const cmdinfo_t *ci);
 
 extern void		add_command(const cmdinfo_t *ci);
 extern void		add_user_command(char *optarg);
-extern void		add_args_command(argsfunc_t af);
+extern void		add_command_iterator(iterfunc_t func);
 extern void		add_check_command(checkfunc_t cf);
 
 extern const cmdinfo_t	*find_command(const char *cmd);
diff --git a/io/init.c b/io/init.c
index ab40f3745390..34009b024833 100644
--- a/io/init.c
+++ b/io/init.c
@@ -90,8 +90,13 @@ init_commands(void)
 	cowextsize_init();
 }
 
+/*
+ * This allows xfs_io commands specified on the command line to be run on every
+ * open file in the file table. Commands that should not be iterated across all
+ * open files need to specify CMD_FLAG_ONESHOT in their command flags.
+ */
 static int
-init_args_command(
+filetable_iterator(
 	int	index)
 {
 	if (index >= filecount)
@@ -214,7 +219,7 @@ init(
 	}
 
 	init_commands();
-	add_args_command(init_args_command);
+	add_command_iterator(filetable_iterator);
 	add_check_command(init_check_command);
 }
 
diff --git a/libxcmd/command.c b/libxcmd/command.c
index dce8361ce3ea..789aeb5c5e5a 100644
--- a/libxcmd/command.c
+++ b/libxcmd/command.c
@@ -23,7 +23,7 @@
 cmdinfo_t	*cmdtab;
 int		ncmds;
 
-static argsfunc_t	args_func;
+static iterfunc_t	iter_func;
 static checkfunc_t	check_func;
 static int		ncmdline;
 static char		**cmdline;
@@ -130,7 +130,7 @@ add_user_command(char *optarg)
  * so we abort straight away.
  */
 static int
-args_command(
+iterate_command(
 	const cmdinfo_t	*ct,
 	int		index)
 {
@@ -138,16 +138,16 @@ args_command(
 		return 0;
 	if (ct->flags & CMD_FLAG_ONESHOT)
 		return -1;
-	if (args_func)
-		return args_func(index);
+	if (iter_func)
+		return iter_func(index);
 	return 0;
 }
 
 void
-add_args_command(
-	argsfunc_t	af)
+add_command_iterator(
+	iterfunc_t	func)
 {
-	args_func = af;
+	iter_func = func;
 }
 
 void
@@ -171,7 +171,7 @@ command_loop(void)
 			ct = find_command(v[0]);
 			if (ct) {
 				j = 0;
-				while (!done && (j = args_command(ct, j)))
+				while (!done && (j = iterate_command(ct, j)))
 					done = command(ct, c, v);
 			} else
 				fprintf(stderr, _("command \"%s\" not found\n"),
diff --git a/quota/init.c b/quota/init.c
index 3bebbb8735f3..193f6421fd59 100644
--- a/quota/init.c
+++ b/quota/init.c
@@ -75,8 +75,13 @@ init_commands(void)
 	state_init();
 }
 
+/*
+ * This function allows xfs_quota commands to iterate across all discovered
+ * quota enabled filesystems. Commands that should not iterate all filesystems
+ * should specify CMD_FLAG_ONESHOT in their command flags.
+ */
 static int
-init_args_command(
+filesystem_iterator(
 	int	index)
 {
 	if (index >= fs_count)
@@ -189,7 +194,7 @@ init(
 	free(projopts);
 
 	init_commands();
-	add_args_command(init_args_command);
+	add_command_iterator(filesystem_iterator);
 	add_check_command(init_check_command);
 
 	/*
-- 
2.10.2

[PATCH 3/6] libxcmd: merge command() and iterate_command()

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Simplify the command loop further by merging the command loop
iteration checks with the command execution function. This removes
all visibility of command iteration from the main command execution
loop, and enables us to factor and clean up the command loop
processing neatly.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
 libxcmd/command.c | 110 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 66 insertions(+), 44 deletions(-)

diff --git a/libxcmd/command.c b/libxcmd/command.c
index 789aeb5c5e5a..2c94a3199115 100644
--- a/libxcmd/command.c
+++ b/libxcmd/command.c
@@ -125,22 +125,30 @@ add_user_command(char *optarg)
 }
 
 /*
- * To detect one-shot commands, they will return a negative index. If we
- * get a negative index on entry, we've already run the one-shot command,
- * so we abort straight away.
+ * Run a command, iterating as necessary. Return 0 for success, non-zero
+ * if an error occurred. Errors terminate loop iteration immediately.
  */
 static int
 iterate_command(
 	const cmdinfo_t	*ct,
-	int		index)
+	int		argc,
+	char		**argv)
 {
-	if (index < 0)
+	int		error = 0;
+	int		j;
+
+	/* if there's nothing to iterate, we're done! */
+	if (!iter_func)
 		return 0;
-	if (ct->flags & CMD_FLAG_ONESHOT)
-		return -1;
-	if (iter_func)
-		return iter_func(index);
-	return 0;
+
+	for (j = iter_func(0); j; j = iter_func(j)) {
+		error = command(ct, argc, argv);
+		if (error)
+			break;
+
+	}
+
+	return error;
 }
 
 void
@@ -150,14 +158,55 @@ add_command_iterator(
 	iter_func = func;
 }
 
-void
-command_loop(void)
+static int
+process_input(
+	char	*input,
+	bool	iterate)
 {
-	int		c, i, j = 0, done = 0;
-	char		*input;
 	char		**v;
 	const cmdinfo_t	*ct;
+	int		c = 0;
+	int		error = 0;
+
+	v = breakline(input, &c);
+	if (!c)
+		goto out;
+
+	ct = find_command(v[0]);
+	if (!ct) {
+		fprintf(stderr, _("command \"%s\" not found\n"), v[0]);
+		goto out;
+	}
+
+	/* oneshot commands don't iterate */
+	if (!iterate || (ct->flags & CMD_FLAG_ONESHOT))
+		error = command(ct, c, v);
+	else
+		error = iterate_command(ct, c, v);
+out:
+	doneline(input, v);
+	return error;
+}
+
+void
+command_loop(void)
+{
+	char	*input;
+	int	done = 0;
+	int	i;
+
+	if (!cmdline) {
+		/* interactive mode */
+		while (!done) {
+			input = fetchline();
+			if (!input)
+				break;
+			done = process_input(input, false);
+		}
+		return;
+	}
 
+	/* command line mode */
 	for (i = 0; !done && i < ncmdline; i++) {
 		input = strdup(cmdline[i]);
 		if (!input) {
@@ -166,37 +215,10 @@ command_loop(void)
 				cmdline[i], strerror(errno));
 			exit(1);
 		}
-		v = breakline(input, &c);
-		if (c) {
-			ct = find_command(v[0]);
-			if (ct) {
-				j = 0;
-				while (!done && (j = iterate_command(ct, j)))
-					done = command(ct, c, v);
-			} else
-				fprintf(stderr, _("command \"%s\" not found\n"),
-					v[0]);
-		}
-		doneline(input, v);
-	}
-	if (cmdline) {
-		free(cmdline);
-		return;
-	}
-	while (!done) {
-		if ((input = fetchline()) == NULL)
-			break;
-		v = breakline(input, &c);
-		if (c) {
-			ct = find_command(v[0]);
-			if (ct)
-				done = command(ct, c, v);
-			else
-				fprintf(stderr, _("command \"%s\" not found\n"),
-					v[0]);
-		}
-		doneline(input, v);
+		done = process_input(input, true);
 	}
+	free(cmdline);
+	return;
 }
 
 void
-- 
2.10.2

[PATCH 4/6] libxcmd: don't check generic library commands

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

The generic "help" and "quit" commands have different methods of
skipping user provided command check functions that may prevent them
from running. xfs_quota use CMD_ALL_FSTYPES and xfs_io uses
CMD_FLAG_ONESHOT.  Add a new CMD_FLAG_LIBRARY to indicate commands
that should not be checked against application specific check
functions so they are always present and can be run regardless of
the context in which they are run.

This gets rid of the CMD_ALL_FSTYPES flag, and enables us to remove
the ONESHOT check in xfs_io so we use only app specific flags for
determining if app commands should run or not.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
 include/command.h | 1 +
 io/init.c         | 3 ---
 libxcmd/command.c | 4 ++++
 libxcmd/help.c    | 2 +-
 libxcmd/quit.c    | 2 +-
 quota/init.c      | 4 ----
 6 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/include/command.h b/include/command.h
index 637ee06e6e9a..348002cbe3ed 100644
--- a/include/command.h
+++ b/include/command.h
@@ -27,6 +27,7 @@
  */
 #define CMD_FLAG_ONESHOT	(1<<31)
 #define CMD_FLAG_FOREIGN_OK	(1<<30)	/* command not restricted to XFS */
+#define CMD_FLAG_LIBRARY	(1<<29)	/* command provided by libxcmd */
 
 typedef int (*cfunc_t)(int argc, char **argv);
 typedef void (*helpfunc_t)(void);
diff --git a/io/init.c b/io/init.c
index 34009b024833..5ce627ef22c9 100644
--- a/io/init.c
+++ b/io/init.c
@@ -109,9 +109,6 @@ static int
 init_check_command(
 	const cmdinfo_t	*ct)
 {
-	if (ct->flags & CMD_FLAG_ONESHOT)
-		return 1;
-
 	if (!file && !(ct->flags & CMD_NOFILE_OK)) {
 		fprintf(stderr, _("no files are open, try 'help open'\n"));
 		return 0;
diff --git a/libxcmd/command.c b/libxcmd/command.c
index 2c94a3199115..decc442a9d03 100644
--- a/libxcmd/command.c
+++ b/libxcmd/command.c
@@ -48,6 +48,10 @@ static int
 check_command(
 	const cmdinfo_t	*ci)
 {
+	/* always run internal library supplied commands */
+	if (ci->flags & CMD_FLAG_LIBRARY)
+		return 1;
+
 	if (check_func)
 		return check_func(ci);
 	return 1;
diff --git a/libxcmd/help.c b/libxcmd/help.c
index bc31d6df1d8a..f888377cabf7 100644
--- a/libxcmd/help.c
+++ b/libxcmd/help.c
@@ -89,7 +89,7 @@ help_init(void)
 	help_cmd.cfunc = help_f;
 	help_cmd.argmin = 0;
 	help_cmd.argmax = 1;
-	help_cmd.flags = CMD_FLAG_ONESHOT | CMD_ALL_FSTYPES;
+	help_cmd.flags = CMD_FLAG_ONESHOT | CMD_FLAG_LIBRARY;
 	help_cmd.args = _("[command]");
 	help_cmd.oneline = _("help for one or all commands");
 
diff --git a/libxcmd/quit.c b/libxcmd/quit.c
index 19431015aee2..1e1ba986f2ba 100644
--- a/libxcmd/quit.c
+++ b/libxcmd/quit.c
@@ -39,7 +39,7 @@ quit_init(void)
 	quit_cmd.cfunc = quit_f;
 	quit_cmd.argmin = -1;
 	quit_cmd.argmax = -1;
-	quit_cmd.flags = CMD_FLAG_ONESHOT | CMD_ALL_FSTYPES;
+	quit_cmd.flags = CMD_FLAG_ONESHOT | CMD_FLAG_LIBRARY;
 	quit_cmd.oneline = _("exit the program");
 
 	add_command(&quit_cmd);
diff --git a/quota/init.c b/quota/init.c
index 193f6421fd59..d45dc4c5461e 100644
--- a/quota/init.c
+++ b/quota/init.c
@@ -117,10 +117,6 @@ init_check_command(
 	if (!fs_path)
 		return 1;
 
-	/* Always run commands that are valid for all fs types. */
-	if (ct->flags & CMD_ALL_FSTYPES)
-		return 1;
-
 	/* If it's an XFS filesystem, always run the command. */
 	if (!(fs_path->fs_flags & FS_FOREIGN))
 		return 1;
-- 
2.10.2

[PATCH 5/6] xfs_io: make various commands one-shot only

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

It makes no sense to iterate the file table for some xfs_io
commands. Some commands are already marked in this way, but lots of
them are not and this leads to bad behaviour. For example, the open
command will run until the process fd table is full and EMFILE is
returned rather than just opening the specified file once.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
 io/file.c      | 2 +-
 io/freeze.c    | 4 ++--
 io/getrusage.c | 3 ++-
 io/imap.c      | 2 +-
 io/inject.c    | 2 +-
 io/link.c      | 2 +-
 io/mmap.c      | 3 ++-
 io/open.c      | 7 ++++---
 io/reflink.c   | 4 ++--
 io/resblks.c   | 2 +-
 io/shutdown.c  | 2 +-
 io/sync.c      | 3 ++-
 12 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/io/file.c b/io/file.c
index 8e3f07122922..349b19cdc420 100644
--- a/io/file.c
+++ b/io/file.c
@@ -95,7 +95,7 @@ file_init(void)
 	file_cmd.cfunc = file_f;
 	file_cmd.argmin = 0;
 	file_cmd.argmax = 1;
-	file_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	file_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	file_cmd.oneline = _("set the current file");
 
 	print_cmd.name = "print";
diff --git a/io/freeze.c b/io/freeze.c
index 3d0d2a4b5601..0305713d99e8 100644
--- a/io/freeze.c
+++ b/io/freeze.c
@@ -65,14 +65,14 @@ freeze_init(void)
 	freeze_cmd.cfunc = freeze_f;
 	freeze_cmd.argmin = 0;
 	freeze_cmd.argmax = 0;
-	freeze_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	freeze_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	freeze_cmd.oneline = _("freeze filesystem of current file");
 
 	thaw_cmd.name = "thaw";
 	thaw_cmd.cfunc = thaw_f;
 	thaw_cmd.argmin = 0;
 	thaw_cmd.argmax = 0;
-	thaw_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	thaw_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	thaw_cmd.oneline = _("unfreeze filesystem of current file");
 
 	if (expert) {
diff --git a/io/getrusage.c b/io/getrusage.c
index bccf94cbc302..cf1f2afd19a8 100644
--- a/io/getrusage.c
+++ b/io/getrusage.c
@@ -113,7 +113,8 @@ getrusage_init(void)
 	getrusage_cmd.argmin = 0;
 	getrusage_cmd.argmax = -1;
 	getrusage_cmd.cfunc = getrusage_f;
-	getrusage_cmd.flags = CMD_NOFILE_OK | CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	getrusage_cmd.flags = CMD_NOFILE_OK | CMD_NOMAP_OK |
+			      CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	getrusage_cmd.oneline = _("report process resource usage");
 
 	if (expert)
diff --git a/io/imap.c b/io/imap.c
index 7123432f411f..f52238e0c450 100644
--- a/io/imap.c
+++ b/io/imap.c
@@ -72,7 +72,7 @@ imap_init(void)
 	imap_cmd.argmin = 0;
 	imap_cmd.argmax = 1;
 	imap_cmd.args = _("[nentries]");
-	imap_cmd.flags = CMD_NOMAP_OK;
+	imap_cmd.flags = CMD_NOMAP_OK | CMD_FLAG_ONESHOT;
 	imap_cmd.oneline = _("inode map for filesystem of current file");
 
 	if (expert)
diff --git a/io/inject.c b/io/inject.c
index 5d5e4aef3dfc..25c70218a1ef 100644
--- a/io/inject.c
+++ b/io/inject.c
@@ -163,7 +163,7 @@ inject_init(void)
 	inject_cmd.cfunc = inject_f;
 	inject_cmd.argmin = 0;
 	inject_cmd.argmax = -1;
-	inject_cmd.flags = CMD_NOMAP_OK;
+	inject_cmd.flags = CMD_NOMAP_OK | CMD_FLAG_ONESHOT;
 	inject_cmd.args = _("[tag ...]");
 	inject_cmd.oneline = _("inject errors into a filesystem");
 	inject_cmd.help = inject_help;
diff --git a/io/link.c b/io/link.c
index ccf8e691bb1d..9b2e8a970942 100644
--- a/io/link.c
+++ b/io/link.c
@@ -59,7 +59,7 @@ flink_init(void)
 	flink_cmd.cfunc = flink_f;
 	flink_cmd.argmin = 1;
 	flink_cmd.argmax = 1;
-	flink_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	flink_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	flink_cmd.args = _("filename");
 	flink_cmd.oneline =
 		_("link the open file descriptor to the supplied filename");
diff --git a/io/mmap.c b/io/mmap.c
index dc188d0557cf..e2d8d5a92326 100644
--- a/io/mmap.c
+++ b/io/mmap.c
@@ -674,7 +674,8 @@ mmap_init(void)
 	mmap_cmd.cfunc = mmap_f;
 	mmap_cmd.argmin = 0;
 	mmap_cmd.argmax = -1;
-	mmap_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK;
+	mmap_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK |
+			 CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	mmap_cmd.args = _("[N] | [-rwx] [-s size] [off len]");
 	mmap_cmd.oneline =
 		_("mmap a range in the current file, show mappings");
diff --git a/io/open.c b/io/open.c
index 722d0f9eacf5..a12f4a2ba528 100644
--- a/io/open.c
+++ b/io/open.c
@@ -918,7 +918,8 @@ open_init(void)
 	open_cmd.cfunc = open_f;
 	open_cmd.argmin = 0;
 	open_cmd.argmax = -1;
-	open_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK;
+	open_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK |
+			 CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	open_cmd.args = _("[-acdrstxT] [-m mode] [path]");
 	open_cmd.oneline = _("open the file specified by path");
 	open_cmd.help = open_help;
@@ -936,7 +937,7 @@ open_init(void)
 	close_cmd.cfunc = close_f;
 	close_cmd.argmin = 0;
 	close_cmd.argmax = 0;
-	close_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	close_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	close_cmd.oneline = _("close the current open file");
 
 	statfs_cmd.name = "statfs";
@@ -980,7 +981,7 @@ open_init(void)
 	inode_cmd.args = _("[-nv] [num]");
 	inode_cmd.argmin = 0;
 	inode_cmd.argmax = 3;
-	inode_cmd.flags = CMD_NOMAP_OK;
+	inode_cmd.flags = CMD_NOMAP_OK | CMD_FLAG_ONESHOT;
 	inode_cmd.oneline =
 		_("Query inode number usage in the filesystem");
 	inode_cmd.help = inode_help;
diff --git a/io/reflink.c b/io/reflink.c
index a09e82dca80a..a22b6b4a07e3 100644
--- a/io/reflink.c
+++ b/io/reflink.c
@@ -304,7 +304,7 @@ reflink_init(void)
 	reflink_cmd.cfunc = reflink_f;
 	reflink_cmd.argmin = 4;
 	reflink_cmd.argmax = -1;
-	reflink_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	reflink_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	reflink_cmd.args =
 _("infile src_off dst_off len");
 	reflink_cmd.oneline =
@@ -318,7 +318,7 @@ _("infile src_off dst_off len");
 	dedupe_cmd.cfunc = dedupe_f;
 	dedupe_cmd.argmin = 4;
 	dedupe_cmd.argmax = -1;
-	dedupe_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	dedupe_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	dedupe_cmd.args =
 _("infile src_off dst_off len");
 	dedupe_cmd.oneline =
diff --git a/io/resblks.c b/io/resblks.c
index 73318ae03fd2..06903f5bb748 100644
--- a/io/resblks.c
+++ b/io/resblks.c
@@ -61,7 +61,7 @@ resblks_init(void)
 	resblks_cmd.cfunc = resblks_f;
 	resblks_cmd.argmin = 0;
 	resblks_cmd.argmax = 1;
-	resblks_cmd.flags = CMD_NOMAP_OK;
+	resblks_cmd.flags = CMD_NOMAP_OK | CMD_FLAG_ONESHOT;
 	resblks_cmd.args = _("[blocks]");
 	resblks_cmd.oneline =
 		_("get and/or set count of reserved filesystem blocks");
diff --git a/io/shutdown.c b/io/shutdown.c
index d8507cc78af7..d9cd520d11e2 100644
--- a/io/shutdown.c
+++ b/io/shutdown.c
@@ -54,7 +54,7 @@ shutdown_init(void)
 	shutdown_cmd.cfunc = shutdown_f;
 	shutdown_cmd.argmin = 0;
 	shutdown_cmd.argmax = 1;
-	shutdown_cmd.flags = CMD_NOMAP_OK;
+	shutdown_cmd.flags = CMD_NOMAP_OK | CMD_FLAG_ONESHOT;
 	shutdown_cmd.args = _("[-f]");
 	shutdown_cmd.oneline =
 		_("shuts down the filesystem where the current file resides");
diff --git a/io/sync.c b/io/sync.c
index 28e3a15e0a96..c77263804a35 100644
--- a/io/sync.c
+++ b/io/sync.c
@@ -52,7 +52,8 @@ sync_init(void)
 {
 	sync_cmd.name = "sync";
 	sync_cmd.cfunc = sync_f;
-	sync_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK | CMD_FOREIGN_OK;
+	sync_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK |
+			 CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
 	sync_cmd.oneline =
 		_("calls sync(2) to flush all in-core filesystem state to disk");
 
-- 
2.10.2

[PATCH 6/6] libxcmd: add non-iterating user commands

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Right now command iteration is not directly controllable by the
user; it is controlled entirely by the application command flag
setup. Sometimes we don't want commands to iterate but only operate
on the currently selected object.

For example, the stat command iterates:

$ xfs_io -c "open -r foo" -c "open bar" -c "file" -c "stat" foo
 000  foo            (foreign,non-sync,non-direct,read-write)
 001  foo            (foreign,non-sync,non-direct,read-only)
[002] bar            (foreign,non-sync,non-direct,read-write)
fd.path = "foo"
fd.flags = non-sync,non-direct,read-write
stat.ino = 462399
stat.type = regular file
stat.size = 776508
stat.blocks = 1528
fd.path = "foo"
fd.flags = non-sync,non-direct,read-only
stat.ino = 462399
stat.type = regular file
stat.size = 776508
stat.blocks = 1528
fd.path = "bar"
fd.flags = non-sync,non-direct,read-write
stat.ino = 475227
stat.type = regular file
stat.size = 0
stat.blocks = 0
$

To do this, add a function to supply a "non-iterating" user command
that will execute an iterating-capable command as though it
CMD_FLAG_ONESHOT was set. Add a new command line option to xfs_io to
drive it (-C <command>) and connect it all up. Document it in the
xfs_io man page, too.

The result of "-C stat":

$ xfs_io -c "open -r foo" -c "open bar" -c "file" -C "stat" foo
 000  foo            (foreign,non-sync,non-direct,read-write)
 001  foo            (foreign,non-sync,non-direct,read-only)
[002] bar            (foreign,non-sync,non-direct,read-write)
fd.path = "bar"
fd.flags = non-sync,non-direct,read-write
stat.ino = 475227
stat.type = regular file
stat.size = 0
stat.blocks = 0
$

Is that we only see the stat output for the active open file
which is "bar".

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
 include/command.h |  1 +
 io/init.c         |  7 +++++--
 libxcmd/command.c | 33 +++++++++++++++++++++++++++------
 man/man8/xfs_io.8 | 40 ++++++++++++++++++++++++++++++++++------
 4 files changed, 67 insertions(+), 14 deletions(-)

diff --git a/include/command.h b/include/command.h
index 348002cbe3ed..fb3f5c79b991 100644
--- a/include/command.h
+++ b/include/command.h
@@ -56,6 +56,7 @@ typedef int (*checkfunc_t)(const cmdinfo_t *ci);
 
 extern void		add_command(const cmdinfo_t *ci);
 extern void		add_user_command(char *optarg);
+extern void		add_oneshot_user_command(char *optarg);
 extern void		add_command_iterator(iterfunc_t func);
 extern void		add_check_command(checkfunc_t cf);
 
diff --git a/io/init.c b/io/init.c
index 5ce627ef22c9..cf8573f0ecd5 100644
--- a/io/init.c
+++ b/io/init.c
@@ -34,7 +34,7 @@ void
 usage(void)
 {
 	fprintf(stderr,
-		_("Usage: %s [-adfinrRstVx] [-m mode] [-p prog] [-c cmd]... file\n"),
+_("Usage: %s [-adfinrRstVx] [-m mode] [-p prog] [[-c|-C] cmd]... file\n"),
 		progname);
 	exit(1);
 }
@@ -145,7 +145,7 @@ init(
 	pagesize = getpagesize();
 	gettimeofday(&stopwatch, NULL);
 
-	while ((c = getopt(argc, argv, "ac:dFfim:p:nrRstTVx")) != EOF) {
+	while ((c = getopt(argc, argv, "ac:C:dFfim:p:nrRstTVx")) != EOF) {
 		switch (c) {
 		case 'a':
 			flags |= IO_APPEND;
@@ -153,6 +153,9 @@ init(
 		case 'c':
 			add_user_command(optarg);
 			break;
+		case 'C':
+			add_oneshot_user_command(optarg);
+			break;
 		case 'd':
 			flags |= IO_DIRECT;
 			break;
diff --git a/libxcmd/command.c b/libxcmd/command.c
index decc442a9d03..5917aea42611 100644
--- a/libxcmd/command.c
+++ b/libxcmd/command.c
@@ -25,8 +25,14 @@ int		ncmds;
 
 static iterfunc_t	iter_func;
 static checkfunc_t	check_func;
-static int		ncmdline;
-static char		**cmdline;
+
+struct cmdline {
+	char	*cmdline;
+	bool	iterate;
+};
+
+static int	ncmdline;
+struct cmdline	*cmdline;
 
 static int
 compare(const void *a, const void *b)
@@ -120,12 +126,27 @@ void
 add_user_command(char *optarg)
 {
 	ncmdline++;
-	cmdline = realloc(cmdline, sizeof(char*) * (ncmdline));
+	cmdline = realloc(cmdline, sizeof(struct cmdline) * (ncmdline));
+	if (!cmdline) {
+		perror("realloc");
+		exit(1);
+	}
+	cmdline[ncmdline-1].cmdline = optarg;
+	cmdline[ncmdline-1].iterate = true;
+
+}
+
+void
+add_oneshot_user_command(char *optarg)
+{
+	ncmdline++;
+	cmdline = realloc(cmdline, sizeof(struct cmdline) * (ncmdline));
 	if (!cmdline) {
 		perror("realloc");
 		exit(1);
 	}
-	cmdline[ncmdline-1] = optarg;
+	cmdline[ncmdline-1].cmdline = optarg;
+	cmdline[ncmdline-1].iterate = false;
 }
 
 /*
@@ -212,14 +233,14 @@ command_loop(void)
 
 	/* command line mode */
 	for (i = 0; !done && i < ncmdline; i++) {
-		input = strdup(cmdline[i]);
+		input = strdup(cmdline[i].cmdline);
 		if (!input) {
 			fprintf(stderr,
 				_("cannot strdup command '%s': %s\n"),
 				cmdline[i], strerror(errno));
 			exit(1);
 		}
-		done = process_input(input, true);
+		done = process_input(input, cmdline[i].iterate);
 	}
 	free(cmdline);
 	return;
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 885df7f141e2..9db2d97d8310 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -9,10 +9,13 @@ xfs_io \- debug the I/O path of an XFS filesystem
 .B \-c
 .I cmd
 ] ... [
+.B \-C
+.I cmd
+] ... [
 .B \-p
 .I prog
 ]
-.I file
+.I [ file ]
 .br
 .B xfs_io \-V
 .SH DESCRIPTION
@@ -25,14 +28,39 @@ These code paths include not only the obvious read/write/mmap interfaces
 for manipulating files, but also cover all of the XFS extensions (such
 as space preallocation, additional inode flags, etc).
 .SH OPTIONS
+.B xfs_io
+commands may be run interactively (the default) or as arguments on the
+command line.
+Interactive mode always runs commands on the current open file, whilst commands
+run from the command line may be repeated on all open files rather than just the current
+open file.
+In general, open file iteration will occur for commands that operate on file
+content or state. In contrast, commands that operate on filesystem or
+system-wide state will only be run on the current file regardless of how many
+files are currently open.
+Multiple arguments may be given on the command line and they are run in the
+sequence given. The program exits one all commands have
+been run.
 .TP 1.0i
 .BI \-c " cmd"
-.B xfs_io
-commands may be run interactively (the default) or as arguments on
-the command line. Multiple
+Run the specified command on all currently open files.
+To maintain compatibility with historical usage, commands that can not be run on
+all open files will still be run but only execute once on the current open file.
+Multiple
+.B \-c
+arguments may be given and may be interleaved on the command line in any order
+with
+.B \-C
+commands.
+.TP
+.BI \-C " cmd"
+Run the specified command only on the current open file. 
+Multiple
+.B \-C
+arguments may be given and may be interleaved on the command line in any order
+with
 .B \-c
-arguments may be given. The commands are run in the sequence given,
-then the program exits.
+commands.
 .TP
 .BI \-p " prog"
 Set the program name for prompts and some error messages,
-- 
2.10.2

Re: [PATCH 6/6] libxcmd: add non-iterating user commands

[Amir Goldstein]

On Fri, Dec 16, 2016 at 6:41 AM, Dave Chinner <david@fromorbit.com> wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Right now command iteration is not directly controllable by the
> user; it is controlled entirely by the application command flag
> setup. Sometimes we don't want commands to iterate but only operate
> on the currently selected object.
>
> For example, the stat command iterates:
>
> $ xfs_io -c "open -r foo" -c "open bar" -c "file" -c "stat" foo
>  000  foo            (foreign,non-sync,non-direct,read-write)
>  001  foo            (foreign,non-sync,non-direct,read-only)
> [002] bar            (foreign,non-sync,non-direct,read-write)
> fd.path = "foo"
> fd.flags = non-sync,non-direct,read-write
> stat.ino = 462399
> stat.type = regular file
> stat.size = 776508
> stat.blocks = 1528
> fd.path = "foo"
> fd.flags = non-sync,non-direct,read-only
> stat.ino = 462399
> stat.type = regular file
> stat.size = 776508
> stat.blocks = 1528
> fd.path = "bar"
> fd.flags = non-sync,non-direct,read-write
> stat.ino = 475227
> stat.type = regular file
> stat.size = 0
> stat.blocks = 0
> $
>
> To do this, add a function to supply a "non-iterating" user command
> that will execute an iterating-capable command as though it
> CMD_FLAG_ONESHOT was set. Add a new command line option to xfs_io to
> drive it (-C <command>) and connect it all up. Document it in the
> xfs_io man page, too.
>
> The result of "-C stat":
>
> $ xfs_io -c "open -r foo" -c "open bar" -c "file" -C "stat" foo
>  000  foo            (foreign,non-sync,non-direct,read-write)
>  001  foo            (foreign,non-sync,non-direct,read-only)
> [002] bar            (foreign,non-sync,non-direct,read-write)
> fd.path = "bar"
> fd.flags = non-sync,non-direct,read-write
> stat.ino = 475227
> stat.type = regular file
> stat.size = 0
> stat.blocks = 0
> $
>
> Is that we only see the stat output for the active open file
> which is "bar".
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> ---
>  include/command.h |  1 +
>  io/init.c         |  7 +++++--
>  libxcmd/command.c | 33 +++++++++++++++++++++++++++------
>  man/man8/xfs_io.8 | 40 ++++++++++++++++++++++++++++++++++------
>  4 files changed, 67 insertions(+), 14 deletions(-)
>
> diff --git a/include/command.h b/include/command.h
> index 348002cbe3ed..fb3f5c79b991 100644
> --- a/include/command.h
> +++ b/include/command.h
> @@ -56,6 +56,7 @@ typedef int (*checkfunc_t)(const cmdinfo_t *ci);
>
>  extern void            add_command(const cmdinfo_t *ci);
>  extern void            add_user_command(char *optarg);
> +extern void            add_oneshot_user_command(char *optarg);
>  extern void            add_command_iterator(iterfunc_t func);
>  extern void            add_check_command(checkfunc_t cf);
>
> diff --git a/io/init.c b/io/init.c
> index 5ce627ef22c9..cf8573f0ecd5 100644
> --- a/io/init.c
> +++ b/io/init.c
> @@ -34,7 +34,7 @@ void
>  usage(void)
>  {
>         fprintf(stderr,
> -               _("Usage: %s [-adfinrRstVx] [-m mode] [-p prog] [-c cmd]... file\n"),
> +_("Usage: %s [-adfinrRstVx] [-m mode] [-p prog] [[-c|-C] cmd]... file\n"),
>                 progname);
>         exit(1);
>  }
> @@ -145,7 +145,7 @@ init(
>         pagesize = getpagesize();
>         gettimeofday(&stopwatch, NULL);
>
> -       while ((c = getopt(argc, argv, "ac:dFfim:p:nrRstTVx")) != EOF) {
> +       while ((c = getopt(argc, argv, "ac:C:dFfim:p:nrRstTVx")) != EOF) {
>                 switch (c) {
>                 case 'a':
>                         flags |= IO_APPEND;
> @@ -153,6 +153,9 @@ init(
>                 case 'c':
>                         add_user_command(optarg);
>                         break;
> +               case 'C':
> +                       add_oneshot_user_command(optarg);
> +                       break;
>                 case 'd':
>                         flags |= IO_DIRECT;
>                         break;
> diff --git a/libxcmd/command.c b/libxcmd/command.c
> index decc442a9d03..5917aea42611 100644
> --- a/libxcmd/command.c
> +++ b/libxcmd/command.c
> @@ -25,8 +25,14 @@ int          ncmds;
>
>  static iterfunc_t      iter_func;
>  static checkfunc_t     check_func;
> -static int             ncmdline;
> -static char            **cmdline;
> +
> +struct cmdline {
> +       char    *cmdline;
> +       bool    iterate;
> +};
> +
> +static int     ncmdline;
> +struct cmdline *cmdline;
>
>  static int
>  compare(const void *a, const void *b)
> @@ -120,12 +126,27 @@ void
>  add_user_command(char *optarg)
>  {
>         ncmdline++;
> -       cmdline = realloc(cmdline, sizeof(char*) * (ncmdline));
> +       cmdline = realloc(cmdline, sizeof(struct cmdline) * (ncmdline));
> +       if (!cmdline) {
> +               perror("realloc");
> +               exit(1);
> +       }
> +       cmdline[ncmdline-1].cmdline = optarg;
> +       cmdline[ncmdline-1].iterate = true;
> +
> +}
> +
> +void
> +add_oneshot_user_command(char *optarg)
> +{
> +       ncmdline++;
> +       cmdline = realloc(cmdline, sizeof(struct cmdline) * (ncmdline));
>         if (!cmdline) {
>                 perror("realloc");
>                 exit(1);
>         }
> -       cmdline[ncmdline-1] = optarg;
> +       cmdline[ncmdline-1].cmdline = optarg;
> +       cmdline[ncmdline-1].iterate = false;
>  }
>
>  /*
> @@ -212,14 +233,14 @@ command_loop(void)
>
>         /* command line mode */
>         for (i = 0; !done && i < ncmdline; i++) {
> -               input = strdup(cmdline[i]);
> +               input = strdup(cmdline[i].cmdline);
>                 if (!input) {
>                         fprintf(stderr,
>                                 _("cannot strdup command '%s': %s\n"),
>                                 cmdline[i], strerror(errno));

cmdline[i].cmdline, strerror(errno));

I wonder how come you don't get a build warning on this?

>                         exit(1);
>                 }
> -               done = process_input(input, true);
> +               done = process_input(input, cmdline[i].iterate);
>         }
>         free(cmdline);
>         return;
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 885df7f141e2..9db2d97d8310 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -9,10 +9,13 @@ xfs_io \- debug the I/O path of an XFS filesystem
>  .B \-c
>  .I cmd
>  ] ... [
> +.B \-C
> +.I cmd
> +] ... [
>  .B \-p
>  .I prog
>  ]
> -.I file
> +.I [ file ]
>  .br
>  .B xfs_io \-V
>  .SH DESCRIPTION
> @@ -25,14 +28,39 @@ These code paths include not only the obvious read/write/mmap interfaces
>  for manipulating files, but also cover all of the XFS extensions (such
>  as space preallocation, additional inode flags, etc).
>  .SH OPTIONS
> +.B xfs_io
> +commands may be run interactively (the default) or as arguments on the
> +command line.
> +Interactive mode always runs commands on the current open file, whilst commands
> +run from the command line may be repeated on all open files rather than just the current
> +open file.
> +In general, open file iteration will occur for commands that operate on file
> +content or state. In contrast, commands that operate on filesystem or
> +system-wide state will only be run on the current file regardless of how many
> +files are currently open.
> +Multiple arguments may be given on the command line and they are run in the
> +sequence given. The program exits one all commands have
> +been run.
>  .TP 1.0i
>  .BI \-c " cmd"
> -.B xfs_io
> -commands may be run interactively (the default) or as arguments on
> -the command line. Multiple
> +Run the specified command on all currently open files.
> +To maintain compatibility with historical usage, commands that can not be run on
> +all open files will still be run but only execute once on the current open file.
> +Multiple
> +.B \-c
> +arguments may be given and may be interleaved on the command line in any order
> +with
> +.B \-C
> +commands.
> +.TP
> +.BI \-C " cmd"
> +Run the specified command only on the current open file.
> +Multiple
> +.B \-C
> +arguments may be given and may be interleaved on the command line in any order
> +with
>  .B \-c
> -arguments may be given. The commands are run in the sequence given,
> -then the program exits.
> +commands.
>  .TP
>  .BI \-p " prog"
>  Set the program name for prompts and some error messages,
> --
> 2.10.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 1/6] libxcmd: check CMD_FLAG_GLOBAL inside args_command()

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/6] libxcmd: rename args_command to command_iterator

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/6] libxcmd: merge command() and iterate_command()

[Christoph Hellwig]

On Fri, Dec 16, 2016 at 03:41:12PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Simplify the command loop further by merging the command loop
> iteration checks with the command execution function. This removes
> all visibility of command iteration from the main command execution
> loop, and enables us to factor and clean up the command loop
> processing neatly.
> 
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>

Looks fine, but some minor nitpicks below:

Signed-off-by: Christoph Hellwig <hch@lst.de>

> +static int
> +process_input(

return bool instead of int (and switch done in the caller to bool as
well)?

> +	char	*input,
> +	bool	iterate)
>  {
> -	int		c, i, j = 0, done = 0;
> -	char		*input;

Align the arguments to the same row as the variables below it?

Re: [PATCH 4/6] libxcmd: don't check generic library commands

[Christoph Hellwig]

On Fri, Dec 16, 2016 at 03:41:13PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The generic "help" and "quit" commands have different methods of
> skipping user provided command check functions that may prevent them
> from running. xfs_quota use CMD_ALL_FSTYPES and xfs_io uses
> CMD_FLAG_ONESHOT.  Add a new CMD_FLAG_LIBRARY to indicate commands
> that should not be checked against application specific check
> functions so they are always present and can be run regardless of
> the context in which they are run.
> 
> This gets rid of the CMD_ALL_FSTYPES flag, and enables us to remove
> the ONESHOT check in xfs_io so we use only app specific flags for
> determining if app commands should run or not.

This should also remove the defintion of CMD_ALL_FSTYPES from init.h.

Except for that it looks fine to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 5/6] xfs_io: make various commands one-shot only

[Christoph Hellwig]

On Fri, Dec 16, 2016 at 03:41:14PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> It makes no sense to iterate the file table for some xfs_io
> commands. Some commands are already marked in this way, but lots of
> them are not and this leads to bad behaviour. For example, the open
> command will run until the process fd table is full and EMFILE is
> returned rather than just opening the specified file once.

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/6] libxcmd: merge command() and iterate_command()

[Eric Sandeen]

On 12/20/16 2:31 AM, Christoph Hellwig wrote:
> On Fri, Dec 16, 2016 at 03:41:12PM +1100, Dave Chinner wrote:
>> From: Dave Chinner <dchinner@redhat.com>
>>
>> Simplify the command loop further by merging the command loop
>> iteration checks with the command execution function. This removes
>> all visibility of command iteration from the main command execution
>> loop, and enables us to factor and clean up the command loop
>> processing neatly.
>>
>> Signed-Off-By: Dave Chinner <dchinner@redhat.com>
> 
> Looks fine, but some minor nitpicks below:
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by, maybe?

> 
>> +static int
>> +process_input(
> 
> return bool instead of int (and switch done in the caller to bool as
> well)?

Everything below it is an int, fwiw, seems like a job for another day.

>> +	char	*input,
>> +	bool	iterate)
>>  {
>> -	int		c, i, j = 0, done = 0;
>> -	char		*input;
> 
> Align the arguments to the same row as the variables below it?

I'll fix that on merge.

Thanks,
-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 3/6] libxcmd: merge command() and iterate_command()

[Christoph Hellwig]

On Tue, Dec 20, 2016 at 02:11:44PM -0600, Eric Sandeen wrote:
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Reviewed-by, maybe?

Yes,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/6] libxcmd: rename args_command to command_iterator

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

It is not particularly easy to understand the function of the
args_command abstraction. it's actually a command iterator interface
that allows callers to specify the target of the command and iterate
the command multiple times over different targets. Rename and
document the abstraction to make this functionality clear.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
 include/command.h |  4 ++--
 io/init.c         |  9 +++++++--
 libxcmd/command.c | 16 ++++++++--------
 quota/init.c      |  9 +++++++--
 4 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/include/command.h b/include/command.h
index 58bfcaac44a0..637ee06e6e9a 100644
--- a/include/command.h
+++ b/include/command.h
@@ -50,12 +50,12 @@ extern int		ncmds;
 extern void		help_init(void);
 extern void		quit_init(void);
 
-typedef int (*argsfunc_t)(int index);
+typedef int (*iterfunc_t)(int index);
 typedef int (*checkfunc_t)(const cmdinfo_t *ci);
 
 extern void		add_command(const cmdinfo_t *ci);
 extern void		add_user_command(char *optarg);
-extern void		add_args_command(argsfunc_t af);
+extern void		add_command_iterator(iterfunc_t func);
 extern void		add_check_command(checkfunc_t cf);
 
 extern const cmdinfo_t	*find_command(const char *cmd);
diff --git a/io/init.c b/io/init.c
index ab40f3745390..34009b024833 100644
--- a/io/init.c
+++ b/io/init.c
@@ -90,8 +90,13 @@ init_commands(void)
 	cowextsize_init();
 }
 
+/*
+ * This allows xfs_io commands specified on the command line to be run on every
+ * open file in the file table. Commands that should not be iterated across all
+ * open files need to specify CMD_FLAG_ONESHOT in their command flags.
+ */
 static int
-init_args_command(
+filetable_iterator(
 	int	index)
 {
 	if (index >= filecount)
@@ -214,7 +219,7 @@ init(
 	}
 
 	init_commands();
-	add_args_command(init_args_command);
+	add_command_iterator(filetable_iterator);
 	add_check_command(init_check_command);
 }
 
diff --git a/libxcmd/command.c b/libxcmd/command.c
index dce8361ce3ea..789aeb5c5e5a 100644
--- a/libxcmd/command.c
+++ b/libxcmd/command.c
@@ -23,7 +23,7 @@
 cmdinfo_t	*cmdtab;
 int		ncmds;
 
-static argsfunc_t	args_func;
+static iterfunc_t	iter_func;
 static checkfunc_t	check_func;
 static int		ncmdline;
 static char		**cmdline;
@@ -130,7 +130,7 @@ add_user_command(char *optarg)
  * so we abort straight away.
  */
 static int
-args_command(
+iterate_command(
 	const cmdinfo_t	*ct,
 	int		index)
 {
@@ -138,16 +138,16 @@ args_command(
 		return 0;
 	if (ct->flags & CMD_FLAG_ONESHOT)
 		return -1;
-	if (args_func)
-		return args_func(index);
+	if (iter_func)
+		return iter_func(index);
 	return 0;
 }
 
 void
-add_args_command(
-	argsfunc_t	af)
+add_command_iterator(
+	iterfunc_t	func)
 {
-	args_func = af;
+	iter_func = func;
 }
 
 void
@@ -171,7 +171,7 @@ command_loop(void)
 			ct = find_command(v[0]);
 			if (ct) {
 				j = 0;
-				while (!done && (j = args_command(ct, j)))
+				while (!done && (j = iterate_command(ct, j)))
 					done = command(ct, c, v);
 			} else
 				fprintf(stderr, _("command \"%s\" not found\n"),
diff --git a/quota/init.c b/quota/init.c
index 3bebbb8735f3..193f6421fd59 100644
--- a/quota/init.c
+++ b/quota/init.c
@@ -75,8 +75,13 @@ init_commands(void)
 	state_init();
 }
 
+/*
+ * This function allows xfs_quota commands to iterate across all discovered
+ * quota enabled filesystems. Commands that should not iterate all filesystems
+ * should specify CMD_FLAG_ONESHOT in their command flags.
+ */
 static int
-init_args_command(
+filesystem_iterator(
 	int	index)
 {
 	if (index >= fs_count)
@@ -189,7 +194,7 @@ init(
 	free(projopts);
 
 	init_commands();
-	add_args_command(init_args_command);
+	add_command_iterator(filesystem_iterator);
 	add_check_command(init_check_command);
 
 	/*
-- 
2.10.2