user journalctl instead of catch kernel log from /var/log/kern.log #250

Bin-QA · 2020-06-10T09:11:54Z

Use journalctl command instead of catch kernel log from /var/log/kern.log file
this change will effect those:

tools/sof-kernel-dump.sh script
tools/sof-kernel-log-check.sh script
tools/sof-get-kernel-line to droped
lib.sh/DMESG_LOG_START_LINE change to CASE_KERNEL_START_TIME
lib.sh/KERNEL_LAST_LINE change to KERNEL_LAST_TIME

limit: journalctl base on system time stamp, so need confirm DUT time stamp is correct

use journalctl command instead of this step: 1. get kernel bootup line number by sof-get-kernel-line.sh 2. user step 1 information to dump the kernel log from /var/log/kern.log file now direct use journalctl to dump the kernel log after system bootup Signed-off-by: Wu, BinX <binx.wu@intel.com>

with replace kern.log file the keyword filter already changed from '] sof-audio' to 'sof-audio' so need refine the case filter Signed-off-by: Wu, BinX <binx.wu@intel.com>

use journalctl command instead of query the begin line from /var/log/kern.log file This change also modify all script which refer load sof-kernel-log-check file Signed-off-by: Wu, BinX <binx.wu@intel.com>

use journalctl instead of catch information from /var/log/kern.log file 1. DMESG_LOG_START_LINE => CASE_KERNEL_START_TIME 2. with this change, sof-get-kernel-line is not useful, remove it Signed-off-by: Wu, BinX <binx.wu@intel.com>

marc-hb

This seems to replace a fairly complex computation of a shifting "begin_line" with something that always returns the same boot time. So I don't understand.

Can you detail what sort of failures and Call Traces this was tested with?

I see 4 commits, are they really changes that could be merged one at a time? If yes then could you maybe split this PR into multiple PRs? If no then squash them together.

test-case/verify-sof-firmware-load.sh

marc-hb · 2020-06-10T17:46:44Z

tools/sof-kernel-log-check.sh

 #!/bin/bash

-begin_line=${1:-1}
+begin_time=${1:-0}


Add a comment explaining when zero is a valid value, why and what it does.

marc-hb · 2020-06-10T17:51:22Z

tools/sof-kernel-log-check.sh

