btf: fix LoadKernelSpec #145
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The current code fails to find any kernel spec because the call to Sprintf may contain too many or too few arguments. The result then contains %!s(MISSING), etc. which of course triggers an error when trying to open the path. Another problem is that the function tries other filenames if reading the BTF itself fails for some reason. In that case it will most likely return ErrNotFound, and the user is none the wiser: we found a BTF but couldn't load it. Change the code so that we return any error from the BTF loading itself.
|
cc @brycekahle this is why CI fails. Now I'm mystified why it worked on the other kernels 🤷 |
brycekahle
reviewed
Oct 1, 2020
| continue | ||
| } | ||
| return spec, nil | ||
| return loadSpecFromVmlinux(fh) |
There was a problem hiding this comment.
There isn't a guarantee that these files will have a .BTF section. Should we keep searching if that is the case?
There was a problem hiding this comment.
I think that's unlikely to help: most systems will only have a single path where vmlinux is located (since it's a single package). Trying other paths has the downside that we return ErrNotFound instead of an error describing why we can't use the existing BTF. That is really confusing to users (and to me when I started to look into the issue).
brycekahle
approved these changes
Oct 2, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current code fails to find any kernel spec because the call to
Sprintf may contain too many or too few arguments. The result then
contains %!s(MISSING), etc. which of course triggers an error when
trying to open the path.
Another problem is that the function tries other filenames if reading
the BTF itself fails for some reason. In that case it will most likely
return ErrNotFound, and the user is none the wiser: we found a BTF
but couldn't load it.
Change the code so that we return any error from the BTF loading itself.