Skip to content
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

Don’t attempt to copy built-in modules #47

Merged
merged 4 commits into from
May 27, 2020

Conversation

andrewshadura
Copy link
Contributor

@andrewshadura andrewshadura commented Feb 27, 2020

Several distributions (including Ubuntu) ship some of the modules we rely on compiled into the kernel. Since these modules are listed in modules.builtin, it’s very easy to detect and skip them.

Signed-off-by: Andrej Shadura [email protected]

@andrewshadura andrewshadura force-pushed the skip-builtins branch 2 times, most recently from 5b7c256 to e914031 Compare February 27, 2020 17:24
Copy link
Member

@obbardc obbardc left a comment

Choose a reason for hiding this comment

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

go indentation format is tab rather than 4spaces
it would be good to include a comment above the codeblock ; something very basic like:

// don't attempt to copy built-in modules

@obbardc
Copy link
Member

obbardc commented Feb 28, 2020

this should close #13 and improve Ubuntu support. Thanks Andrej!

@obbardc
Copy link
Member

obbardc commented Feb 28, 2020

actually, looking into this a bit further shows two other pull requests with similar goals:
#45
#36

still, all three patchsets do not support reading from modules.builtin.bin

@andrewshadura
Copy link
Contributor Author

andrewshadura commented Feb 28, 2020

To be honest, I think while my knowledge of Go is very basic, reading the file into a map seems a better choice for me than slurping it into a string, splitting it into an array and doing linear searches in it.

@andrewshadura andrewshadura force-pushed the skip-builtins branch 3 times, most recently from 77ffb8a to ccd122f Compare February 28, 2020 14:56
elboulangero and others added 4 commits February 28, 2020 15:57
…he loop

Also rename usrpath to moddir ie. "modules directory".

Signed-off-by: Arnaud Rebillout <[email protected]>
(this is to prepare upcoming commits)

Signed-off-by: Arnaud Rebillout <[email protected]>
This is needed for ArchLinux.

With this commit, if we don't find a `.ko` modules, we re-try with
`.ko.xz`, and if we don't find it, we definitely fail.

Signed-off-by: Arnaud Rebillout <[email protected]>
Several distributions (including Ubuntu) ship some of the modules we
rely on compiled into the kernel. Since these modules are listed in
modules.builtin, it’s very easy to detect and skip them.

This change fixes go-debos#13 and supersedes go-debos#36.

Signed-off-by: Andrej Shadura <[email protected]>
@andrewshadura
Copy link
Contributor Author

I have included xz support commits from #45 into my pull requests.

@obbardc
Copy link
Member

obbardc commented Feb 28, 2020

agreed with the above, nice work ;-)

var builtinModules = make(map[string]bool)

f, err := os.Open(path.Join(moddir, kernelRelease, "modules.builtin"))
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

hm, if the file does not exist, we should not attempt to load modules.builtin and just load the modules normally ?
what about systems that use modules.builtin.bin ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was considering this, but then I realised we already unconditionally copy modules.builtin (unless I’m misreading the code), so it is already required to be present.

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