[v2] common/rc: Add _require_filefrag_options() to check options for filefrag
diff mbox series

Message ID 1542950663-7784-1-git-send-email-yangx.jy@cn.fujitsu.com
State New
Headers show
Series
  • [v2] common/rc: Add _require_filefrag_options() to check options for filefrag
Related show

Commit Message

Xiao Yang Nov. 23, 2018, 5:24 a.m. UTC
In generic/519, filefrag command use FIBMAP ioctl(-B option) to print
output in extent format(-e option) on purpose and sync file(-s option),
so add _require_filefrag_options() to check if the command supports
all of these options.

References:
1) filefrag supports -e option by commit 2508eaa since e2fsprogs v1.42.7.
2) filefrag supports -B option by commit 5d5e01d since e2fsprogs v1.41.9.
3) filefrag supports -s option by commit e62847c since e2fsprogs v1.41.6.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 common/rc         | 15 ++++++++++++++-
 tests/generic/519 |  2 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

Comments

Dave Chinner Nov. 26, 2018, 9:50 p.m. UTC | #1
On Fri, Nov 23, 2018 at 01:24:23PM +0800, Xiao Yang wrote:
> In generic/519, filefrag command use FIBMAP ioctl(-B option) to print
> output in extent format(-e option) on purpose and sync file(-s option),
> so add _require_filefrag_options() to check if the command supports
> all of these options.
> 
> References:
> 1) filefrag supports -e option by commit 2508eaa since e2fsprogs v1.42.7.
> 2) filefrag supports -B option by commit 5d5e01d since e2fsprogs v1.41.9.
> 3) filefrag supports -s option by commit e62847c since e2fsprogs v1.41.6.
> 
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  common/rc         | 15 ++++++++++++++-
>  tests/generic/519 |  2 +-
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index ecb1738..e5da648 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3843,10 +3843,23 @@ _require_scratch_btime()
>  	_scratch_unmount
>  }
>  
> -_require_fibmap()
> +_require_filefrag_options()
>  {
>  	_require_command "$FILEFRAG_PROG" filefrag
>  
> +	local options=$1
> +	local file="$TEST_DIR/options_testfile"
> +
> +	echo "XX" > $file
> +	${FILEFRAG_PROG} -$options $file 2>&1 | grep -q "invalid option" && \
> +		_notrun "filefrag doesn't support $options option"
> +	rm -f $file
> +}

Can we just get rid of filefrag and use xfs_io's fiemap commands
instead?

Cheers,

Dave.
Xiao Yang Nov. 27, 2018, 2:47 a.m. UTC | #2
On 2018/11/27 5:50, Dave Chinner wrote:
> On Fri, Nov 23, 2018 at 01:24:23PM +0800, Xiao Yang wrote:
>> In generic/519, filefrag command use FIBMAP ioctl(-B option) to print
>> output in extent format(-e option) on purpose and sync file(-s option),
>> so add _require_filefrag_options() to check if the command supports
>> all of these options.
>>
>> References:
>> 1) filefrag supports -e option by commit 2508eaa since e2fsprogs v1.42.7.
>> 2) filefrag supports -B option by commit 5d5e01d since e2fsprogs v1.41.9.
>> 3) filefrag supports -s option by commit e62847c since e2fsprogs v1.41.6.
>>
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> ---
>>   common/rc         | 15 ++++++++++++++-
>>   tests/generic/519 |  2 +-
>>   2 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/rc b/common/rc
>> index ecb1738..e5da648 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -3843,10 +3843,23 @@ _require_scratch_btime()
>>   	_scratch_unmount
>>   }
>>
>> -_require_fibmap()
>> +_require_filefrag_options()
>>   {
>>   	_require_command "$FILEFRAG_PROG" filefrag
>>
>> +	local options=$1
>> +	local file="$TEST_DIR/options_testfile"
>> +
>> +	echo "XX">  $file
>> +	${FILEFRAG_PROG} -$options $file 2>&1 | grep -q "invalid option"&&  \
>> +		_notrun "filefrag doesn't support $options option"
>> +	rm -f $file
>> +}
> Can we just get rid of filefrag and use xfs_io's fiemap commands
> instead?
Hi Dave,

