Merge ~rickwu4444/bugit:1945153-add-journalctl-if-system-freeze into bugit:master

Proposed by Rick Wu
Status: Merged
Approved by: Rick Wu
Approved revision: 78adec0997daeac008e1400b303300329b567c5c
Merged at revision: 59c242c90910dc2bd118f1e550f51ebb58463331
Proposed branch: ~rickwu4444/bugit:1945153-add-journalctl-if-system-freeze
Merge into: bugit:master
Diff against target: 75 lines (+27/-5)
2 files modified
qabro/bug_assistant.py (+17/-0)
qabro/ui.py (+10/-5)
Reviewer Review Type Date Requested Status
Pierre Equoy Approve
StanleyHuang Needs Information
Doug Jacobs Pending
Vic Liu Pending
Review via email: mp+413503@code.staging.launchpad.net

Commit message

Add attach journalctl kernel logs from last boot if issue is about system freeze.

Description of the change

Due to if journalctl has only one log, which is current boot. The return code of 'journalctl -b -1 -k' will be '0', but it will be empty value of 'stdout'. Check multiple conditions in if statement.

ceqa@ubuntu:~$ sudo journalctl --list-boots
 0 f8ccbca4631d4bca91ab870f134e9fc0 Tue 2021-12-28 15:07:28 UTC—Tue 2021-12-28 16:15:11 UTC
ceqa@ubuntu:~$ sudo journalctl -b -1 -k
Data from the specified boot (-1) is not available: No such boot ID in journal
ceqa@ubuntu:~$ echo $?
0

https://bugs.staging.launchpad.net/qabro/+bug/1955723

To post a comment you must log in.
Revision history for this message
StanleyHuang (stanley31) wrote :

A little suggestion inline. Thanks for implement this feature.

review: Needs Information
Revision history for this message
Rick Wu (rickwu4444) wrote :

> A little suggestion inline. Thanks for implement this feature.
Indeed, but we use "-k" here, it means show only kernel messages.

Revision history for this message
Pierre Equoy (pieq) wrote :

Thanks for tackling this issue, Rick!

I compiled qabro from your changes and gave it a try on my customer project and it works as expected. Good job! \o/ → https://bugs.staging.launchpad.net/qabro/+bug/1955844

I like your solution to add the kernel log when the device froze.

One thing I'm afraid is some day, we might change the string for "Device froze, issue reported and logs collected right after a reboot", and if that's the case, we may break your solution.

Maybe it would be better to use global strings for the different stages:

    STAGE_IMMEDIATE = "Issue reported and logs collected right after it happened"
    STAGE_FROZEN_DEVICE = "Device froze, issue reported and logs collected right after a reboot"
    STAGE_LATER = "Issue reported and logs collected at a later stage"

Then we can use these strings to populate `stage_bug_filed`, and to set the boolean value of `options['device_froze']`.

What do you think?

Other that that, I added a few comments inline.

review: Needs Fixing
Revision history for this message
Rick Wu (rickwu4444) wrote :

Fixed. Please help to review it again. many thanks.

Revision history for this message
Pierre Equoy (pieq) wrote :

+1

Thanks again!

review: Approve

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: