Re: [PATCH 6/6] libxcmd: add non-iterating user commands

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



On Wed, Dec 7, 2016 at 6:49 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> On Wed, Dec 7, 2016 at 5:47 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
>> From: Dave Chinner <dchinner@xxxxxxxxxx>
>>
>
> Thank you for fixing this!
> See some typo fixes below.
> I will test the fix later today.
>

Short of one compilation warning I commented on,
I tested these changes and found no regression with -g quick run
I also verified that my test can be converted to use the one shot commands,
e.g.:

$XFS_IO_PROG -r foo \
       -C "open foo" \
       -C "pwrite -S 0x61 0 16" \
       -C "file 0" \
       -C "pread -v 0 16" \
| _filter_xfs_io

$XFS_IO_PROG -r bar \
       -C "mmap -r 0 16" \
       -C "open bar" \
       -C "pwrite -S 0x61 0 16" \
       -C "mread -v 0 16" \
| _filter_xfs_io

Notice that I used explicit -C for all commands including the implicit
one shot commands.

1. Do you think that xfs_io should error on -c "open foo"  to force
explicit -C "open foo"?
    it can't be breaking any existing users, because -c "open foo" is
already very broken.
2. You marked mmap ONE_SHOT, but not all the m* commands.
   Stands to reason that they should all be marked ONE_SHOT. because iterating
   the file table has nothing to do with the m* commands. no?

About the fix to overlay/016.
How would you prefer to address the conditional availability of xfs_io -C?

1. new helper _require_xfs_io_one_shot_command
2. _require_xfs_io_command "open" (which internally checks for xfs_io -C "open")
3. third option?

Cheers,
Amir.
--
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