From: Dave Chinner <david@fromorbit.com>
To: Zorro Lang <zlang@redhat.com>
Cc: fstests@vger.kernel.org
Subject: Re: [PATCH 1/2] common/rc: modify _require_xfs_io_command function
Date: Thu, 19 May 2016 09:55:31 +1000 [thread overview]
Message-ID: <20160518235531.GB26977@dastard> (raw)
In-Reply-To: <1463502981-15593-1-git-send-email-zlang@redhat.com>
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@fromorbit.com
next prev parent reply other threads:[~2016-05-18 23:55 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-17 16:36 [PATCH 1/2] common/rc: modify _require_xfs_io_command function Zorro Lang
2016-05-17 16:36 ` [PATCH 2/2] generic/352: new case test mmaped data when blocksize less than pagesize Zorro Lang
2016-05-23 9:07 ` Eryu Guan
2016-05-23 9:19 ` Zirong Lang
2016-05-18 23:55 ` Dave Chinner [this message]
2016-05-19 3:36 ` [PATCH 1/2] common/rc: modify _require_xfs_io_command function Zirong Lang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160518235531.GB26977@dastard \
--to=david@fromorbit.com \
--cc=fstests@vger.kernel.org \
--cc=zlang@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox