-
Notifications
You must be signed in to change notification settings - Fork 2.6k
machine: enable nested virt on libkrun by default #25922
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
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jakecorrenti The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Disclaimer: I only have an M2 so I can't test the nested virtualization works. I was able to test that it failed to start the VM, however, if I modified the condition to include M2. CC @slp |
LGTM |
is there any downside to doing this @slp or @jakecorrenti ? |
There's no immediate downsides AFAIK. There's some inherent risk since it's a new feature from Apple, but if there's any issues we can always just remove the use of the flag. |
LGTM, thanks @jakecorrenti |
@baude Want to merge, or have concerns? |
what happens if the mac or os is not capable of the nested virt? |
On the libkrun side, if the hardware isn't capable of nested virt it will return an error and not try to start the VM. I avoid that in this PR by checking the CPUID for the CPU model to ensure it's a new-enough chip. With regards to OS version, the feature requires macOS 15. I could add an OS check in Podman to make sure it's not enabled if you see that as necessary/beneficial. I don't have the right hardware so I'm not sure what would happen in this case if you have the right chip but the wrong OS version. |
Hm... I haven't thought of OS version. When running on macOS < 15, libkrun does detect the missing feature and errors out with EINVAL. The caller could retry, but ideally this should be detected beforehand (@jakecorrenti). I have 14 in a VM, so I can give it a try once this PR is updated. |
pkg/machine/apple/apple.go
Outdated
// only M3 processors and newer can support nested virtualization | ||
if strings.Contains(cpuBrandString, "M3") || strings.Contains(cpuBrandString, "M4") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like constant maintenance cost, if there is an M5 chip it would be fair to assume it would work there as well I think. And I rather not have that updated every year because we forget until someone reports a bug.
(And in general trying to match the logic from libkrun here again just introduces the possibility of drift between the two implementations. I rather have some smart feature flag in krunkit that enabled the virt but only if possible? Any reason this is not the default in libkrun?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far we tried to avoid adding policy in libkrun, leaving that to the caller. What we can do is add a new API to allow callers to check if nested is supported in the platform. WDYT @jakecorrenti ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a good compromise. I can look into that today
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't nested be on by default and libkrun figure out if it is supported or not? This seems like an issue on the libkrun side? Is there ever a reason a user would want to run --nested=false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we've bumped krunkit to 0.2.1, it includes the krun_check_nested_virt
API from the latest libkrun release. This now allows Podman to do --nested
, but if the host isn't supported it will ignore the argument and continue starting the VM as normal. This avoids Podman having to worry about the maintenance burden of managing supported OS and CPU versions.
Update: krunkit has cut a release with a fix, but I'm still waiting on libkrun to cut a release. Once that happens, I'll update the PR. |
063d45d
to
6890748
Compare
With the recent release of krunkit 0.2.0, a CLI option was added to enable nested virtualization on macOS hosts with an M3 or higher. Enable this by default. If the host does not support this feature, krunkit will ignore the argument and continue starting the VM. Signed-off-by: Jake Correnti <[email protected]>
6890748
to
97609a0
Compare
Hi @antreos, thanks for taking a look. Just to cover all our bases, can you try updating krunkit to 0.2.1 to see if that fixes anything. I don't have M3+ hardware, so I can't try and reproduce this myself. |
thanks for the quick response! I updated via brew upgrade to 0.2.1. Unfortunately it's the same behavior meaning: without --nested it's working again. With --nested it stalls at the same point during boot (after "EFI stub: Using"). I tried various configurations including --rootful & with higher memory as well as only 1 CPU, but no luck. If I should try a different image/configuration let me know. I'd really like to see nested virtualization working, as it would be quite useful for me. |
@antreos could you try using the |
@antreos Do an rosetta bug in applehv our images are held back a bit so the kernel is a bit behind. We have a new build in containers/podman-machine-os#116, so if you download the image from there you could try that, it should have a newer kernel in case this is kernel related: Once that is downloaded you could use it with |
@jakecorrenti - I hope the command is correct: Without --nested I can configure the root password and get into a shell. (Fedora-Server-Host-Generic-42-1.1.aarch64.raw.xz ) With --nested I get a bit further then with podman but it seems to hang aswell. (krunkit also gets to 100% again) Edit: |
Hi @Luap99 thanks for the newer image. I had to use this URL: https://api.cirrus-ci.com/v1/artifact/task/6008012294324224/image/podman-machine.aarch64.applehv.raw.zst as your URL gave me an 404 error. Unfortunately the result seems the same. Edit: I've attached a --krun-log-level trace with the --nested flag set. |
With this change, I was able to run the podman machine on an Apple M3 Pro. Command:
Machine info:
Host OS version:
|
With the recent release of krunkit 0.2.0, a CLI option was added to enable nested virtualization on macOS hosts with an M3 or higher. Enable this by default on supported hosts.
Does this PR introduce a user-facing change?
For users running Podman Machines with krunkit on an M3 or newer host, nested virtualization is enabled by default.