On Wed, May 18, 2016 at 12:36:20AM +0800, Zorro Lang wrote: > 1. This function try to run $XFS_IO_PROG -c "$command help", that's > wrong. It should be "help $command". I think there was more to it that than: $ sudo xfs_io -c "pwrite help" $ sudo xfs_io -c "help pwrite" pwrite [-i infile [-d] [-s skip]] [-b bs] [-S seed] [-wW] [-FBR [-Z N]] [-V N] off len -- writes a number of bytes at a specified offset writes a range of bytes (in block size increments) from the given offset [....] $ sudo xfs_io -c "foo help" command "foo" not found i.e. that check simply tells us whether the command exists or not with an error message that is captured and then tested. If we actually run the help command, we got lots of output for commands that exist and that may confuse the error string checking we then do. Yes, it's a bit of a hack, and we may not even need the "help" parameter anymore, but ISTR older versions of xfs_io needed it to parse the CLI successfully for all the different commands.... > 2. The $param can't be used for all command's options, for example > "help pwrite" include: > > -Z N -- zeed the random number generator (used when writing randomly) > (heh, zorry, the -s/-S arguments were already in use in pwrite) This bit is fine. In general, you should put separate changes into separate patches. If you find yourself iterating a list of things a patch fixes in the description, then that's a good sign it should be split into multiple patches. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html