Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions case-lib/hijack.sh
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,17 @@ function func_exit_handler()
logfile="$logfile"

local loggerBin wcLog; loggerBin=$(basename "$SOFLOGGER")
# We need this to avoid the confusion of a "Terminated" message
local cavstoolBin=cavstool.py

# We need this dlogi to avoid the confusion of a "Terminated" message
# without context.
if is_zephyr; then
dlogi "pkill -TERM -f $cavstoolBin"
sudo pkill -TERM -f "$cavstoolBin" || {
dloge "cavstool.py was already dead"
exit_status=1
}
fi
dlogi "pkill -TERM $loggerBin"
sudo pkill -TERM "$loggerBin" || {
dloge "sof-logger was already dead"
Expand All @@ -76,11 +85,16 @@ function func_exit_handler()
fi
}
sleep 1s
if pgrep "$loggerBin"; then
dloge "$loggerBin resisted pkill -TERM, using -KILL"
sudo pkill -KILL "$loggerBin"
exit_status=1
fi

local logtool
for logtool in "$cavstoolBin" "$loggerBin"
do
if pgrep -f "$logtool"; then
dloge "$logtool resisted pkill -TERM, using -KILL"
sudo pkill -KILL -f "$logtool"
exit_status=1
fi
done

if test -e "$logfile"; then

Expand Down
30 changes: 30 additions & 0 deletions case-lib/lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,25 @@ find_ldc_file()
printf '%s' "$ldcFile"
}

func_cavstool_etrace_collect()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment / header documenting important control flow expectations

  • whether this function leaves anything running in the background. Any timeout.
  • what the caller is expected to clean up and when

{
local clogopt="--log-only --verbose"
local clogfile=$LOG_ROOT/etrace.txt

if [ -z "$CAVSTOOL" ]; then
CAVSTOOL=$(command -v cavstool.py) || {
dlogw 'No cavstool.py found in PATH'
return 1
}
fi

local cavstoolCmd=("$CAVSTOOL" "$clogopt")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
local cavstoolCmd=("$CAVSTOOL" "$clogopt")
local cavstoolCmd=("$CAVSTOOL" --log-only --verbose)

Otherwise this tries to load a firmware file named '--log-only --verbose'

I just wasted a couple of hours on this... already asked to inline this pointless variable before (when it was a single option and working)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I realize only I see this because I disable the sof-test sudo function, I'm using the regular sudo command.

Should still be fixed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please help review and merge this sudo() fix first:

dlogi "Starting ${cavstoolCmd[*]}"
# Cleaned up by func_exit_handler() in hijack.sh
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleanup should be in the same commit as this comment.

# shellcheck disable=SC2024
sudo "${cavstoolCmd[@]}" >& "$clogfile" &
}

SOF_LOG_COLLECT=0
# This function starts a logger in the background using '&'
#
Expand Down Expand Up @@ -334,7 +353,18 @@ func_lib_start_log_collect()

if [ "X$is_etrace" == "X0" ];then
logfile=$LOG_ROOT/slogger.txt

# start cavstool in the background to collect the etrace.
# Since cavstool can follow a ring buffer in etrace, so it should be
# started at the start of the test and we would not miss any Zephyr logs.
# FIXME (new) sof SOF bug #6039
is_zephyr && func_cavstool_etrace_collect
else
# cavstool starts at the beginning of the test, we don't need to start
# it again. We only need to start sof-logger to collect the etrace logs
# only for the non-zephyr platforms at the end of a test.
is_zephyr && return 0

Copy link
Collaborator

@marc-hb marc-hb Jul 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $logfile variable in this function is a global variable passed to lib.sh and I just realized it is a complete mess that must be fixed before this PR

logfile=$LOG_ROOT/etrace.txt
logopt=""
fi
Expand Down