xfs_io: fix up command iteration
v2 cover xfs_io: fix up command iteration
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 v2 1/6 libxcmd: check CMD_FLAG_GLOBAL inside args_command()
From: Dave Chinner <[email protected]>
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.
--- 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(-)
include/command.h +6 -1
@@ -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);
io/file.c +1 -1
@@ -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);
io/init.c +1 -1
@@ -104,7 +104,7 @@ static intinit_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 )) {
libxcmd/command.c +13 -7
@@ -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 ]);
libxcmd/help.c +1 -1
@@ -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" );
libxcmd/quit.c +1 -1
@@ -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);
quota/path.c +2 -2
@@ -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)
quota/report.c +1 -1
@@ -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);
Signature · 1 line
2.10.2
Re: [PATCH 1/6] libxcmd: check CMD_FLAG_GLOBAL inside args_command()
Looks good,
PATCH v2 2/6 libxcmd: rename args_command to command_iterator
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
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.
--- include/command.h | 4 ++-- io/init.c | 9 +++++++-- libxcmd/command.c | 16 ++++++++-------- quota/init.c | 9 +++++++-- 4 files changed, 24 insertions(+), 14 deletions(-)
include/command.h +2 -2
@@ -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);
io/init.c +7 -2
@@ -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); }
libxcmd/command.c +8 -8
@@ -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"),
quota/init.c +7 -2
@@ -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);/*
Signature · 1 line
2.10.2
Re: [PATCH 2/6] libxcmd: rename args_command to command_iterator
Looks fine,
PATCH v2 3/6 libxcmd: merge command() and iterate_command()
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
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.
--- libxcmd/command.c | 110 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 66 insertions(+), 44 deletions(-)
libxcmd/command.c +66 -44
@@ -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
Signature · 1 line
2.10.2
Re: [PATCH 3/6] libxcmd: merge command() and iterate_command()
Quoted 9 lines · Dave Chinner
From: Dave Chinner <[email protected]>
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 <[email protected]>
Looks fine, but some minor nitpicks below:
libxcmd/command.c add_command_iterator( · line 162
Quoted 2 lines
libxcmd/command.c +2
-0+static int +process_input(
return bool instead of int (and switch done in the caller to bool as well)?
libxcmd/command.c add_command_iterator( · line 157
Quoted 5 lines
libxcmd/command.c +2
-2+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 3/6] libxcmd: merge command() and iterate_command()
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Dave Chinner
From: Dave Chinner <[email protected]>
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.
Quoted 5 lines
Quoted 1 line
Signed-off-by Dave Chinner <[email protected]>Looks fine, but some minor nitpicks below:
Signed-off-by Christoph Hellwig <[email protected]>
Reviewed-by, maybe?
Quoted 6 lines
libxcmd/command.c add_command_iterator( · line 162
Quoted 3 lines
libxcmd/command.c +2
-0+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.
Quoted 7 lines
libxcmd/command.c add_command_iterator( · line 157
Quoted 5 lines
libxcmd/command.c +2
-2+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
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
Reviewed-byRe: [PATCH 3/6] libxcmd: merge command() and iterate_command()
Quoted 3 lines · Eric Sandeen
Quoted 1 line
Signed-off-by Christoph Hellwig <[email protected]>Reviewed-by, maybe?
Yes,
PATCH v2 4/6 libxcmd: don't check generic library commands
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
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.
--- 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(-)
include/command.h +1 -0
@@ -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 );
io/init.c +0 -3
@@ -109,9 +109,6 @@ static intinit_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 ;
libxcmd/command.c +4 -0
@@ -48,6 +48,10 @@ static intcheck_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 ;
libxcmd/help.c +1 -1
@@ -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" );
libxcmd/quit.c +1 -1
@@ -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);
quota/init.c +0 -4
@@ -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 ;
Signature · 1 line
2.10.2
Re: [PATCH 4/6] libxcmd: don't check generic library commands
Quoted 10 lines · Dave Chinner
From: Dave Chinner <[email protected]>
The generic "help" and "quit" commands have different methods of skipping user provided command check functions that may prevent them from running.
xfs_quotauseCMD_ALL_FSTYPESandxfs_iousesCMD_FLAG_ONESHOT. Add a newCMD_FLAG_LIBRARYto 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.
Quoted 3 lines
This gets rid of the
CMD_ALL_FSTYPESflag, and enables us to remove the ONESHOT check inxfs_ioso 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:
PATCH v2 5/6 xfs_io: make various commands one-shot only
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
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.
--- 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(-)
io/file.c +1 -1
@@ -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" ;
io/freeze.c +2 -2
@@ -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) {
io/getrusage.c +2 -1
@@ -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)
io/imap.c +1 -1
@@ -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)
io/inject.c +1 -1
@@ -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;
io/link.c +1 -1
@@ -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" );
io/mmap.c +2 -1
@@ -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" );
io/open.c +4 -3
@@ -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;
io/reflink.c +2 -2
@@ -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 =
io/resblks.c +1 -1
@@ -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" );
io/shutdown.c +1 -1
@@ -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" );
io/sync.c +2 -1
@@ -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" );
Signature · 1 line
2.10.2
Re: [PATCH 5/6] xfs_io: make various commands one-shot only
Quoted 7 lines · Dave Chinner
From: Dave Chinner <[email protected]>
It makes no sense to iterate the file table for some
xfs_iocommands. 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,
PATCH v2 6/6 libxcmd: add non-iterating user commands
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
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".
--- 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(-)
include/command.h +1 -0
@@ -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);
io/init.c +5 -2
@@ -34,7 +34,7 @@ voidusage (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 ;
libxcmd/command.c +27 -6
@@ -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 ;
man/man8/xfs_io.8 +34 -6
@@ -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,
Signature · 1 line
2.10.2
Re: [PATCH 6/6] libxcmd: add non-iterating user commands
Quoted 160 lines · Dave Chinner
From: Dave Chinner <[email protected]>
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_ONESHOTwas set. Add a new command line option toxfs_ioto drive it (-C <command>) and connect it all up. Document it in thexfs_ioman 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 <[email protected]>--- 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(-)include/command.h +1
-0@@ -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);io/init.c +5
-2@@ -34,7 +34,7 @@ voidusage (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 ;libxcmd/command.c +25
-4@@ -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; }/*
libxcmd/command.c command_loop(void) · line 240
Quoted 9 lines
libxcmd/command.c +1
-1@@ -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?
Quoted 78 lines
libxcmd/command.c +1
-1exit (1 ); } - done =process_input (input, true); + done =process_input (input, cmdline[i].iterate); }free (cmdline);return ;man/man8/xfs_io.8 +34
-7@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html