Skip to content

kmsg: Log the raw buffer if we fail to parse #1704

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.

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

Merged
merged 2 commits into from
Jul 16, 2025

Conversation

smalis-msft
Copy link
Contributor

We've seen a crash here in the wild, but haven't yet figured out what exactly is failing to parse. Add some more logging so we can hopefully get this if it happens again.

@smalis-msft smalis-msft requested a review from a team as a code owner July 15, 2025 21:29
@mattkur mattkur requested a review from Copilot July 15, 2025 22:08
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances logging for failed kmsg parses by capturing and displaying the raw buffer on errors, helping diagnose unknown parse failures.

  • Extended InvalidKmsgEntry error to include the raw byte slice.
  • Changed parse error handling: tests now use unwrap, the tracing task unwraps, and the diagnostic tool matches on the parse result to log failures.
  • Updated test and runtime code to log or panic on invalid kmsg entries.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
vmm_tests/vmm_tests/tests/tests/x86_64/openhcl_uefi.rs Switched ? to unwrap on kmsg parse to panic/abort.
vmm_tests/vmm_tests/tests/tests/multiarch.rs Switched ? to unwrap on kmsg parse to panic/abort.
support/kmsg/src/lib.rs Changed InvalidKmsgEntry to carry the raw buffer.
petri/src/tracing.rs Unwrapped kmsg parse in logging task, causing panic.
openhcl/ohcldiag-dev/src/main.rs Matched parse result to print invalid entries.
Comments suppressed due to low confidence (1)

support/kmsg/src/lib.rs:73

  • [nitpick] Consider adding a doc comment to InvalidKmsgEntry explaining that it carries the raw buffer for debugging, and how it will be formatted in the error message.
pub struct InvalidKmsgEntry<'a>(&'a [u8]);

@smalis-msft smalis-msft added the backport_2505 Change should be backported to the release/2505 branch label Jul 16, 2025
@smalis-msft smalis-msft enabled auto-merge (squash) July 16, 2025 18:00
@smalis-msft smalis-msft merged commit f24168c into microsoft:main Jul 16, 2025
28 checks passed
@smalis-msft smalis-msft deleted the kmsg-parse-error branch July 16, 2025 18:34
smalis-msft added a commit to smalis-msft/openvmm that referenced this pull request Jul 16, 2025
We've seen a crash here in the wild, but haven't yet figured out what
exactly is failing to parse. Add some more logging so we can hopefully
get this if it happens again.
smalis-msft added a commit that referenced this pull request Jul 16, 2025
We've seen a crash here in the wild, but haven't yet figured out what
exactly is failing to parse. Add some more logging so we can hopefully
get this if it happens again.

Cherry-pick of #1704
@benhillis
Copy link
Member

Backported in #1709

@benhillis benhillis added backported_2505 PR that has been backported to release/2505 and removed backport_2505 Change should be backported to the release/2505 branch labels Jul 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported_2505 PR that has been backported to release/2505
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants