Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new local_config.bash files to solve device-specific issues #1234

Merged
merged 4 commits into from
Oct 8, 2024
Merged

Add new local_config.bash files to solve device-specific issues #1234

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5948867
fix statically -> statistically typo in apause.exp
marc-hb Aug 13, 2024
ea0419b
sof-kernel-log-check.sh: simplify journalctl logic
marc-hb Sep 23, 2024
b42cc3c
lib.sh: source additional files local_config.bash
marc-hb Sep 23, 2024
33c5730
sof-kernel-log-check.sh: add new "${sof_local_extra_kernel_ignores[@]}"
marc-hb Sep 23, 2024
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
14 changes: 13 additions & 1 deletion tools/sof-kernel-log-check.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,18 @@ ignore_str="$ignore_str"'|nvme0: Admin Cmd\(0x[[:digit:]]+\), I/O Error \(sct 0x
# SDW related logs
#

# This expects an array like for instance:
# sof_local_extra_kernel_ignores=(-e 'error 1' -e err2)
# Arrays
# - provide whitespace/quoting safety
# - can be empty/optional
# - can be arbitrarily complex. Best avoided but only on specific systems anyway.
# shellcheck disable=SC2154
if &>/dev/null declare -p sof_local_extra_kernel_ignores; then
dlogw "Ignoring extra errors on this particular system:"
Copy link
Contributor

Choose a reason for hiding this comment

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

One concern I have with this facility is that when adding local exceptions becomes easier, it can be used too much and people reading the test logs might not realize some DUTs have local exceptions others don't have. The global ignore list has drawbacks, but at least one needs to make a git commit with proper description and leave something that can be search through git history. OTOH, this dlogw does make local exception use clear in the log, which alleviates my concern. It really depends on how this will be used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the review!

This is a risk but historically, the abuse has been the other way around: the global list has been abused to add issues with "personal" devices. Just look at the git blame, log and past PRs. I expect the existence of this new feature to be missed and the existing abuse to continue :-) Also, it may seem quick to edit files on one device but what 5 or 6?

Both situations exist, I really think the code should have offered both possibilities a long time ago...

declare -p sof_local_extra_kernel_ignores
fi

# confirm begin_timestamp is in UNIX timestamp format, otherwise search full log
if [[ $begin_timestamp =~ ^[0-9]{10} ]]; then
cmd="journalctl_cmd --since=@$begin_timestamp"
Expand All @@ -429,7 +441,7 @@ fi
declare -p cmd

if err=$($cmd --priority=err |
grep -v -E -e "$ignore_str"); then
grep -v -E -e "$ignore_str" "${sof_local_extra_kernel_ignores[@]}"); then

type journalctl_cmd
echo "$(date -u '+%Y-%m-%d %T %Z')" "[ERROR]" "Caught kernel log error"
Expand Down
Loading