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

[Zirong Lang <zlang@redhat.com>]

----- 原始邮件 -----
> 发件人: "Dave Chinner" <david@fromorbit.com>
> 收件人: "Zorro Lang" <zlang@redhat.com>
> 抄送: fstests@vger.kernel.org
> 发送时间: 星期四, 2016年 5 月 19日 上午 7:55:31
> 主题: Re: [PATCH 1/2] common/rc: modify _require_xfs_io_command function
> 
> 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....

Hmm, that's a smart way. So I don't need to change that before someone
new xfsprogs break this "hack rule":)

> 
> > 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.

Thanks for point that, I'll send a V2 patch only contain this change.

Thanks,
Zorro

> 
> Cheers,
> 
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>