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

VisionFive2: Use docker recommended kernel config #309

Closed
wants to merge 1 commit into from

Conversation

maxberger
Copy link
Contributor

Config was created using the helper script from https://github.com/moby/moby/raw/master/contrib/check-config.sh

@paralin
Copy link
Collaborator

paralin commented Mar 11, 2024

@maxberger if you enable apps/docker all of this will be enabled as well.

Is there some reason to include this in the base config? In general we try to keep the base config minimal and add the necessary bits for docker when enabling docker. Thanks!

@maxberger
Copy link
Contributor Author

Interestingly the settings where not included for me when I tried building (2024.1 tag), this is why I added them.

One reason could be that they are excplicitly set to be unset in

https://github.com/skiffos/SkiffOS/blob/master/configs/starfive/common/kernel/base

Could that be an issue? Then I would only re-add the ones that are unset in that file

@paralin
Copy link
Collaborator

paralin commented Mar 11, 2024

The settings are merged using the kernel merge_config.sh - config packages that appear later in SKIFF_CONFIG override those earlier in the list. As long as apps/docker appears after starfive/visionfive2 on the list, it should override those settings.

Maybe some are missing in apps/docker? Or some prerequisites are not enabled?

You can check by using "make linux-menuconfig" in the workspace directory and then going to one of the options that isn't set properly (press / then search for it) and it will tell you which prerequisites are missing.

@paralin paralin closed this in d0bb086 Mar 31, 2024
@paralin
Copy link
Collaborator

paralin commented Mar 31, 2024

@maxberger this should be fixed now! thanks.

@maxberger
Copy link
Contributor Author

@maxberger this should be fixed now! thanks.

Thanks! Sorry for not following up with this.

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.

None yet

2 participants