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

Set applehv as default darwin provider #21145

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

baude
Copy link
Member

@baude baude commented Jan 3, 2024

Podman 5 will not support QEMU on darwin anymore. This PR only changes the default from qemu to applehv. Code changes to enforce not supporting qemu will come later.

[NO NEW TESTS NEEDED]

Does this PR introduce a user-facing change?

The default machine provider for MacOS is the apple hypervisor

Podman 5 will not support QEMU on darwin anymore.  This PR only changes the default from `qemu` to `applehv`.  Code changes to enforce not supporting qemu will come later.

[NO NEW TESTS NEEDED]

Signed-off-by: Brent Baude <[email protected]>
Copy link
Contributor

openshift-ci bot commented Jan 3, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: baude

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 3, 2024
@rhatdan
Copy link
Member

rhatdan commented Jan 3, 2024

LGTM
@ashley-cui @Luap99 @mheon PTAL

@mheon
Copy link
Member

mheon commented Jan 3, 2024

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 3, 2024
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 3, 2024
@cgwalters
Copy link
Contributor

The UX here will be that we start silently ignoring their old machine, right? Probably want to help them clean up old files perhaps?

@baude
Copy link
Member Author

baude commented Jan 3, 2024

more cleanup will be needed but i want a more stable experience with applehv and the problems we are seeing. there will not be likely any migration supported machines from 4 -> 5 ... thats the official story; unofficially, we would like to do as much as we can.

@baude
Copy link
Member Author

baude commented Jan 3, 2024

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 3, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 7d131b8 into containers:main Jan 3, 2024
89 of 92 checks passed
@cgwalters
Copy link
Contributor

cgwalters commented Jan 28, 2024

@baude I'm digging into a few things and...can you (or someone) either link me to or briefly create a summary of why we did the switch to applehv vs qemu? What were the top problems with qemu?

The main thing I'm starting to hit is the lack of any standard way to inject credentials into a generic disk image with virtualization.framework. I am thinking of a way to hack things where we detect the root disk blockdev name in the initramfs and if it's a hardcoded rootwithvirtiofsmeta or something then we require a virtiofs mount to be provided and copy creds from it to the @initrd space.

(xref systemd issue 29175)

But with qemu we can just use the existing SMBIOS bits.

Edit: hmmm...I guess one thing here is that we're using 9p mounts on both Linux and MacOS...on Linux we should definitely be using virtiofsd instead, just needs some work similar to what I did similar to coreos/coreos-assembler#3428 in coreos-assembler. But we can't do that yet on MacOS because "our" virtiofsd doesn't run on MacOS - and the MacOS-owned one is part of Virtualization.Framework.

Wait wait...OK, actually we are also working on a different host-side implementation of virtiofs in https://github.com/containers/libkrun/tree/adb5eb1fb0a5b4643e249b5d1e29df0866776358/src/devices/src/virtio/fs ? And this was apparently forked a while ago from a distinct crosvm implementation?

(Total tangent here but I think it's confusing the new podman backend is called "applehv" and not "applevf" for example, because before qemu still used apple hypervisor...the new thing is really the "vf" part not the "hv" part)

@rhatdan
Copy link
Member

rhatdan commented Jan 28, 2024

QEMU was very unstable, particularly since anyone could push and broken update. Podman was regularly broken by qemu updates and the upstream community showed very little interest in supporting MAC.

Docker and CRC had already abandoned QEMU for similar reasons.

Another major reason was the support for virtiofsd which is not supported by QEMU but is via apple hypervisor. We see much better performance with it then Plan9 with QEMU.

Final reason was the emulation mode (Rosetta) for running X86 code on Arm systems is only supported by native hypervisor.

BTW I think in the future we might want to use krunvm rather then both of these since it gives us back the control, better support for virtiosfd, and GPU sharing inside of the VM with the host. But krunvm is not ready for prime time yet.

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Apr 28, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. machine release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants