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

templates: merge experimental/{riscv64,armv7l} into default #2730

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

AkihiroSuda
Copy link
Member

These archs still remain experimental but can be merged into the default template.

.containerd.user now defaults to false for these archs, as nerdctl-full archive is still missing for them.

@AkihiroSuda AkihiroSuda added this to the v1.0 milestone Oct 13, 2024
logrus.Warn("template://experimental/riscv64 was merged into the default template in Lima v1.0. Use `limactl create --arch=riscv64 template://default` instead.")
case "experimental/armv7l":
logrus.Warn("template://experimental/armv7l was merged into the default template in Lima v1.0. Use `limactl create --arch=armv7l template://default` instead.")
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll also remove experimental/{9p,virtiofs-linux,vz} in favor of the limactl CLI flags, in a separate PR

@@ -20,7 +20,7 @@ vmOpts:
qemu:
# Minimum version of QEMU required to create an instance of this template.
# Will be ignored if the vmType is not "qemu"
# 🟢 Builtin default: not set
# 🟢 Builtin default: not set (but RISC-V requires QEMU 9.1 or later)
minimumVersion: null
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe minimumVersion should be typed as map[Arch] bool? (in a separate PR)

Copy link
Member Author

Choose a reason for hiding this comment

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

(Anyway riscv64 is experimental; I don't want to spend too much time for overengineering it)

Copy link
Member

Choose a reason for hiding this comment

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

I think it worked on earlier versions as well, as long as you supplied the firmware and kernel?

Maybe the qemu implementation details can stay out of the default configuration comments.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it worked on earlier versions as well, as long as you supplied the firmware and kernel?

Yes, but s/and/or/

Maybe the qemu implementation details can stay out of the default configuration comments.

Done

These archs still remain experimental but can be merged into the
`default` template.

`.containerd.user` now defaults to `false` for these archs,
as `nerdctl-full` archive is still missing for them.

Signed-off-by: Akihiro Suda <[email protected]>
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

@jandubois jandubois merged commit c6b5b31 into lima-vm:master Oct 15, 2024
26 checks passed
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.

3 participants