Allow dkms to work in a chroot environment without /proc #485
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
|
@akorn I'm all for the change, but the tests are failing, can you have a look? Thanks. If the change requires changes to the tests, please provide those as well. |
In do_status_weak(), the previous code that relied on "< <" didn't run the loop when `module_status_weak()` returned failure. The new code, with "<<<", ignores the return status of `module_status_weak()` and only uses its output, even if it's empty. This change exits the `while` loop in `do_status_weak()` if `module_status_weak()` returned nothing. Similar changes may be necessary elsewhere, but let's see if this fixes the test.
In `setup_kernel_arches()`, we try to determine whether the architecture should be `ia32e`, based on some heuristics involving /proc/cpuinfo. This change allows the code to continue if `/proc/cpuinfo` doesn't exist, e.g. because we're in a chroot and `/proc` isn't mounted. In chroot situations where this heuristic architecture detection is expected to work (which I assume to be rare), a faux `/proc/cpuinfo` file should be created before calling `dkms`, e.g. by copying it from the host's real `/proc`. Since `dkms` would have just failed in such chroot environments before, this change still results in an improvement; the code can be refined later if necessary.
The shift from the `< <(cmd)` idom to `<<<"$(cmd)"` which is needed to work without /proc being mounted has the unfortunate side effect that the exit status of `cmd` is ignored where it wasn't before. This can cause statements that previously always had valid input to be called on empty strings. One such occurrence was in `setup_kernels_arches()`, which, among other things, populates the `kernelver` array; after the `<<<` change, this array could contain an empty string as an element. This commit adds a sanity check to make sure we don't pretend there is a kernel whose version number is the empty string.
|
@scaronni, with my latest commit the tests mostly succeed, except this one: It seems that where the test expects Is that really a problem? If yes, I don't know how to fix it (i.e. break it again); I couldn't immediately understand how the tests even work. |
|
@anbe42, you wrote that noisy test that now succeeds instead of failing. Can you help a bit here? Why/how is |
|
Do you have the same kernel running on the host and installed in the chroot? Try a different kernel in the chroot. Usually host and chroot have different kernels (as it happens in all the CI scenarios) and the wrong 'make clean' command picks the wrong kernel (the one from the host). IIRC I had some checks for this issue, but maybe not in this specific test case. |
If I only have a different kernel in the chroot, the tests fail much sooner: If I have both the same and a newer kernel, the testsuite fails later: And if I have an older, the same, and a newer kernel: It doesn't seem like I can satisfy this particular test. @anbe42, any suggestions? |
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.
I didn't author these changes; I'm just submitting the pull request (with the author's permission) in the hope that it can be merged with little or no discussion.
It's a fairly small and self-explanatory set of changes that, at least at first glance, shouldn't alter existing behaviour, just make dkms as a whole more robust by not requiring
/procto be mounted for it to work.