* [PATCH v3 1/2] btrfs-progs: tests: Filter output for run_check_stdout
2020-04-07 7:18 [PATCH v3 0/2] btrfs-progs: tests: Enahance INSTRUMENT coverage Qu Wenruo
@ 2020-04-07 7:18 ` Qu Wenruo
2020-04-09 15:52 ` David Sterba
2020-04-07 7:18 ` [PATCH v3 2/2] btrfs-progs: tests: Introduce expand_command() to inject aruguments more accurately Qu Wenruo
1 sibling, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2020-04-07 7:18 UTC (permalink / raw)
To: linux-btrfs
Since run_check_stdout() can insert INSTRUMENT for all btrfs related
programs, which could easily pollute the stdout, any caller of
run_check_stdout() should do proper filter.
The following callers are affected:
- misc/004
Filter the output of "btrfs ins min-dev-size"
- misc/009
- misc/013
- misc/024
They are all calling "btrfs ins rootid", so introduce get_subvolid()
function to grab the subvolid properly.
- misc/031
Loose the filter for "btrfs qgroup show". No need for "tail -n 1".
So we still have the same coverage, but now these tests won't cause
false alert if we insert INSTRUMENT for all btrfs commands.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
tests/common | 13 ++++++++++++
tests/misc-tests/004-shrink-fs/test.sh | 11 ++++++++--
.../009-subvolume-sync-must-wait/test.sh | 2 +-
.../013-subvolume-sync-crash/test.sh | 2 +-
.../024-inspect-internal-rootid/test.sh | 21 +++++++------------
.../031-qgroup-parent-child-relation/test.sh | 4 ++--
6 files changed, 33 insertions(+), 20 deletions(-)
diff --git a/tests/common b/tests/common
index 71661e950db0..f8fc3cfd8b4f 100644
--- a/tests/common
+++ b/tests/common
@@ -169,6 +169,9 @@ run_check()
# same as run_check but the stderr+stdout output is duplicated on stdout and
# can be processed further
+#
+# NOTE: If this function is called on btrfs related command, caller should
+# filter the output, as INSTRUMENT can easily pollute the output.
run_check_stdout()
{
local spec
@@ -636,6 +639,16 @@ check_min_kernel_version()
return 0
}
+# Get subvolume id for specified path
+get_subvolid()
+{
+ # run_check_stdout may have INSTRUMENT pollating the output,
+ # need to filter the output.
+ run_check_stdout "$TOP/btrfs" inspect-internal rootid "$1" | \
+ grep -e "^[[:digit:]]\+$"
+
+}
+
# compare running kernel version to the given parameter, return success
# if running is newer than requested (let caller decide if to fail or skip)
# $1: minimum version of running kernel in major.minor format (eg. 4.19)
diff --git a/tests/misc-tests/004-shrink-fs/test.sh b/tests/misc-tests/004-shrink-fs/test.sh
index 741500634edb..7410f847a43e 100755
--- a/tests/misc-tests/004-shrink-fs/test.sh
+++ b/tests/misc-tests/004-shrink-fs/test.sh
@@ -11,11 +11,18 @@ check_prereq btrfs
setup_root_helper
+get_min_dev_size()
+{
+ size=$(run_check_stdout $SUDO_HELPER "$TOP/btrfs" inspect-internal \
+ min-dev-size ${1:+--id "$1"} "$TEST_MNT" | \
+ grep -e "^[[:digit:]]\+.*)$" | cut -d ' ' -f 1)
+ echo "$size"
+}
+
# Optionally take id of the device to shrink
shrink_test()
{
- min_size=$(run_check_stdout $SUDO_HELPER "$TOP/btrfs" inspect-internal min-dev-size ${1:+--id "$1"} "$TEST_MNT")
- min_size=$(echo "$min_size" | cut -d ' ' -f 1)
+ min_size=$(get_min_dev_size $1)
_log "min size = ${min_size}"
if [ -z "$min_size" ]; then
_fail "Failed to parse minimum size"
diff --git a/tests/misc-tests/009-subvolume-sync-must-wait/test.sh b/tests/misc-tests/009-subvolume-sync-must-wait/test.sh
index 91dcebbbebf9..9f4b49a92080 100755
--- a/tests/misc-tests/009-subvolume-sync-must-wait/test.sh
+++ b/tests/misc-tests/009-subvolume-sync-must-wait/test.sh
@@ -31,7 +31,7 @@ done
run_check $SUDO_HELPER "$TOP/btrfs" subvolume list .
run_check $SUDO_HELPER "$TOP/btrfs" subvolume list -d .
-idtodel=`run_check_stdout $SUDO_HELPER "$TOP/btrfs" inspect-internal rootid snap3`
+idtodel=$(get_subvolid snap3)
# delete, sync after some time
run_check $SUDO_HELPER "$TOP/btrfs" subvolume delete -c snap3
diff --git a/tests/misc-tests/013-subvolume-sync-crash/test.sh b/tests/misc-tests/013-subvolume-sync-crash/test.sh
index 64ba99598462..8d4e76032db5 100755
--- a/tests/misc-tests/013-subvolume-sync-crash/test.sh
+++ b/tests/misc-tests/013-subvolume-sync-crash/test.sh
@@ -32,7 +32,7 @@ done
run_check $SUDO_HELPER "$TOP/btrfs" subvolume list .
run_check $SUDO_HELPER "$TOP/btrfs" subvolume list -d .
-idtodel=`run_check_stdout $SUDO_HELPER "$TOP/btrfs" inspect-internal rootid snap3`
+idtodel=`$SUDO_HELPER "$TOP/btrfs" inspect-internal rootid snap3`
# delete, sync after some time
run_check $SUDO_HELPER "$TOP/btrfs" subvolume delete -c snap*
diff --git a/tests/misc-tests/024-inspect-internal-rootid/test.sh b/tests/misc-tests/024-inspect-internal-rootid/test.sh
index b1c804d9aedd..cb3fbc6db02c 100755
--- a/tests/misc-tests/024-inspect-internal-rootid/test.sh
+++ b/tests/misc-tests/024-inspect-internal-rootid/test.sh
@@ -21,20 +21,13 @@ run_check touch file1
run_check touch dir/file2
run_check touch sub/file3
-id1=$(run_check_stdout "$TOP/btrfs" inspect-internal rootid .) \
- || { echo $id1; exit 1; }
-id2=$(run_check_stdout "$TOP/btrfs" inspect-internal rootid sub) \
- || { echo $id2; exit 1; }
-id3=$(run_check_stdout "$TOP/btrfs" inspect-internal rootid sub/subsub) \
- || { echo $id3; exit 1; }
-id4=$(run_check_stdout "$TOP/btrfs" inspect-internal rootid dir) \
- || { echo $id4; exit 1; }
-id5=$(run_check_stdout "$TOP/btrfs" inspect-internal rootid file1) \
- || { echo $id5; exit 1; }
-id6=$(run_check_stdout "$TOP/btrfs" inspect-internal rootid dir/file2) \
- || { echo $id6; exit 1; }
-id7=$(run_check_stdout "$TOP/btrfs" inspect-internal rootid sub/file3) \
- || { echo $id7; exit 1; }
+id1=$(get_subvolid .) || { echo $id1; exit 1; }
+id2=$(get_subvolid sub) || { echo $id2; exit 1; }
+id3=$(get_subvolid sub/subsub) || { echo $id3; exit 1; }
+id4=$(get_subvolid dir) || { echo $id4; exit 1; }
+id5=$(get_subvolid file1) || { echo $id5; exit 1; }
+id6=$(get_subvolid dir/file2) || { echo $id6; exit 1; }
+id7=$(get_subvolid sub/file3) || { echo $id7; exit 1; }
if ! ([ "$id1" -ne "$id2" ] && [ "$id1" -ne "$id3" ] && [ "$id2" -ne "$id3" ]); then
_fail "inspect-internal rootid: each subvolume must have different id"
diff --git a/tests/misc-tests/031-qgroup-parent-child-relation/test.sh b/tests/misc-tests/031-qgroup-parent-child-relation/test.sh
index f85290584742..53bc953e21d6 100755
--- a/tests/misc-tests/031-qgroup-parent-child-relation/test.sh
+++ b/tests/misc-tests/031-qgroup-parent-child-relation/test.sh
@@ -18,10 +18,10 @@ run_check $SUDO_HELPER "$TOP/btrfs" qgroup assign 0/5 1/0 "$TEST_MNT"
run_check $SUDO_HELPER "$TOP/btrfs" quota rescan -w "$TEST_MNT"
run_check_stdout $SUDO_HELPER "$TOP/btrfs" qgroup show --sort=-qgroupid \
- -p "$TEST_MNT" | tail -n 1 | grep -q "1/0" \
+ -p "$TEST_MNT" | grep -q "1/0" \
|| _fail "parent qgroup check failed, please check the log"
run_check_stdout $SUDO_HELPER "$TOP/btrfs" qgroup show --sort=qgroupid \
- -c "$TEST_MNT" | tail -n 1 | grep -q "0/5" \
+ -c "$TEST_MNT" | grep -q "0/5" \
|| _fail "child qgroup check failed, please check the log"
run_check_umount_test_dev "$TEST_MNT"
--
2.26.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 2/2] btrfs-progs: tests: Introduce expand_command() to inject aruguments more accurately
2020-04-07 7:18 [PATCH v3 0/2] btrfs-progs: tests: Enahance INSTRUMENT coverage Qu Wenruo
2020-04-07 7:18 ` [PATCH v3 1/2] btrfs-progs: tests: Filter output for run_check_stdout Qu Wenruo
@ 2020-04-07 7:18 ` Qu Wenruo
1 sibling, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2020-04-07 7:18 UTC (permalink / raw)
To: linux-btrfs
[PROBLEM]
We want to inject $INSTRUMENT (mostly valgrind) before btrfs command but
after root_helper.
Currently we won't inject $INSTRUMENT at all if we are using
root_helper.
This means the coverage is not good enough.
[FIX]
This patch introduce a new function, expand_command(), to handle all
parameter/argument injection, including existing 'btrfs check' inject.
This function will:
- Detect where to inject $INSTRUMENT
If we have root_helper and the command is target command
(btrfs/mkfs.btrfs/btrfs-convert), then we inject $INSTRUMENT after
root_helper.
If we don't have root_helper, and the command is target command,
we inject $INSTRUMENT before the command.
Or we don't inject $INSTRUMENT (it's not the target command).
- Use existing spec facility to inject extra arguments
- Use an array to restore to result
To avoid bash interpret the IFS inside path/commands.
Now we can make sure no matter if we use root_helper, $INSTRUMENT is
always injected corrected.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
tests/common | 176 +++++++++++++++++++++++++--------------------------
1 file changed, 87 insertions(+), 89 deletions(-)
diff --git a/tests/common b/tests/common
index f8fc3cfd8b4f..c2c1cb9d5993 100644
--- a/tests/common
+++ b/tests/common
@@ -3,6 +3,9 @@
# Common routines for all tests
#
+# Temporary command array for building real command
+declare -a cmd_array
+
# assert that argument is not empty and is an existing path (file or directory)
_assert_path()
{
@@ -135,6 +138,60 @@ _cmd_spec()
fi
}
+_is_target_command()
+{
+ [[ $1 =~ /btrfs$ ]] && return 0
+ [[ $1 =~ /mkfs.btrfs$ ]] && return 0
+ [[ $1 =~ /btrfs-convert$ ]] && return 0
+ [[ $1 =~ /btrfs-corrupt-block$ ]] && return 0
+ [[ $1 =~ /btrfs-image$ ]] && return 0
+ [[ $1 =~ /btrfs-select-super$ ]] && return 0
+ [[ $1 =~ /btrfs-find-root$ ]] && return 0
+ [[ $1 =~ /btrfstune$ ]] && return 0
+ return 1
+}
+
+# Expanding extra commands/options for current command string
+# This function is responsible for inserting the following things:
+# - @INSTRUMENT before btrfs commands
+# To ensure that @INSTRUMENT is not executed for things like sudo/mount
+# which normally has setuid/setgid which will not be traced.
+#
+# - Add extra arguments for 'btrfs check'/'mkfs.btrfs'/'btrfs-convert'
+# This allows us to test certain function like lowmem mode for btrfs check
+expand_command()
+{
+ local command_index
+ local ins
+ local spec
+ local i
+
+ ins=$(_get_spec_ins "$@")
+ spec=$(($ins-1))
+ spec=$(_cmd_spec "${@:$spec}")
+ cmd_array=()
+
+ if [ "$1" = 'root_helper' ] && _is_target_command "$2" ; then
+ command_index=2
+ elif _is_target_command "$1" ; then
+ command_index=1
+ else
+ # Since we iterate from offset 1, this never get hit,
+ # thus we won't insert $INSTRUMENT
+ command_index=0
+ fi
+
+ for ((i = 1; i <= $#; i++ )); do
+ if [ $i -eq $command_index -a ! -z "$INSTRUMENT" ]; then
+ cmd_array+=($INSTRUMENT)
+ fi
+ if [ $i -eq $ins -a ! -z "$spec" ]; then
+ cmd_array+=("$spec")
+ fi
+ cmd_array+=("${!i}")
+ done
+}
+
# Argument passing magic:
# the command passed to run_* helpers is inspected, if there's 'btrfs command'
# found and there are defined additional arguments, they're inserted just after
@@ -145,26 +202,11 @@ _cmd_spec()
run_check()
{
- local spec
- local ins
+ expand_command "$@"
+ echo "====== RUN CHECK ${cmd_array[@]}" >> "$RESULTS" 2>&1
+ if [[ $TEST_LOG =~ tty ]]; then echo "CMD: ${cmd_array[@]}" > /dev/tty; fi
- ins=$(_get_spec_ins "$@")
- spec=$(($ins-1))
- spec=$(_cmd_spec "${@:$spec}")
- set -- "${@:1:$(($ins-1))}" $spec "${@: $ins}"
- echo "====== RUN CHECK $@" >> "$RESULTS" 2>&1
- if [[ $TEST_LOG =~ tty ]]; then echo "CMD: $@" > /dev/tty; fi
-
- # If the command is `root_helper` or mount/umount, don't call INSTRUMENT
- # as most profiling tool like valgrind can't handle setuid/setgid/setcap
- # which mount normally has.
- # And since mount/umount is only called with run_check(), we don't need
- # to do the same check on other run_*() functions.
- if [ "$1" = 'root_helper' -o "$1" = 'mount' -o "$1" = 'umount' ]; then
- "$@" >> "$RESULTS" 2>&1 || _fail "failed: $@"
- else
- $INSTRUMENT "$@" >> "$RESULTS" 2>&1 || _fail "failed: $@"
- fi
+ "${cmd_array[@]}" >> "$RESULTS" 2>&1 || _fail "failed: ${cmd_array[@]}"
}
# same as run_check but the stderr+stdout output is duplicated on stdout and
@@ -174,20 +216,11 @@ run_check()
# filter the output, as INSTRUMENT can easily pollute the output.
run_check_stdout()
{
- local spec
- local ins
-
- ins=$(_get_spec_ins "$@")
- spec=$(($ins-1))
- spec=$(_cmd_spec "${@:$spec}")
- set -- "${@:1:$(($ins-1))}" $spec "${@: $ins}"
- echo "====== RUN CHECK $@" >> "$RESULTS" 2>&1
- if [[ $TEST_LOG =~ tty ]]; then echo "CMD(stdout): $@" > /dev/tty; fi
- if [ "$1" = 'root_helper' ]; then
- "$@" 2>&1 | tee -a "$RESULTS"
- else
- $INSTRUMENT "$@" 2>&1 | tee -a "$RESULTS"
- fi
+ expand_command "$@"
+ echo "====== RUN CHECK ${cmd_array[@]}" >> "$RESULTS" 2>&1
+ if [[ $TEST_LOG =~ tty ]]; then echo "CMD(stdout): ${cmd_array[@]}" \
+ > /dev/tty; fi
+ "${cmd_array[@]}" 2>&1 | tee -a "$RESULTS"
if [ ${PIPESTATUS[0]} -ne 0 ]; then
_fail "failed: $@"
fi
@@ -198,21 +231,11 @@ run_check_stdout()
# output is logged
run_mayfail()
{
- local spec
- local ins
- local ret
-
- ins=$(_get_spec_ins "$@")
- spec=$(($ins-1))
- spec=$(_cmd_spec "${@:$spec}")
- set -- "${@:1:$(($ins-1))}" $spec "${@: $ins}"
- echo "====== RUN MAYFAIL $@" >> "$RESULTS" 2>&1
- if [[ $TEST_LOG =~ tty ]]; then echo "CMD(mayfail): $@" > /dev/tty; fi
- if [ "$1" = 'root_helper' ]; then
- "$@" >> "$RESULTS" 2>&1
- else
- $INSTRUMENT "$@" >> "$RESULTS" 2>&1
- fi
+ expand_command "$@"
+ echo "====== RUN MAYFAIL ${cmd_array[@]}" >> "$RESULTS" 2>&1
+ if [[ $TEST_LOG =~ tty ]]; then echo "CMD(mayfail): ${cmd_array[@]}" \
+ > /dev/tty; fi
+ "${cmd_array[@]}" >> "$RESULTS" 2>&1
ret=$?
if [ $ret != 0 ]; then
echo "failed (ignored, ret=$ret): $@" >> "$RESULTS"
@@ -234,23 +257,13 @@ run_mayfail()
# store the output to a variable for further processing.
run_mayfail_stdout()
{
- local spec
- local ins
- local ret
-
tmp_output=$(mktemp --tmpdir btrfs-progs-test--mayfail-stdtout.XXXXXX)
- ins=$(_get_spec_ins "$@")
- spec=$(($ins-1))
- spec=$(_cmd_spec "${@:$spec}")
- set -- "${@:1:$(($ins-1))}" $spec "${@: $ins}"
- echo "====== RUN MAYFAIL $@" >> "$RESULTS" 2>&1
- if [[ $TEST_LOG =~ tty ]]; then echo "CMD(mayfail): $@" > /dev/tty; fi
- if [ "$1" = 'root_helper' ]; then
- "$@" 2>&1 > "$tmp_output"
- else
- $INSTRUMENT "$@" 2>&1 > "$tmp_output"
- fi
+ expand_command "$@"
+ echo "====== RUN MAYFAIL ${cmd_array[@]}" >> "$RESULTS" 2>&1
+ if [[ $TEST_LOG =~ tty ]]; then echo "CMD(mayfail): ${cmd_array[@]}" \
+ > /dev/tty; fi
+ "${cmd_array[@]}" 2>&1 > "$tmp_output"
ret=$?
cat "$tmp_output" >> "$RESULTS"
@@ -275,8 +288,6 @@ run_mayfail_stdout()
# same as run_check but expects the command to fail, output is logged
run_mustfail()
{
- local spec
- local ins
local msg
msg="$1"
@@ -287,17 +298,12 @@ run_mustfail()
exit 1
fi
- ins=$(_get_spec_ins "$@")
- spec=$(($ins-1))
- spec=$(_cmd_spec "${@:$spec}")
- set -- "${@:1:$(($ins-1))}" $spec "${@: $ins}"
- echo "====== RUN MUSTFAIL $@" >> "$RESULTS" 2>&1
- if [[ $TEST_LOG =~ tty ]]; then echo "CMD(mustfail): $@" > /dev/tty; fi
- if [ "$1" = 'root_helper' ]; then
- "$@" >> "$RESULTS" 2>&1
- else
- $INSTRUMENT "$@" >> "$RESULTS" 2>&1
- fi
+
+ expand_command "$@"
+ echo "====== RUN MUSTFAIL ${cmd_array[@]}" >> "$RESULTS" 2>&1
+ if [[ $TEST_LOG =~ tty ]]; then echo "CMD(mustfail): ${cmd_array[@]}" \
+ > /dev/tty; fi
+ "${cmd_array[@]}" >> "$RESULTS" 2>&1
if [ $? != 0 ]; then
echo "failed (expected): $@" >> "$RESULTS"
return 0
@@ -315,8 +321,6 @@ run_mustfail()
# So it doesn't support pipeline in the @cmd
run_mustfail_stdout()
{
- local spec
- local ins
local msg
local ret
local tmp_output
@@ -331,17 +335,11 @@ run_mustfail_stdout()
exit 1
fi
- ins=$(_get_spec_ins "$@")
- spec=$(($ins-1))
- spec=$(_cmd_spec "${@:$spec}")
- set -- "${@:1:$(($ins-1))}" $spec "${@: $ins}"
- echo "====== RUN MUSTFAIL $@" >> "$RESULTS" 2>&1
- if [[ $TEST_LOG =~ tty ]]; then echo "CMD(mustfail): $@" > /dev/tty; fi
- if [ "$1" = 'root_helper' ]; then
- "$@" 2>&1 > "$tmp_output"
- else
- $INSTRUMENT "$@" 2>&1 > "$tmp_output"
- fi
+ expand_command "$@"
+ echo "====== RUN MUSTFAIL ${cmd_array[@]}" >> "$RESULTS" 2>&1
+ if [[ $TEST_LOG =~ tty ]]; then echo "CMD(mustfail): ${cmd_array[@]}" \
+ > /dev/tty; fi
+ "${cmd_array[@]}" 2>&1 > "$tmp_output"
ret=$?
cat "$tmp_output" >> "$RESULTS"
--
2.26.0
^ permalink raw reply related [flat|nested] 6+ messages in thread