* [PATCH 1/2] fstests: common: rename and enhance _require_btrfs to _require_btrfs_subcommand @ 2016-12-08 2:04 Qu Wenruo 2016-12-08 2:04 ` [PATCH 2/2] fstests: btrfs: Use _require_btrfs_qgroup_report to replace open code Qu Wenruo 2016-12-08 4:00 ` [PATCH 1/2] fstests: common: rename and enhance _require_btrfs to _require_btrfs_subcommand Eryu Guan 0 siblings, 2 replies; 6+ messages in thread From: Qu Wenruo @ 2016-12-08 2:04 UTC (permalink / raw) To: linux-btrfs, fstests Rename _require_btrfs() to _require_btrfs_subcommand() to avoid confusion, as all other _require_btrfs_* has a quite clear suffix, like _require_btrfs_mkfs_feature() or _require_btrfs_fs_feature(). Also enhance _require_btrfs_subcommand() to accept 2nd level commands or options. Options will be determined by the first "-" char. This is quite useful for case like "btrfs inspect-internal dump-tree" and "btrfs check --qgroup-report". Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> --- common/rc | 29 ++++++++++++++++++++++++----- tests/btrfs/004 | 3 ++- tests/btrfs/048 | 2 +- tests/btrfs/059 | 2 +- tests/btrfs/131 | 2 +- 5 files changed, 29 insertions(+), 9 deletions(-) diff --git a/common/rc b/common/rc index 8c99306..1703232 100644 --- a/common/rc +++ b/common/rc @@ -3019,15 +3019,34 @@ _require_deletable_scratch_dev_pool() } # We check for btrfs and (optionally) features of the btrfs command -_require_btrfs() +# _require_btrfs_subcommand <subcommand> [<subfunction>|<option>] +# It can handle both subfunction like "inspect-internal dump-tree" +# and options like "check --qgroup-report" +_require_btrfs_subcommand() { - cmd=$1 - _require_command "$BTRFS_UTIL_PROG" btrfs if [ -z "$1" ]; then - return 1; + echo "Usage: _require_btrfs_subcommand command [subcommand]" 1>&2 + exit 1 fi - $BTRFS_UTIL_PROG $cmd --help >/dev/null 2>&1 + cmd=$1 + param=$2 + + _require_command "$BTRFS_UTIL_PROG" btrfs + $BTRFS_UTIL_PROG $cmd --help &>/dev/null [ $? -eq 0 ] || _notrun "$BTRFS_UTIL_PROG too old (must support $cmd)" + + test -z "$param" && return + + # if $param is an option, replace leading "-"s for grep + if [ ${param:0:1} == "-" ]; then + param=$(echo $param | sed 's/^-*//') + $BTRFS_UTIL_PROG $cmd --help | grep $param > /dev/null || \ + _not_run "$BTRFS_UTIL_PROG too old (must support $cmd $param)" + return + fi + + $BTRFS_UTIL_PROG $cmd $param --help &>/dev/null + [ $? -eq 0 ] || _notrun "$BTRFS_UTIL_PROG too old (must support $cmd $param)" } # Check that fio is present, and it is able to execute given jobfile diff --git a/tests/btrfs/004 b/tests/btrfs/004 index 905770a..e60a034 100755 --- a/tests/btrfs/004 +++ b/tests/btrfs/004 @@ -51,7 +51,8 @@ _supported_fs btrfs _supported_os Linux _require_scratch _require_no_large_scratch_dev -_require_btrfs inspect-internal +_require_btrfs_subcommand inspect-internal logical-resolve +_require_btrfs_subcommand inspect-internal inode-resolve _require_command "/usr/sbin/filefrag" filefrag rm -f $seqres.full diff --git a/tests/btrfs/048 b/tests/btrfs/048 index 0b907b0..ac731d1 100755 --- a/tests/btrfs/048 +++ b/tests/btrfs/048 @@ -48,7 +48,7 @@ _supported_fs btrfs _supported_os Linux _require_test _require_scratch -_require_btrfs "property" +_require_btrfs_subcommand "property" send_files_dir=$TEST_DIR/btrfs-test-$seq diff --git a/tests/btrfs/059 b/tests/btrfs/059 index 8f106d2..fd67ebb 100755 --- a/tests/btrfs/059 +++ b/tests/btrfs/059 @@ -51,7 +51,7 @@ _supported_fs btrfs _supported_os Linux _require_test _require_scratch -_require_btrfs "property" +_require_btrfs_subcommand "property" rm -f $seqres.full diff --git a/tests/btrfs/131 b/tests/btrfs/131 index d1a11d2..d7c7f12 100755 --- a/tests/btrfs/131 +++ b/tests/btrfs/131 @@ -48,7 +48,7 @@ rm -f $seqres.full _supported_fs btrfs _supported_os Linux _require_scratch -_require_btrfs inspect-internal +_require_btrfs_subcommand inspect-internal dump-super mkfs_v1() { -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] fstests: btrfs: Use _require_btrfs_qgroup_report to replace open code 2016-12-08 2:04 [PATCH 1/2] fstests: common: rename and enhance _require_btrfs to _require_btrfs_subcommand Qu Wenruo @ 2016-12-08 2:04 ` Qu Wenruo 2016-12-08 5:27 ` Eryu Guan 2016-12-08 4:00 ` [PATCH 1/2] fstests: common: rename and enhance _require_btrfs to _require_btrfs_subcommand Eryu Guan 1 sibling, 1 reply; 6+ messages in thread From: Qu Wenruo @ 2016-12-08 2:04 UTC (permalink / raw) To: linux-btrfs, fstests Introduce new _require_btrfs_qgroup_report function, which will check the accessibility to "btrfs check --qgroup-report", then set a global flag to info _check_scratch_fs() to do extra qgroup check. Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> --- common/rc | 22 ++++++++++++++++++++++ tests/btrfs/022 | 5 +++++ tests/btrfs/028 | 5 ++--- tests/btrfs/042 | 6 ++---- tests/btrfs/099 | 1 + tests/btrfs/104 | 20 +++++--------------- tests/btrfs/122 | 10 +++------- tests/btrfs/123 | 5 ++--- 8 files changed, 42 insertions(+), 32 deletions(-) diff --git a/common/rc b/common/rc index 1703232..bce3a09 100644 --- a/common/rc +++ b/common/rc @@ -2624,6 +2624,20 @@ _check_btrfs_filesystem() mountpoint=`_umount_or_remount_ro $device` fi + # Check qgroup numbers + if [ "$BTRFS_NEED_QGROUP_REPORT" == "yes" ];then + btrfsck $device --qgroup-report > $tmp.qgroup_report 2>&1 + if grep -qE "Counts for qgroup.*are different" $tmp.qgroup_report ; then + echo "_check_btrfs_filesystem: filesystem on $device has wrong qgroup numbers (see $seqres.full)" + echo "_check_btrfs_filesystem: filesystem on $device has wrong qgroup numbers" \ + >> $seqres.full + echo "*** qgroup_report.$FSTYP output ***" >>$seqres.full + cat $tmp.qgroup_report >>$seqres.full + echo "*** qgroup_report.$FSTYP output ***" >>$seqres.full + fi + fi + rm -f $tmp.qgroup_report + btrfsck $device >$tmp.fsck 2>&1 if [ $? -ne 0 ] then @@ -3049,6 +3063,14 @@ _require_btrfs_subcommand() [ $? -eq 0 ] || _notrun "$BTRFS_UTIL_PROG too old (must support $cmd $param)" } +# Require "btrfs check --qgroup-report" fucntion and will check the qgroup +# numbers at _check_scratch_fs() +_require_btrfs_qgroup_report() +{ + _require_btrfs_subcommand check --qgroup-report + export BTRFS_NEED_QGROUP_REPORT="yes" +} + # Check that fio is present, and it is able to execute given jobfile _require_fio() { diff --git a/tests/btrfs/022 b/tests/btrfs/022 index 56d4f3d..2f21a78 100755 --- a/tests/btrfs/022 +++ b/tests/btrfs/022 @@ -43,6 +43,7 @@ _cleanup() _supported_fs btrfs _supported_os Linux _require_scratch +_require_btrfs_qgroup_report rm -f $seqres.full @@ -125,20 +126,24 @@ _scratch_mkfs > /dev/null 2>&1 _scratch_mount _basic_test _scratch_unmount +_check_scratch_fs _scratch_mkfs > /dev/null 2>&1 _scratch_mount _rescan_test _scratch_unmount +_check_scratch_fs _scratch_mkfs > /dev/null 2>&1 _scratch_mount _limit_test_exceed _scratch_unmount +_check_scratch_fs _scratch_mkfs > /dev/null 2>&1 _scratch_mount _limit_test_noexceed +_check_scratch_fs # success, all done echo "Silence is golden" diff --git a/tests/btrfs/028 b/tests/btrfs/028 index 1425609..a3d9a27 100755 --- a/tests/btrfs/028 +++ b/tests/btrfs/028 @@ -51,6 +51,7 @@ rm -f $seqres.full _supported_fs btrfs _supported_os Linux _require_scratch +_require_btrfs_qgroup_report _scratch_mkfs _scratch_mount @@ -86,9 +87,7 @@ _run_btrfs_util_prog filesystem sync $SCRATCH_MNT _scratch_unmount -# generate a qgroup report and look for inconsistent groups -$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \ - grep -E "Counts for qgroup.*are different" +# qgroup will be checked at _check_scratch_fs() by fstest. echo "Silence is golden" status=0 diff --git a/tests/btrfs/042 b/tests/btrfs/042 index 498ccc9..dc9b762 100755 --- a/tests/btrfs/042 +++ b/tests/btrfs/042 @@ -43,6 +43,7 @@ _cleanup() _supported_fs btrfs _supported_os Linux _require_scratch +_require_btrfs_qgroup_report rm -f $seqres.full @@ -84,10 +85,7 @@ for i in `seq 10 -1 1`; do total_written=$(($total_written+$filesize)) done -#check if total written exceeds limit -if [ $total_written -gt $LIMIT_SIZE ];then - _fail "total written should be less than $LIMIT_SIZE" -fi +# qgroup will be checked automatically at _check_scratch_fs() by fstest # success, all done echo "Silence is golden" diff --git a/tests/btrfs/099 b/tests/btrfs/099 index 70f07b5..65ea79b 100755 --- a/tests/btrfs/099 +++ b/tests/btrfs/099 @@ -46,6 +46,7 @@ _cleanup() _supported_fs btrfs _supported_os Linux _require_scratch +_require_btrfs_qgroup_report # Use big blocksize to ensure there is still enough space left for metadata # space reserve. diff --git a/tests/btrfs/104 b/tests/btrfs/104 index 6afaa02..e6a6d3b 100755 --- a/tests/btrfs/104 +++ b/tests/btrfs/104 @@ -58,6 +58,7 @@ rm -f $seqres.full _supported_fs btrfs _supported_os Linux _require_scratch +_require_btrfs_qgroup_report rm -f $seqres.full @@ -145,21 +146,10 @@ _scratch_cycle_mount # referenced above. _run_btrfs_util_prog subvolume delete $SCRATCH_MNT/snap1 -# There is no way from userspace to force btrfs_drop_snapshot to run -# at a given time (even via mount/unmount). We must wait for it to -# start and complete. This is the shortest time on my tests systems I -# have found which always allows drop_snapshot to run to completion. -sleep 45 +# "btrfs filesystem sync" will trigger subvolume deletion +_run_btrfs_util_prog filesystem sync $SCRATCH_MNT -_scratch_unmount - -# generate a qgroup report and look for inconsistent groups -# - don't use _run_btrfs_util_prog here as it captures the output and -# we need to grep it. -$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \ - grep -E -q "Counts for qgroup.*are different" -if [ $? -ne 0 ]; then - status=0 -fi +# Qgroup will be checked by fstest at _check_scratch_fs() +status=0 exit diff --git a/tests/btrfs/122 b/tests/btrfs/122 index 82252ab..212571f 100755 --- a/tests/btrfs/122 +++ b/tests/btrfs/122 @@ -49,6 +49,7 @@ rm -f $seqres.full _supported_fs btrfs _supported_os Linux _require_scratch +_require_btrfs_qgroup_report rm -f $seqres.full @@ -78,11 +79,6 @@ _run_btrfs_util_prog subvolume snapshot $SCRATCH_MNT "$SCRATCH_MNT/snaps/snap2" _scratch_unmount -# generate a qgroup report and look for inconsistent groups -$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \ - grep -q -E "Counts for qgroup.*are different" -if [ $? -ne 0 ]; then - status=0 -fi - +# qgroup will be checked by fstest at _check_scratch_fs() +status=0 exit diff --git a/tests/btrfs/123 b/tests/btrfs/123 index e89d541..52d87fb 100755 --- a/tests/btrfs/123 +++ b/tests/btrfs/123 @@ -53,6 +53,7 @@ rm -f $seqres.full _supported_fs btrfs _supported_os Linux _require_scratch +_require_btrfs_qgroup_report _scratch_mkfs # Need to use inline extents to fill metadata rapidly @@ -76,9 +77,7 @@ _run_btrfs_util_prog quota rescan -w $SCRATCH_MNT _run_btrfs_util_prog balance start -d $SCRATCH_MNT _scratch_unmount -# generate a qgroup report and look for inconsistent groups -$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \ - grep -E "Counts for qgroup.*are different" +# qgroup will be check at _check_scratch_fs() by fstest # success, all done status=0 -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] fstests: btrfs: Use _require_btrfs_qgroup_report to replace open code 2016-12-08 2:04 ` [PATCH 2/2] fstests: btrfs: Use _require_btrfs_qgroup_report to replace open code Qu Wenruo @ 2016-12-08 5:27 ` Eryu Guan 2016-12-08 5:49 ` Qu Wenruo 0 siblings, 1 reply; 6+ messages in thread From: Eryu Guan @ 2016-12-08 5:27 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs, fstests On Thu, Dec 08, 2016 at 10:04:56AM +0800, Qu Wenruo wrote: > Introduce new _require_btrfs_qgroup_report function, which will check > the accessibility to "btrfs check --qgroup-report", then set a global > flag to info _check_scratch_fs() to do extra qgroup check. > > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> > --- > common/rc | 22 ++++++++++++++++++++++ This needs rebase too. > tests/btrfs/022 | 5 +++++ > tests/btrfs/028 | 5 ++--- > tests/btrfs/042 | 6 ++---- > tests/btrfs/099 | 1 + > tests/btrfs/104 | 20 +++++--------------- > tests/btrfs/122 | 10 +++------- > tests/btrfs/123 | 5 ++--- > 8 files changed, 42 insertions(+), 32 deletions(-) > > diff --git a/common/rc b/common/rc > index 1703232..bce3a09 100644 > --- a/common/rc > +++ b/common/rc > @@ -2624,6 +2624,20 @@ _check_btrfs_filesystem() > mountpoint=`_umount_or_remount_ro $device` > fi > > + # Check qgroup numbers > + if [ "$BTRFS_NEED_QGROUP_REPORT" == "yes" ];then So we can bypass the _require_btrfs_qgroup_report check if we set BTRFS_NEED_QGROUP_REPORT to "yes" directly, right? How about doing something like _require_scratch do, e.g. touching some signal file in $RESULT_DIR and only do qgroup check if that file exists? > + btrfsck $device --qgroup-report > $tmp.qgroup_report 2>&1 Shouldn't "$BTRFS_UTIL_PROG check $device ..." be used for new code? I might be wrong on this, I think btrfsck is deprecated. Thanks, Eryu ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] fstests: btrfs: Use _require_btrfs_qgroup_report to replace open code 2016-12-08 5:27 ` Eryu Guan @ 2016-12-08 5:49 ` Qu Wenruo 0 siblings, 0 replies; 6+ messages in thread From: Qu Wenruo @ 2016-12-08 5:49 UTC (permalink / raw) To: Eryu Guan; +Cc: linux-btrfs, fstests At 12/08/2016 01:27 PM, Eryu Guan wrote: > On Thu, Dec 08, 2016 at 10:04:56AM +0800, Qu Wenruo wrote: >> Introduce new _require_btrfs_qgroup_report function, which will check >> the accessibility to "btrfs check --qgroup-report", then set a global >> flag to info _check_scratch_fs() to do extra qgroup check. >> >> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> >> --- >> common/rc | 22 ++++++++++++++++++++++ > > This needs rebase too. > >> tests/btrfs/022 | 5 +++++ >> tests/btrfs/028 | 5 ++--- >> tests/btrfs/042 | 6 ++---- >> tests/btrfs/099 | 1 + >> tests/btrfs/104 | 20 +++++--------------- >> tests/btrfs/122 | 10 +++------- >> tests/btrfs/123 | 5 ++--- >> 8 files changed, 42 insertions(+), 32 deletions(-) >> >> diff --git a/common/rc b/common/rc >> index 1703232..bce3a09 100644 >> --- a/common/rc >> +++ b/common/rc >> @@ -2624,6 +2624,20 @@ _check_btrfs_filesystem() >> mountpoint=`_umount_or_remount_ro $device` >> fi >> >> + # Check qgroup numbers >> + if [ "$BTRFS_NEED_QGROUP_REPORT" == "yes" ];then > > So we can bypass the _require_btrfs_qgroup_report check if we set > BTRFS_NEED_QGROUP_REPORT to "yes" directly, right? How about doing > something like _require_scratch do, e.g. touching some signal file in > $RESULT_DIR and only do qgroup check if that file exists? Nice idea. > >> + btrfsck $device --qgroup-report > $tmp.qgroup_report 2>&1 > > Shouldn't "$BTRFS_UTIL_PROG check $device ..." be used for new code? I > might be wrong on this, I think btrfsck is deprecated. Oh, the code in common/btrfs is just too old. I'll update them together in next version. Thanks, Qu > > Thanks, > Eryu > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] fstests: common: rename and enhance _require_btrfs to _require_btrfs_subcommand 2016-12-08 2:04 [PATCH 1/2] fstests: common: rename and enhance _require_btrfs to _require_btrfs_subcommand Qu Wenruo 2016-12-08 2:04 ` [PATCH 2/2] fstests: btrfs: Use _require_btrfs_qgroup_report to replace open code Qu Wenruo @ 2016-12-08 4:00 ` Eryu Guan 2016-12-08 5:44 ` Qu Wenruo 1 sibling, 1 reply; 6+ messages in thread From: Eryu Guan @ 2016-12-08 4:00 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs, fstests On Thu, Dec 08, 2016 at 10:04:55AM +0800, Qu Wenruo wrote: > Rename _require_btrfs() to _require_btrfs_subcommand() to avoid > confusion, as all other _require_btrfs_* has a quite clear suffix, like > _require_btrfs_mkfs_feature() or _require_btrfs_fs_feature(). > > Also enhance _require_btrfs_subcommand() to accept 2nd level commands or > options. > Options will be determined by the first "-" char. > This is quite useful for case like "btrfs inspect-internal dump-tree" > and "btrfs check --qgroup-report". > > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> > --- > common/rc | 29 ++++++++++++++++++++++++----- Can you rebase on top of current master please? We've moved btrfs-specific functions to common/btrfs > tests/btrfs/004 | 3 ++- > tests/btrfs/048 | 2 +- > tests/btrfs/059 | 2 +- > tests/btrfs/131 | 2 +- > 5 files changed, 29 insertions(+), 9 deletions(-) > > diff --git a/common/rc b/common/rc > index 8c99306..1703232 100644 > --- a/common/rc > +++ b/common/rc > @@ -3019,15 +3019,34 @@ _require_deletable_scratch_dev_pool() > } > > # We check for btrfs and (optionally) features of the btrfs command > -_require_btrfs() > +# _require_btrfs_subcommand <subcommand> [<subfunction>|<option>] > +# It can handle both subfunction like "inspect-internal dump-tree" > +# and options like "check --qgroup-report" > +_require_btrfs_subcommand() I'd prefer a name similar to _require_xfs_io_command, e.g. _require_btrfs_command, "subcommand" seems not necessary to me. > { > - cmd=$1 > - _require_command "$BTRFS_UTIL_PROG" btrfs > if [ -z "$1" ]; then > - return 1; > + echo "Usage: _require_btrfs_subcommand command [subcommand]" 1>&2 > + exit 1 > fi > - $BTRFS_UTIL_PROG $cmd --help >/dev/null 2>&1 > + cmd=$1 > + param=$2 > + > + _require_command "$BTRFS_UTIL_PROG" btrfs > + $BTRFS_UTIL_PROG $cmd --help &>/dev/null > [ $? -eq 0 ] || _notrun "$BTRFS_UTIL_PROG too old (must support $cmd)" > + > + test -z "$param" && return > + > + # if $param is an option, replace leading "-"s for grep > + if [ ${param:0:1} == "-" ]; then > + param=$(echo $param | sed 's/^-*//') > + $BTRFS_UTIL_PROG $cmd --help | grep $param > /dev/null || \ Use "grep -w" to be safer? And "-q" instead of "> /dev/null" > + _not_run "$BTRFS_UTIL_PROG too old (must support $cmd $param)" $param here is without leading "-", so the _notrun message is kind of misleading. And _notrun not _not_run :) Thanks, Eryu ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] fstests: common: rename and enhance _require_btrfs to _require_btrfs_subcommand 2016-12-08 4:00 ` [PATCH 1/2] fstests: common: rename and enhance _require_btrfs to _require_btrfs_subcommand Eryu Guan @ 2016-12-08 5:44 ` Qu Wenruo 0 siblings, 0 replies; 6+ messages in thread From: Qu Wenruo @ 2016-12-08 5:44 UTC (permalink / raw) To: Eryu Guan; +Cc: linux-btrfs, fstests At 12/08/2016 12:00 PM, Eryu Guan wrote: > On Thu, Dec 08, 2016 at 10:04:55AM +0800, Qu Wenruo wrote: >> Rename _require_btrfs() to _require_btrfs_subcommand() to avoid >> confusion, as all other _require_btrfs_* has a quite clear suffix, like >> _require_btrfs_mkfs_feature() or _require_btrfs_fs_feature(). >> >> Also enhance _require_btrfs_subcommand() to accept 2nd level commands or >> options. >> Options will be determined by the first "-" char. >> This is quite useful for case like "btrfs inspect-internal dump-tree" >> and "btrfs check --qgroup-report". >> >> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> >> --- >> common/rc | 29 ++++++++++++++++++++++++----- > > Can you rebase on top of current master please? We've moved > btrfs-specific functions to common/btrfs Finally！ Good news. > >> tests/btrfs/004 | 3 ++- >> tests/btrfs/048 | 2 +- >> tests/btrfs/059 | 2 +- >> tests/btrfs/131 | 2 +- >> 5 files changed, 29 insertions(+), 9 deletions(-) >> >> diff --git a/common/rc b/common/rc >> index 8c99306..1703232 100644 >> --- a/common/rc >> +++ b/common/rc >> @@ -3019,15 +3019,34 @@ _require_deletable_scratch_dev_pool() >> } >> >> # We check for btrfs and (optionally) features of the btrfs command >> -_require_btrfs() >> +# _require_btrfs_subcommand <subcommand> [<subfunction>|<option>] >> +# It can handle both subfunction like "inspect-internal dump-tree" >> +# and options like "check --qgroup-report" >> +_require_btrfs_subcommand() > > I'd prefer a name similar to _require_xfs_io_command, e.g. > _require_btrfs_command, "subcommand" seems not necessary to me. > Right, the subcommand seems not that handy compared to other _require_*. >> { >> - cmd=$1 >> - _require_command "$BTRFS_UTIL_PROG" btrfs >> if [ -z "$1" ]; then >> - return 1; >> + echo "Usage: _require_btrfs_subcommand command [subcommand]" 1>&2 >> + exit 1 >> fi >> - $BTRFS_UTIL_PROG $cmd --help >/dev/null 2>&1 >> + cmd=$1 >> + param=$2 >> + >> + _require_command "$BTRFS_UTIL_PROG" btrfs >> + $BTRFS_UTIL_PROG $cmd --help &>/dev/null >> [ $? -eq 0 ] || _notrun "$BTRFS_UTIL_PROG too old (must support $cmd)" >> + >> + test -z "$param" && return >> + >> + # if $param is an option, replace leading "-"s for grep >> + if [ ${param:0:1} == "-" ]; then >> + param=$(echo $param | sed 's/^-*//') >> + $BTRFS_UTIL_PROG $cmd --help | grep $param > /dev/null || \ > > Use "grep -w" to be safer? And "-q" instead of "> /dev/null" Right, -w is much safer. I'll use "-q" in next version. > >> + _not_run "$BTRFS_UTIL_PROG too old (must support $cmd $param)" > > $param here is without leading "-", so the _notrun message is kind of > misleading. And _notrun not _not_run :) Right, I'll using safe_param. Thanks, Qu > > Thanks, > Eryu > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-08 5:50 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-12-08 2:04 [PATCH 1/2] fstests: common: rename and enhance _require_btrfs to _require_btrfs_subcommand Qu Wenruo 2016-12-08 2:04 ` [PATCH 2/2] fstests: btrfs: Use _require_btrfs_qgroup_report to replace open code Qu Wenruo 2016-12-08 5:27 ` Eryu Guan 2016-12-08 5:49 ` Qu Wenruo 2016-12-08 4:00 ` [PATCH 1/2] fstests: common: rename and enhance _require_btrfs to _require_btrfs_subcommand Eryu Guan 2016-12-08 5:44 ` Qu Wenruo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox