Skip to content

Conversation

@Bin-QA
Copy link
Contributor

@Bin-QA Bin-QA commented Jun 28, 2020

use journalctl command instead of query the begin line from /var/log/kern.log file

Signed-off-by: Wu, BinX binx.wu@intel.com

use journalctl command instead of query the begin line from /var/log/kern.log file

Signed-off-by: Wu, BinX <binx.wu@intel.com>
{
# shellcheck disable=SC2034 # external script will use it
KERNEL_LAST_LINE=$(wc -l /var/log/kern.log|awk '{print $1;}')
KERNEL_LAST_LINE=$(journalctl --dmesg --no-pager -n 1 -o short-iso-precise|awk '/kernel/ {print $1;}')
Copy link
Collaborator

Choose a reason for hiding this comment

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

On my system this prints the boot time (uptime -s) which doesn't seem to match what was before.

Should this simply be KERNEL_LAST_LINE=$(date)?

I like that you're trying to save a lot of changed lines by keeping the KERNEL_LAST_LINE name the same, however there should be a big fat warning comment saying "This is not a line anymore"

Copy link
Member

Choose a reason for hiding this comment

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

I rename the whole thing as KERNEL_LAST_TIMESTAMP in PR #354


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=$("$cmd" |grep 'Call Trace' -A5 -B3)$("$cmd" | grep $project_key | grep -E "$err_str"|grep -vE "$ignore_str")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please submit these cleanups first and separately so they don't noise the functional changes later.

KERNEL_LAST_LINE=$(wc -l /var/log/kern.log|awk '{print $1;}')
KERNEL_LAST_LINE=$(journalctl --dmesg --no-pager -n 1 -o short-iso-precise|awk '/kernel/ {print $1;}')
KERNEL_LAST_LINE=${KERNEL_LAST_LINE:0:-5}
KERNEL_LAST_LINE=${KERNEL_LAST_LINE/T/ }
Copy link
Collaborator

Choose a reason for hiding this comment

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

There should be a comment explaining what this removes and why. It's surprising to drop such a critical piece of information.

@xiulipan
Copy link
Contributor

xiulipan commented Sep 2, 2020

I will close this one as @Bin-QA no longer work on this.
@aiChaoSONG @marc-hb do we still need some similar issues? I will leave all journalctl related issue to you @marc-hb

@marc-hb
Copy link
Collaborator

marc-hb commented Sep 2, 2020

@plbossart is undeterred in #354

@plbossart
Copy link
Member

This PR would not work anyways because there are still tons of 'sudo dmesg -C' everywhere.

@plbossart plbossart closed this Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants