search

crc-org / vfkit

Apache License 2.0
119 stars 23 forks source link

Add support for nvme devices #78

Closed cgwalters closed 7 months ago

cgwalters commented 8 months ago

This also seems to avoid the disk corruption that we see with virtio-blk; it reportedly has a small performance hit for raw speed, but I think avoiding the double caching (guest and host) is much better from a performance perspective.

openshift-ci[bot] commented 8 months ago

Hi @cgwalters. Thanks for your PR.

I'm waiting for a crc-org member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
cgwalters commented 7 months ago

Tested this e2e in concert with https://github.com/containers/podman/pull/21208 and I am also not seeing any corruption.

gbraad commented 7 months ago

/ok-to-test

gbraad commented 7 months ago

the NVME device is only supported on macOS 14, so this would error on 13.

cgwalters commented 7 months ago

Your fixup LGTM

cfergeau commented 7 months ago

/lgtm /approve /hold (holding this to leave time for the tests to run)

openshift-ci[bot] commented 7 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfergeau

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: - ~~[OWNERS](https://github.com/crc-org/vfkit/blob/main/OWNERS)~~ [cfergeau] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
cfergeau commented 7 months ago

/unhold

cfergeau commented 4 months ago

Interestingly, Apple recommends using virtio-blk https://developer.apple.com/videos/play/wwdc2023/10007/?time=630 They describe NVMe as a fallback for linux VM images which don't have the virtio-blk module built-in, and need an alternative way to access the disk data.