* [PATCH] Fix caller's argument for _require_command()
@ 2015-03-30 9:11 Zhaolei
2015-03-30 13:25 ` Lukáš Czerner
0 siblings, 1 reply; 9+ messages in thread
From: Zhaolei @ 2015-03-30 9:11 UTC (permalink / raw)
To: fstests; +Cc: Zhao Lei
From: Zhao Lei <zhaolei@cn.fujitsu.com>
_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@cn.fujitsu.com>
---
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
_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"
_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
--
1.8.5.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix caller's argument for _require_command()
2015-03-30 9:11 [PATCH] Fix caller's argument for _require_command() Zhaolei
@ 2015-03-30 13:25 ` Lukáš Czerner
2015-03-31 2:34 ` Zhao Lei
2015-04-02 13:17 ` Filipe David Manana
0 siblings, 2 replies; 9+ messages in thread
From: Lukáš Czerner @ 2015-03-30 13:25 UTC (permalink / raw)
To: Zhaolei; +Cc: fstests
On Mon, 30 Mar 2015, Zhaolei wrote:
> Date: Mon, 30 Mar 2015 17:11:27 +0800
> From: Zhaolei <zhaolei@cn.fujitsu.com>
> To: fstests@vger.kernel.org
> Cc: Zhao Lei <zhaolei@cn.fujitsu.com>
> Subject: [PATCH] Fix caller's argument for _require_command()
>
> From: Zhao Lei <zhaolei@cn.fujitsu.com>
>
> _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@cn.fujitsu.com>
> ---
> 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 ?
> _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 ?
-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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] Fix caller's argument for _require_command()
2015-03-30 13:25 ` Lukáš Czerner
@ 2015-03-31 2:34 ` Zhao Lei
2015-04-02 13:47 ` Lukáš Czerner
2015-04-02 13:17 ` Filipe David Manana
1 sibling, 1 reply; 9+ messages in thread
From: Zhao Lei @ 2015-03-31 2:34 UTC (permalink / raw)
To: 'Lukáš Czerner'; +Cc: fstests
Hi, Czerner
Thanks for review.
> -----Original Message-----
> From: Lukáš Czerner [mailto:lczerner@redhat.com]
> Sent: Monday, March 30, 2015 9:25 PM
> To: Zhaolei
> Cc: fstests@vger.kernel.org
> Subject: Re: [PATCH] Fix caller's argument for _require_command()
>
> On Mon, 30 Mar 2015, Zhaolei wrote:
>
> > Date: Mon, 30 Mar 2015 17:11:27 +0800
> > From: Zhaolei <zhaolei@cn.fujitsu.com>
> > To: fstests@vger.kernel.org
> > Cc: Zhao Lei <zhaolei@cn.fujitsu.com>
> > Subject: [PATCH] Fix caller's argument for _require_command()
> >
> > From: Zhao Lei <zhaolei@cn.fujitsu.com>
> >
> > _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@cn.fujitsu.com>
> > ---
> > 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 ?
>
"$DEFRAG_PROG" include program and argument,
and can not passed test of:
[[ -x "$DEFRAG_PROG" ]]
in _require_command().
> > _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 ?
>
In some case, "$XFS_IO_PROG" include both program and argument,
and cause wrong result in _require_command().
Thanks
Zhaolei
> -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
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix caller's argument for _require_command()
2015-03-30 13:25 ` Lukáš Czerner
2015-03-31 2:34 ` Zhao Lei
@ 2015-04-02 13:17 ` Filipe David Manana
2015-04-02 13:42 ` Lukáš Czerner
1 sibling, 1 reply; 9+ messages in thread
From: Filipe David Manana @ 2015-04-02 13:17 UTC (permalink / raw)
To: Lukáš Czerner; +Cc: Zhaolei, fstests
On Mon, Mar 30, 2015 at 2:25 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
> On Mon, 30 Mar 2015, Zhaolei wrote:
>
>> Date: Mon, 30 Mar 2015 17:11:27 +0800
>> From: Zhaolei <zhaolei@cn.fujitsu.com>
>> To: fstests@vger.kernel.org
>> Cc: Zhao Lei <zhaolei@cn.fujitsu.com>
>> Subject: [PATCH] Fix caller's argument for _require_command()
>>
>> From: Zhao Lei <zhaolei@cn.fujitsu.com>
>>
>> _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@cn.fujitsu.com>
>> ---
>> 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@vger.kernel.org
> 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."
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix caller's argument for _require_command()
2015-04-02 13:17 ` Filipe David Manana
@ 2015-04-02 13:42 ` Lukáš Czerner
0 siblings, 0 replies; 9+ messages in thread
From: Lukáš Czerner @ 2015-04-02 13:42 UTC (permalink / raw)
To: Filipe David Manana; +Cc: Zhaolei, fstests
[-- Attachment #1: Type: TEXT/PLAIN, Size: 4474 bytes --]
On Thu, 2 Apr 2015, Filipe David Manana wrote:
> Date: Thu, 2 Apr 2015 14:17:43 +0100
> From: Filipe David Manana <fdmanana@gmail.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: Zhaolei <zhaolei@cn.fujitsu.com>, fstests@vger.kernel.org
> Subject: Re: [PATCH] Fix caller's argument for _require_command()
>
> On Mon, Mar 30, 2015 at 2:25 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
> > On Mon, 30 Mar 2015, Zhaolei wrote:
> >
> >> Date: Mon, 30 Mar 2015 17:11:27 +0800
> >> From: Zhaolei <zhaolei@cn.fujitsu.com>
> >> To: fstests@vger.kernel.org
> >> Cc: Zhao Lei <zhaolei@cn.fujitsu.com>
> >> Subject: [PATCH] Fix caller's argument for _require_command()
> >>
> >> From: Zhao Lei <zhaolei@cn.fujitsu.com>
> >>
> >> _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@cn.fujitsu.com>
> >> ---
> >> 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.
Ah Ok, now it makes sense to me, but the description needs to be updated.
Thanks!
-Lukas
>
> >
> >> _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@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] Fix caller's argument for _require_command()
2015-03-31 2:34 ` Zhao Lei
@ 2015-04-02 13:47 ` Lukáš Czerner
2015-04-03 12:50 ` Zhao Lei
2015-04-10 11:13 ` Zhao Lei
0 siblings, 2 replies; 9+ messages in thread
From: Lukáš Czerner @ 2015-04-02 13:47 UTC (permalink / raw)
To: Zhao Lei; +Cc: fstests
[-- Attachment #1: Type: TEXT/PLAIN, Size: 4607 bytes --]
On Tue, 31 Mar 2015, Zhao Lei wrote:
> Date: Tue, 31 Mar 2015 10:34:51 +0800
> From: Zhao Lei <zhaolei@cn.fujitsu.com>
> To: 'Lukáš Czerner' <lczerner@redhat.com>
> Cc: fstests@vger.kernel.org
> Subject: RE: [PATCH] Fix caller's argument for _require_command()
>
> Hi, Czerner
>
> Thanks for review.
>
> > -----Original Message-----
> > From: Lukáš Czerner [mailto:lczerner@redhat.com]
> > Sent: Monday, March 30, 2015 9:25 PM
> > To: Zhaolei
> > Cc: fstests@vger.kernel.org
> > Subject: Re: [PATCH] Fix caller's argument for _require_command()
> >
> > On Mon, 30 Mar 2015, Zhaolei wrote:
> >
> > > Date: Mon, 30 Mar 2015 17:11:27 +0800
> > > From: Zhaolei <zhaolei@cn.fujitsu.com>
> > > To: fstests@vger.kernel.org
> > > Cc: Zhao Lei <zhaolei@cn.fujitsu.com>
> > > Subject: [PATCH] Fix caller's argument for _require_command()
> > >
> > > From: Zhao Lei <zhaolei@cn.fujitsu.com>
> > >
> > > _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@cn.fujitsu.com>
> > > ---
> > > 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 ?
> >
>
> "$DEFRAG_PROG" include program and argument,
> and can not passed test of:
> [[ -x "$DEFRAG_PROG" ]]
> in _require_command().
>
> > > _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 ?
> >
> In some case, "$XFS_IO_PROG" include both program and argument,
> and cause wrong result in _require_command().
It should not include any arguments. It's probably -F argument you
have in mind, but that's deprecated and is not needed anymore. So if
we have some places where we add those it needs to be fixed there.
Thanks!
-Lukas
>
> Thanks
> Zhaolei
>
> > -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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] Fix caller's argument for _require_command()
2015-04-02 13:47 ` Lukáš Czerner
@ 2015-04-03 12:50 ` Zhao Lei
2015-04-10 11:13 ` Zhao Lei
1 sibling, 0 replies; 9+ messages in thread
From: Zhao Lei @ 2015-04-03 12:50 UTC (permalink / raw)
To: 'Lukáš Czerner'; +Cc: fstests
Hi, Lukas
> -----Original Message-----
> From: Lukáš Czerner [mailto:lczerner@redhat.com]
> Sent: Thursday, April 02, 2015 9:47 PM
> To: Zhao Lei
> Cc: fstests@vger.kernel.org
> Subject: RE: [PATCH] Fix caller's argument for _require_command()
>
> On Tue, 31 Mar 2015, Zhao Lei wrote:
>
> > Date: Tue, 31 Mar 2015 10:34:51 +0800
> > From: Zhao Lei <zhaolei@cn.fujitsu.com>
> > To: 'Lukáš Czerner' <lczerner@redhat.com>
> > Cc: fstests@vger.kernel.org
> > Subject: RE: [PATCH] Fix caller's argument for _require_command()
> >
> > Hi, Czerner
> >
> > Thanks for review.
> >
> > > -----Original Message-----
> > > From: Lukáš Czerner [mailto:lczerner@redhat.com]
> > > Sent: Monday, March 30, 2015 9:25 PM
> > > To: Zhaolei
> > > Cc: fstests@vger.kernel.org
> > > Subject: Re: [PATCH] Fix caller's argument for _require_command()
> > >
> > > On Mon, 30 Mar 2015, Zhaolei wrote:
> > >
> > > > Date: Mon, 30 Mar 2015 17:11:27 +0800
> > > > From: Zhaolei <zhaolei@cn.fujitsu.com>
> > > > To: fstests@vger.kernel.org
> > > > Cc: Zhao Lei <zhaolei@cn.fujitsu.com>
> > > > Subject: [PATCH] Fix caller's argument for _require_command()
> > > >
> > > > From: Zhao Lei <zhaolei@cn.fujitsu.com>
> > > >
> > > > _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@cn.fujitsu.com>
> > > > ---
> > > > 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 ?
> > >
> >
> > "$DEFRAG_PROG" include program and argument, and can not passed test
> > of:
> > [[ -x "$DEFRAG_PROG" ]]
> > in _require_command().
> >
> > > > _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 ?
> > >
> > In some case, "$XFS_IO_PROG" include both program and argument, and
> > cause wrong result in _require_command().
>
> It should not include any arguments. It's probably -F argument you have in mind,
> but that's deprecated and is not needed anymore. So if we have some places
> where we add those it needs to be fixed there.
>
Yes, current code support -F option although it is deprecated:
common/rc:
# Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
xfs_io -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
export XFS_IO_PROG="$XFS_IO_PROG -F"
Is it used to support old verion of xfs? Or could I remove your above code
to make _require_command() works with $XFS_IO_PROG?
Thanks
Zhaolei
> Thanks!
> -Lukas
>
> >
> > Thanks
> > Zhaolei
> >
> > > -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@vger.kernel.org More majordomo info
> > at http://vger.kernel.org/majordomo-info.html
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] Fix caller's argument for _require_command()
2015-04-02 13:47 ` Lukáš Czerner
2015-04-03 12:50 ` Zhao Lei
@ 2015-04-10 11:13 ` Zhao Lei
2015-04-10 11:28 ` Lukáš Czerner
1 sibling, 1 reply; 9+ messages in thread
From: Zhao Lei @ 2015-04-10 11:13 UTC (permalink / raw)
To: 'Lukáš Czerner'; +Cc: fstests
Hi, Lukas
> -----Original Message-----
> From: Zhao Lei [mailto:zhaolei@cn.fujitsu.com]
> Sent: Friday, April 03, 2015 8:51 PM
> To: 'Lukáš Czerner'
> Cc: 'fstests@vger.kernel.org'
> Subject: RE: [PATCH] Fix caller's argument for _require_command()
>
> Hi, Lukas
>
> > -----Original Message-----
> > From: Lukáš Czerner [mailto:lczerner@redhat.com]
> > Sent: Thursday, April 02, 2015 9:47 PM
> > To: Zhao Lei
> > Cc: fstests@vger.kernel.org
> > Subject: RE: [PATCH] Fix caller's argument for _require_command()
> >
> > On Tue, 31 Mar 2015, Zhao Lei wrote:
> >
> > > Date: Tue, 31 Mar 2015 10:34:51 +0800
> > > From: Zhao Lei <zhaolei@cn.fujitsu.com>
> > > To: 'Lukáš Czerner' <lczerner@redhat.com>
> > > Cc: fstests@vger.kernel.org
> > > Subject: RE: [PATCH] Fix caller's argument for _require_command()
> > >
> > > Hi, Czerner
> > >
> > > Thanks for review.
> > >
> > > > -----Original Message-----
> > > > From: Lukáš Czerner [mailto:lczerner@redhat.com]
> > > > Sent: Monday, March 30, 2015 9:25 PM
> > > > To: Zhaolei
> > > > Cc: fstests@vger.kernel.org
> > > > Subject: Re: [PATCH] Fix caller's argument for _require_command()
> > > >
> > > > On Mon, 30 Mar 2015, Zhaolei wrote:
> > > >
> > > > > Date: Mon, 30 Mar 2015 17:11:27 +0800
> > > > > From: Zhaolei <zhaolei@cn.fujitsu.com>
> > > > > To: fstests@vger.kernel.org
> > > > > Cc: Zhao Lei <zhaolei@cn.fujitsu.com>
> > > > > Subject: [PATCH] Fix caller's argument for _require_command()
> > > > >
> > > > > From: Zhao Lei <zhaolei@cn.fujitsu.com>
> > > > >
> > > > > _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@cn.fujitsu.com>
> > > > > ---
> > > > > 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 ?
> > > >
> > >
> > > "$DEFRAG_PROG" include program and argument, and can not passed test
> > > of:
> > > [[ -x "$DEFRAG_PROG" ]]
> > > in _require_command().
> > >
> > > > > _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 ?
> > > >
> > > In some case, "$XFS_IO_PROG" include both program and argument, and
> > > cause wrong result in _require_command().
> >
> > It should not include any arguments. It's probably -F argument you
> > have in mind, but that's deprecated and is not needed anymore. So if
> > we have some places where we add those it needs to be fixed there.
> >
>
> Yes, current code support -F option although it is deprecated:
> common/rc:
> # Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
> xfs_io -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
> export XFS_IO_PROG="$XFS_IO_PROG -F"
>
> Is it used to support old verion of xfs? Or could I remove your above code to
> make _require_command() works with $XFS_IO_PROG?
>
Since this bug prevent some test cases,
could you allow me to send v2 to delete your above 3 lines for -F option?
Thanks
Zhaolei
> Thanks
> Zhaolei
>
> > Thanks!
> > -Lukas
> >
> > >
> > > Thanks
> > > Zhaolei
> > >
> > > > -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@vger.kernel.org More majordomo
> > > info at http://vger.kernel.org/majordomo-info.html
> > >
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] Fix caller's argument for _require_command()
2015-04-10 11:13 ` Zhao Lei
@ 2015-04-10 11:28 ` Lukáš Czerner
0 siblings, 0 replies; 9+ messages in thread
From: Lukáš Czerner @ 2015-04-10 11:28 UTC (permalink / raw)
To: Zhao Lei; +Cc: fstests
[-- Attachment #1: Type: TEXT/PLAIN, Size: 7334 bytes --]
On Fri, 10 Apr 2015, Zhao Lei wrote:
> Date: Fri, 10 Apr 2015 19:13:56 +0800
> From: Zhao Lei <zhaolei@cn.fujitsu.com>
> To: 'Lukáš Czerner' <lczerner@redhat.com>
> Cc: fstests@vger.kernel.org
> Subject: RE: [PATCH] Fix caller's argument for _require_command()
>
> Hi, Lukas
>
> > -----Original Message-----
> > From: Zhao Lei [mailto:zhaolei@cn.fujitsu.com]
> > Sent: Friday, April 03, 2015 8:51 PM
> > To: 'Lukáš Czerner'
> > Cc: 'fstests@vger.kernel.org'
> > Subject: RE: [PATCH] Fix caller's argument for _require_command()
> >
> > Hi, Lukas
> >
> > > -----Original Message-----
> > > From: Lukáš Czerner [mailto:lczerner@redhat.com]
> > > Sent: Thursday, April 02, 2015 9:47 PM
> > > To: Zhao Lei
> > > Cc: fstests@vger.kernel.org
> > > Subject: RE: [PATCH] Fix caller's argument for _require_command()
> > >
> > > On Tue, 31 Mar 2015, Zhao Lei wrote:
> > >
> > > > Date: Tue, 31 Mar 2015 10:34:51 +0800
> > > > From: Zhao Lei <zhaolei@cn.fujitsu.com>
> > > > To: 'Lukáš Czerner' <lczerner@redhat.com>
> > > > Cc: fstests@vger.kernel.org
> > > > Subject: RE: [PATCH] Fix caller's argument for _require_command()
> > > >
> > > > Hi, Czerner
> > > >
> > > > Thanks for review.
> > > >
> > > > > -----Original Message-----
> > > > > From: Lukáš Czerner [mailto:lczerner@redhat.com]
> > > > > Sent: Monday, March 30, 2015 9:25 PM
> > > > > To: Zhaolei
> > > > > Cc: fstests@vger.kernel.org
> > > > > Subject: Re: [PATCH] Fix caller's argument for _require_command()
> > > > >
> > > > > On Mon, 30 Mar 2015, Zhaolei wrote:
> > > > >
> > > > > > Date: Mon, 30 Mar 2015 17:11:27 +0800
> > > > > > From: Zhaolei <zhaolei@cn.fujitsu.com>
> > > > > > To: fstests@vger.kernel.org
> > > > > > Cc: Zhao Lei <zhaolei@cn.fujitsu.com>
> > > > > > Subject: [PATCH] Fix caller's argument for _require_command()
> > > > > >
> > > > > > From: Zhao Lei <zhaolei@cn.fujitsu.com>
> > > > > >
> > > > > > _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@cn.fujitsu.com>
> > > > > > ---
> > > > > > 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 ?
> > > > >
> > > >
> > > > "$DEFRAG_PROG" include program and argument, and can not passed test
> > > > of:
> > > > [[ -x "$DEFRAG_PROG" ]]
> > > > in _require_command().
> > > >
> > > > > > _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 ?
> > > > >
> > > > In some case, "$XFS_IO_PROG" include both program and argument, and
> > > > cause wrong result in _require_command().
> > >
> > > It should not include any arguments. It's probably -F argument you
> > > have in mind, but that's deprecated and is not needed anymore. So if
> > > we have some places where we add those it needs to be fixed there.
> > >
> >
> > Yes, current code support -F option although it is deprecated:
> > common/rc:
> > # Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
> > xfs_io -c stat $TEST_DIR 2>&1 | grep -q "is not on an XFS filesystem" && \
> > export XFS_IO_PROG="$XFS_IO_PROG -F"
> >
> > Is it used to support old verion of xfs? Or could I remove your above code to
> > make _require_command() works with $XFS_IO_PROG?
> >
> Since this bug prevent some test cases,
> could you allow me to send v2 to delete your above 3 lines for -F option?
Ah, I am sorry for the delay I completely missed your email. Yes, I
think that we should remove the -F option entirely.
Thanks!
-Lukas
>
> Thanks
> Zhaolei
>
> > Thanks
> > Zhaolei
> >
> > > Thanks!
> > > -Lukas
> > >
> > > >
> > > > Thanks
> > > > Zhaolei
> > > >
> > > > > -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@vger.kernel.org More majordomo
> > > > info at http://vger.kernel.org/majordomo-info.html
> > > >
>
>
> --
> 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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-04-10 11:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-30 9:11 [PATCH] Fix caller's argument for _require_command() Zhaolei
2015-03-30 13:25 ` Lukáš Czerner
2015-03-31 2:34 ` Zhao Lei
2015-04-02 13:47 ` Lukáš Czerner
2015-04-03 12:50 ` Zhao Lei
2015-04-10 11:13 ` Zhao Lei
2015-04-10 11:28 ` Lukáš Czerner
2015-04-02 13:17 ` Filipe David Manana
2015-04-02 13:42 ` Lukáš Czerner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.