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

providers/applehv: Add Apple Hypervisor #1696

Merged
merged 1 commit into from
Oct 31, 2023
Merged

providers/applehv: Add Apple Hypervisor #1696

merged 1 commit into from
Oct 31, 2023

Conversation

baude
Copy link
Contributor

@baude baude commented Aug 15, 2023

  • Add applehv platform
  • Ignintion read from vsock connection with the host

See coreos/fedora-coreos-tracker#1533 and
coreos/fedora-coreos-tracker#1548

Copy link
Member

@travier travier left a comment

Choose a reason for hiding this comment

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

Looks nice and clean

docs/release-notes.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@prestist prestist left a comment

Choose a reason for hiding this comment

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

Wow thank you for putting in this work!

Looks great, I just have a few questions/nits.

Thank you again!

internal/providers/applehv/applehv.go Show resolved Hide resolved
internal/providers/applehv/applehv.go Outdated Show resolved Hide resolved
internal/providers/applehv/applehv.go Outdated Show resolved Hide resolved
internal/providers/applehv/applehv.go Show resolved Hide resolved
@baude
Copy link
Contributor Author

baude commented Aug 16, 2023

updated based on comments.

@travier
Copy link
Member

travier commented Aug 16, 2023

Agree with the two remaining comments: Let's document what the URL and port mean and how to setup QEMU to test it maybe?

@baude
Copy link
Contributor Author

baude commented Aug 17, 2023

Agree with the two remaining comments: Let's document what the URL and port mean and how to setup QEMU to test it maybe?

do you really mean qemu?

@travier
Copy link
Member

travier commented Aug 21, 2023

Yes. We don't need the full QEMU setup, only what additional options would be needed to add to an existing QEMU instance to be able to test this. I think QEMU has support for vsock which should let us test this code outside of applehv systems.

@travier
Copy link
Member

travier commented Aug 21, 2023

We don't have specific doc pages for each provider yet. So either some comments here or a new doc page would be good.

@baude
Copy link
Contributor Author

baude commented Aug 22, 2023

ok, i have updated this pr based on comments. i will add specific documentation on how to do this without podman/vfkit/gvproxy to the fcos documentation like i did with hyperv.

Copy link
Collaborator

@prestist prestist left a comment

Choose a reason for hiding this comment

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

Looks good to me, I dont see anything that needs to change.

Thank you for getting this work done.

internal/providers/applehv/applehv.go Show resolved Hide resolved
need an httpd server that is capable of responding to a GET over vsock. Alternatively, the httpd
server could listen on a unix domain socket (uds) and a utility would need to be written that copies
from the vsock <-> uds
*/
Copy link
Member

@travier travier Aug 23, 2023

Choose a reason for hiding this comment

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

Would this be a valid "diagram" of the platform setup?

Emulated: HTTP server (podman) <--> vsock (host) <--> QEMU <--> kernel (vsock-virtio) <--> Ignition (vsock client)
Actual: HTTP server (podman) <--> vsock (host) <--> Apple HV <--> kernel (vsock-virtio) <--> Ignition (vsock client)

internal/providers/applehv/applehv.go Outdated Show resolved Hide resolved
internal/providers/applehv/applehv.go Show resolved Hide resolved
@travier
Copy link
Member

travier commented Aug 23, 2023

A couple more questions to clarify the limit between what's set by the platform and what we're deciding.

@travier
Copy link
Member

travier commented Oct 25, 2023

OK, LGTM. Thanks for the clarifications.

@travier
Copy link
Member

travier commented Oct 25, 2023

@prestist I'll let you merge. We'll likely need to do a release soon to get that in FCOS.

docs/release-notes.md Outdated Show resolved Hide resolved
dracut/30ignition/module-setup.sh Outdated Show resolved Hide resolved
dracut/30ignition/module-setup.sh Outdated Show resolved Hide resolved
internal/providers/applehv/applehv.go Show resolved Hide resolved
@baude baude force-pushed the applehv2 branch 2 times, most recently from 99dc001 to 5858f58 Compare October 30, 2023 21:48
* Add applehv platform
* Ignintion read from vsock connection with the host

See coreos/fedora-coreos-tracker#1533 and
coreos/fedora-coreos-tracker#1548

Signed-off-by: Brent Baude <[email protected]>
@jlebon jlebon merged commit feca436 into coreos:main Oct 31, 2023
9 checks passed
baude added a commit to baude/ignition that referenced this pull request Nov 1, 2023
A review comment to coreos#1696 suggested I see if I could eliminate a pair of
modules needed in the ramdisk to consume an ignition file over vsock.
In doing the testing, I experienced a false positive; unfortunately, I
have determined they are in fact needed and this PR asks to add them
again.

Signed-off-by: Brent Baude <[email protected]>
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