-#[[ $kernel_buffer_size -lt $dmesg ]] && cmd="dmesg" || cmd="sed -n '$begin_line,\$p' /var/log/kern.log"
+[[ ! "$err_str" ]] && {
+    echo "Missing error keyword list"
+    builtin exit 0


I don't think builtin exit still makes a difference since #229 . Either remove this builtin or reset the trap

PS: don't fix all the other builtin in this PR.

the script on tools, we direct to load them as command,
so the trap exit is not effect them

I'm still lost sorry. So let's focus on the present: what does builtin achieve here now?

the older code use exit as function to overwrite exit command,
so we need builtin exit to let them not confuse, now we use the trap exit
it looks we can change builtin exit to exit at sof-test

it looks we can change builtin exit to exit at sof-test

Thanks, that's one of the things I meant. Keeping builtin is confusing. However it's not the only thing.

Before #229 you could avoid the exit handler thanks to builtin. How do you avoid the exit handler now after #229? Discuss this second thing in #229, not here.

marc-hb · 2020-06-10T17:53:36Z

tools/sof-kernel-log-check.sh

-[[ "$begin_line" -eq 0 ]] && cmd="dmesg" || cmd="sed -n '$begin_line,\$p' /var/log/kern.log"
-
-#echo "run $0 with parameter '$*' for check kernel message error"
+if [ "X$begin_time" == "X0" ]; then


== is a bashim inside [ ] which is not a bashism. It works but it's not guaranteed to work. For something so simple just use [ = ].

marc-hb · 2020-06-10T17:56:58Z

case-lib/hijack.sh

-    if [[ -n "$DMESG_LOG_START_LINE" && "$DMESG_LOG_START_LINE" -ne 0 ]]; then
-        tail -n +"$DMESG_LOG_START_LINE" /var/log/kern.log |cut -f5- -d ' ' > "$LOG_ROOT/dmesg.txt"
+    if [[ -n "$CASE_KERNEL_START_TIME" ]]; then
+        journalctl --flush


Why --flush here?

just use to confirm data already store, so I can remove it?

journalctl --flush can help only if there is a system crash soon. If don't expect a crash between this line and the next line, do you?

when we load this part at exit, so it seems don't need line

marc-hb · 2020-06-10T18:10:30Z

tools/sof-kernel-log-check.sh


 if [ "$ignore_str" ]; then
-    err=$(eval $cmd|grep 'Call Trace' -A5 -B3)$(eval $cmd | grep $project_key | grep -E "$err_str"|grep -vE "$ignore_str")
+    err=$(eval "$cmd"|grep 'Call Trace' -A5 -B3)$(eval "$cmd" | grep $project_key | grep -E "$err_str"|grep -vE "$ignore_str")


Again it's not clear what the eval achieve here.

PS: do not remove all useless eval, only the modified ones.

tools/sof-kernel-dump.sh

marc-hb · 2020-06-10T18:17:27Z

tools/sof-kernel-log-check.sh

+    date -d "$begin_time" +'%F %T' > /dev/null || {
+        echo "Error parameter for date: $begin_time"
+        echo "Support date format: date +'%F %T'"
+        builtin exit 0


I don't think builtin works since #229

marc-hb · 2020-06-10T18:28:41Z

case-lib/hijack.sh

+        journalctl --dmesg --no-pager --no-hostname -o short-precise --since="$CASE_KERNEL_START_TIME" > "$LOG_ROOT/dmesg.txt"
    else
-        cut -f5- -d ' ' /var/log/kern.log > "$LOG_ROOT/dmesg.txt"
+        journalctl --dmesg --no-pager --no-hostname -o short-precise > "$LOG_ROOT/dmesg.txt"


Please put --no-pager --no-hostname -o short-precise into a new JOURNALCTL_FORMAT_OPTS variable so they can be modified in one place.

case-lib/lib.sh

Bin-QA · 2020-06-11T00:49:04Z

OK, I will split them into different PR,
I just think it is all refer the /var/log/kern.log query,
so I add them into one PR

For the call trace I find them at journalctl,
so I decide to replace query from /var/log/kern.log to journalctl
but for the really usage, I'm afraid need to query this kernel for test

Bin-QA · 2020-06-11T01:37:38Z

PR: #251 : tools: sof-kernel-dump: use journalctl command to query kernel log

Bin-QA · 2020-06-11T02:20:35Z

PR: #252 test-case: verify-sof-firware-load: update filter of fireware keyword

Bin-QA · 2020-06-28T01:19:03Z

PR: #262 tools: sof-kernel-log-check: use journalctl command to query kernel log

Bin-QA · 2020-06-28T01:52:22Z

PR: #263 lib: hijack: use journalctl command to dump kernel information

fredoh9 · 2020-06-28T03:30:38Z

@Bin-QA journalctl --flush need sudo permission. Without it I have this error.

Failed to connect to /run/systemd/journal/io.systemd.journal: Permission denied

Bin-QA · 2020-06-28T04:28:53Z

@fredoh9 I already split the PR to different PR, which apply the change advice and fix journalctl --flush

Bin-QA · 2020-06-30T02:38:24Z

with split this PR to 4 PR: #251 , #252 , #262 , #263 close current

Bin-QA added 3 commits June 8, 2020 22:35

test-case: verify-sof-firware-load: update filter of fireware keyword

5d95b80

with replace kern.log file the keyword filter already changed from '] sof-audio' to 'sof-audio' so need refine the case filter Signed-off-by: Wu, BinX <binx.wu@intel.com>

tools: sof-kernel-log-check: use journalctl command to query kernel log

419ee3e

use journalctl command instead of query the begin line from /var/log/kern.log file This change also modify all script which refer load sof-kernel-log-check file Signed-off-by: Wu, BinX <binx.wu@intel.com>

Bin-QA requested review from aiChaoSONG, fredoh9, keqiaozhang, libinyang, marc-hb and xiulipan as code owners June 10, 2020 09:11

Bin-QA force-pushed the journalctl branch from 8e739c2 to 1a1257c Compare June 10, 2020 11:36

marc-hb requested changes Jun 10, 2020

View reviewed changes

Bin-QA closed this Jun 30, 2020

Bin-QA deleted the journalctl branch June 30, 2020 02:38

user journalctl instead of catch kernel log from /var/log/kern.log #250

user journalctl instead of catch kernel log from /var/log/kern.log #250

Uh oh!

Conversation

Bin-QA commented Jun 10, 2020

Uh oh!

marc-hb left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Bin-QA commented Jun 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bin-QA commented Jun 11, 2020

Uh oh!

Bin-QA commented Jun 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bin-QA commented Jun 28, 2020

Uh oh!

Bin-QA commented Jun 28, 2020

Uh oh!

fredoh9 commented Jun 28, 2020

Uh oh!

Bin-QA commented Jun 28, 2020

Uh oh!

Bin-QA commented Jun 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marc-hb left a comment •

edited

Loading

Bin-QA commented Jun 11, 2020 •

edited

Loading

Bin-QA commented Jun 11, 2020 •

edited

Loading