Re: [PATCH 1/2] common/rc: modify _require_xfs_io_command function

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



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



[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux