On Mon, Mar 30, 2015 at 2:25 PM, Lukáš Czerner <lczerner@xxxxxxxxxx> wrote:
> On Mon, 30 Mar 2015, Zhaolei wrote:
>
>> Date: Mon, 30 Mar 2015 17:11:27 +0800
>> From: Zhaolei <zhaolei@xxxxxxxxxxxxxx>
>> To: fstests@xxxxxxxxxxxxxxx
>> Cc: Zhao Lei <zhaolei@xxxxxxxxxxxxxx>
>> Subject: [PATCH] Fix caller's argument for _require_command()
>>
>> From: Zhao Lei <zhaolei@xxxxxxxxxxxxxx>
>>
>> _require_command() only accept 2 arguments but some caller
>> misused this function, and caused "not_run" after we limit
>> calling way.
>>
>> This patch fixed above caller.
>>
>> Signed-off-by: Zhao Lei <zhaolei@xxxxxxxxxxxxxx>
>> ---
>> common/defrag | 13 +++++++++----
>> tests/xfs/094 | 2 +-
>> tests/xfs/103 | 2 +-
>> tests/xfs/195 | 2 +-
>> 4 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/common/defrag b/common/defrag
>> index f923dc0..b44ef98 100644
>> --- a/common/defrag
>> +++ b/common/defrag
>> @@ -22,22 +22,27 @@
>>
>> _require_defrag()
>> {
>> + local defrag_cmd
>> +
>> case "$FSTYP" in
>> xfs)
>> - DEFRAG_PROG="$XFS_FSR_PROG"
>> + defrag_cmd="$XFS_FSR_PROG"
>> + DEFRAG_PROG="$defrag_cmd"
>> ;;
>> ext4|ext4dev)
>> - DEFRAG_PROG="$E4DEFRAG_PROG"
>> + defrag_cmd="$E4DEFRAG_PROG"
>> + DEFRAG_PROG="$defrag_cmd"
>> ;;
>> btrfs)
>> - DEFRAG_PROG="$BTRFS_UTIL_PROG filesystem defragment"
>> + defrag_cmd="$BTRFS_UTIL_PROG"
>> + DEFRAG_PROG="defrag_cmd filesystem defragment"
>> ;;
>> *)
>> _notrun "defragmentation not supported for fstype \"$FSTYP\""
>> ;;
>> esac
>>
>> - _require_command "$DEFRAG_PROG" defragment
>> + _require_command "$defrag_cmd" defragment
>
> Hi,
>
> what's the point of this change ?
Lukas, this is response to my comment at:
https://patchwork.kernel.org/patch/6016291/
But still I agree with you, this change seems a bit to come out of
thin air. A more detailed explanation on the commit message would
help, perhaps giving the example of btrfs/005 failing due to the
reason I pointed out.
>
>> _require_xfs_io_command "fiemap"
>> }
>>
>> diff --git a/tests/xfs/094 b/tests/xfs/094
>> index cee42d6..5c6e98d 100755
>> --- a/tests/xfs/094
>> +++ b/tests/xfs/094
>> @@ -46,7 +46,7 @@ _supported_fs xfs
>> _supported_os IRIX Linux
>> _require_realtime
>> _require_scratch
>> -_require_command "$XFS_IO_PROG" xfs_io
>> +[ -n "$XFS_IO_PROG" ] || _notrun "xfs_io executable not found"
>
> Why ? Do not we get the same result from
>
> _require_command "$XFS_IO_PROG" xfs_io
>
> anyway if the $XFS_IO_PROG isn't set ?
Agreed. Seems unneeded because XFS_IO_PROG is always a single word
therefore not causing a problem like btrfs' "btrfs filesystem
defragment" case.
>
> -Lukas
>
>>
>> _filter_realtime_flag()
>> {
>> diff --git a/tests/xfs/103 b/tests/xfs/103
>> index cbe884f..d80dbf2 100755
>> --- a/tests/xfs/103
>> +++ b/tests/xfs/103
>> @@ -66,7 +66,7 @@ _filter_noymlinks_flag()
>> # real QA test starts here
>> _supported_os Linux IRIX
>> _supported_fs xfs
>> -_require_command "$XFS_IO_PROG" xfs_io
>> +[ -n "$XFS_IO_PROG" ] || _notrun "xfs_io executable not found"
>> _require_scratch
>>
>> _create_scratch
>> diff --git a/tests/xfs/195 b/tests/xfs/195
>> index 21fcb00..075022d 100755
>> --- a/tests/xfs/195
>> +++ b/tests/xfs/195
>> @@ -65,7 +65,7 @@ _supported_os Linux
>>
>> _require_test
>> _require_user
>> -_require_command "$XFSDUMP_PROG" xfsdump
>> +[ "$XFSDUMP_PROG" = "" ] && _notrun "xfsdump not found"
>>
>> echo "Preparing subtree"
>> mkdir $TEST_DIR/d
>>
> --
> 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
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
--
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
