| Message ID | 1481003038-23740-1-git-send-email-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed, Dec 7, 2016 at 12:51 AM, Dave Chinner <david@fromorbit.com> wrote: ... > Again, you've defined the solution to meet only your immediate > needs. The git history will tells us what is "proper", and I'm > pretty sure what I suggested first up (use of CMD_FLAG_GLOBAL) will > be close to the mark.... > True, I've defined the solution to meet my needs, though I believe my needs are not crazy and serve a greater good. The reason I did not go for the "proper" solution is because I did not understand how your suggestion aims to solve the problem defined by my test. And by solve I don't mean just avoid the endless loop. > So the initial fix for the current problem is to mark all the > "one-shot" commands with CMD_FLAG_GLOBAL so they don't iterate the > file table and behave sanely by default. I'd rename CMD_FLAG_GLOBAL > to something more appropriate as well, CMD_FLAG_ONESHOT? > and probably rework the main > command loop to get this check out of the main loop and into the > args_command() function where we can do single shot command > control cleanly. > And I would gladly follow your suggestion to fix endless loop issue, but first I would like to understood where this is going to end up in terms of "expected behavior" (see below). > The next fix is to be able to control iteration directly, so if we > want to stat all or just the current file we can. I'd suggest that > we should introduce a "-C <cmd>" option to indicate that the command > should only be run on the current active file. That will allow new > tests that use multiple files to control behaviour appropriately... > So that is what I was missing in your first reply. I realized that a new semantic must be added to be able to both keep existing behavior and enable the desired functionality. But I am not sure that -C <cmd> does the job. See the example below from test overlay/016. It's a perfectly valid use case that cannot be described with existing semantics. # # case #1: # open file for read (rofd) # open file for write (rwfd) # write to rwfd # read from rofd # $XFS_IO_PROG \ -c "open -r foo" \ -c "open foo" \ -c "pwrite -S 0x61 0 16" \ -c "file 0" \ -c "pread -v 0 16" \ With the suggested -C <cmd> semantics, it would look something like this: $XFS_IO_PROG \ -c "open -r foo" \ -c "open foo" \ -C "pwrite -S 0x61 0 16" \ -c "file 0" \ -C "pread -v 0 16" \ This could work out if -C meant to mark that cmd CMD_FLAG_ONESHOT and run it only once. It suffice for the use case of writing a script, but not sure if that is what you meant. I assert that any usage containing multiple files and -C commands is going to be confusing to read, but if that is the price we have to pay for not getting the semantics clear from the start, so be it. I am hoping you are able to clarify that we are on the same page, so I can go on with implementation. Thanks, Amir. -- 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
diff --git a/io/init.c b/io/init.c index a9191cf..54b998c 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 cmd]... [file]...\n"), progname); exit(1); } @@ -90,14 +90,15 @@ init_commands(void) cowextsize_init(); } +/* + * Return true for first call and false for the next call, + * to execute each command just once. + */ static int init_args_command( int index) { - if (index >= filecount) - return 0; - file = &filetable[index++]; - return index; + return !index; } static int diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8 index 885df7f..f61eee6 100644 --- a/man/man8/xfs_io.8 +++ b/man/man8/xfs_io.8 @@ -11,8 +11,9 @@ xfs_io \- debug the I/O path of an XFS filesystem ] ... [ .B \-p .I prog -] +] [ .I file +] ... .br .B xfs_io \-V .SH DESCRIPTION @@ -42,7 +43,11 @@ the default value is .B \-f Create .I file -if it does not already exist. +if it does not already exist. Multiple +.I file +arguments may be given. The files are open before the +.B \-c +commands are executed. .TP .B \-r Open
There is an undocumented and possibly unused feature in xfs_io where all commands are executed per file when multiple files are provided in the args list. This feature creates ambiguity when trying to execute commands such as "open" and "file" from command line and result in an endless loop in the command loop code, e.g.: xfs_io -r -c "open -f bar" -c print foo bar: Too many open files [000] foo (foreign,non-sync,non-direct,read-only) 001 bar (foreign,non-sync,non-direct,read-write) 002 bar (foreign,non-sync,non-direct,read-write) 003 bar (foreign,non-sync,non-direct,read-write) 004 bar (foreign,non-sync,non-direct,read-write) 005 bar (foreign,non-sync,non-direct,read-write) 006 bar (foreign,non-sync,non-direct,read-write) Also, when running xfs_io -c <cmd> without any file args, xfs_io exits without doing anything. This behavior is also undocumented and makes very little sense. Change the behavior of xfs_io, so commands given with -c <cmd> are executed exactly once, regardless of the number of file args. This enables writing proper xfs_io scripts in command line, which include "open" and "file" commands. Update the man page and usage to reflect the fact that multiple as well as zero file args can be given in args list. This change does not modify the behavior of xfs_io in the most commonly used case of single file argument in command line and no "open" and "file" commands in command line. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- io/init.c | 11 ++++++----- man/man8/xfs_io.8 | 9 +++++++-- 2 files changed, 13 insertions(+), 7 deletions(-) v4: - Allow multiple file args - Update man page and usage to allow multiple file args and spell out the expected behavior in this case v3: - Forbid multiple file args - Update man page and usage to allow no file arg - Add endless loop bug report to commit message v2: - Fix the case of multiple file args v1: - Fix the case of zero file args