According to the comment in test,  we have to reproduce the bug by 
specifying FIBMAP ioctl instead of FIEMAP ioctl.
It seems that we can just use FIEMAP ioctl by xfs_io's fiemap command, 
so we use FIBMAP ioctl  forcely by filefrag
with -B option.

Best Regards,
Xiao Yang
> Cheers,
>
> Dave.
Dave Chinner Nov. 29, 2018, 1:55 a.m. UTC | #3
On Tue, Nov 27, 2018 at 10:47:58AM +0800, Xiao Yang wrote:
> On 2018/11/27 5:50, Dave Chinner wrote:
> >On Fri, Nov 23, 2018 at 01:24:23PM +0800, Xiao Yang wrote:
> >>In generic/519, filefrag command use FIBMAP ioctl(-B option) to print
> >>output in extent format(-e option) on purpose and sync file(-s option),
> >>so add _require_filefrag_options() to check if the command supports
> >>all of these options.
> >>
> >>References:
> >>1) filefrag supports -e option by commit 2508eaa since e2fsprogs v1.42.7.
> >>2) filefrag supports -B option by commit 5d5e01d since e2fsprogs v1.41.9.
> >>3) filefrag supports -s option by commit e62847c since e2fsprogs v1.41.6.
> >>
> >>Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
> >>---
> >>  common/rc         | 15 ++++++++++++++-
> >>  tests/generic/519 |  2 +-
> >>  2 files changed, 15 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/common/rc b/common/rc
> >>index ecb1738..e5da648 100644
> >>--- a/common/rc
> >>+++ b/common/rc
> >>@@ -3843,10 +3843,23 @@ _require_scratch_btime()
> >>  	_scratch_unmount
> >>  }
> >>
> >>-_require_fibmap()
> >>+_require_filefrag_options()
> >>  {
> >>  	_require_command "$FILEFRAG_PROG" filefrag
> >>
> >>+	local options=$1
> >>+	local file="$TEST_DIR/options_testfile"
> >>+
> >>+	echo "XX">  $file
> >>+	${FILEFRAG_PROG} -$options $file 2>&1 | grep -q "invalid option"&&  \
> >>+		_notrun "filefrag doesn't support $options option"
> >>+	rm -f $file
> >>+}
> >Can we just get rid of filefrag and use xfs_io's fiemap commands
> >instead?
> Hi Dave,
> 
> According to the comment in test,  we have to reproduce the bug by
> specifying FIBMAP ioctl instead of FIEMAP ioctl.
> It seems that we can just use FIEMAP ioctl by xfs_io's fiemap
> command, so we use FIBMAP ioctl  forcely by filefrag
> with -B option.

Add an fibmap command to xfs_io so we are not dependent on filefrag
for testing fibmap.

Cheers,

Dave.

Patch
diff mbox series

diff --git a/common/rc b/common/rc
index ecb1738..e5da648 100644
--- a/common/rc
+++ b/common/rc
@@ -3843,10 +3843,23 @@  _require_scratch_btime()
 	_scratch_unmount
 }
 
-_require_fibmap()
+_require_filefrag_options()
 {
 	_require_command "$FILEFRAG_PROG" filefrag
 
+	local options=$1
+	local file="$TEST_DIR/options_testfile"
+
+	echo "XX" > $file
+	${FILEFRAG_PROG} -$options $file 2>&1 | grep -q "invalid option" && \
+		_notrun "filefrag doesn't support $options option"
+	rm -f $file
+}
+
+_require_fibmap()
+{
+	_require_filefrag_options "B"
+
 	local file="$TEST_DIR/fibmap_testfile"
 
 	echo "XX" > $file
diff --git a/tests/generic/519 b/tests/generic/519
index 229c5b4..d1a3c17 100755
--- a/tests/generic/519
+++ b/tests/generic/519
@@ -33,8 +33,8 @@  rm -f $seqres.full
 _supported_fs generic
 _supported_os Linux
 _require_scratch
-_require_command "$FILEFRAG_PROG" filefrag
 _require_fibmap
+_require_filefrag_options "es"
 
 testfile="$SCRATCH_MNT/$seq-testfile"