machine/applehv: Switch to NVMe by default

[cgwalters]

This depends on crc-org/vfkit#78 and is an alternative to crc-org/vfkit#76 that I like better for fixing
#21160

It looks like at least UTM switched to NVMe for Linux guests by default for example.

[NO NEW TESTS NEEDED]

[openshift-ci]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cgwalters
Once this PR has been reviewed and has the lgtm label, please assign rhatdan for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[packit-as-a-service]

Ephemeral COPR build failed. @containers/packit-build please check.

[cfergeau]

I don't know which macOS versions podman supports, but NVMe is macOS 14+ only ( https://developer.apple.com/documentation/virtualization/vznvmexpresscontrollerdeviceconfiguration?language=objc ). macOS 14 was released about 3 months ago.

[cgwalters]

OK yeah, let's just close this then to get it off the board.

It's just a bit tricky because it's one of those things I think we should do after enough time passes...ideally there'd be automation to set "hey a year from now, reopen this PR" (hmm what length of time do macos releases last actually?).

[rhatdan]

Couldn't we just fail over?

       case "nvme":
		disk, err = vfConfig.NVMExpressControllerNew(imagePath)
                if err is NOt supported {
	        	disk, err = vfConfig.VirtioBlkNew(imagePath)
               }
	case "virtio":
		disk, err = vfConfig.VirtioBlkNew(imagePath)

[cgwalters]

Yep I like that idea!

[cgwalters]

Although one thing I don't know is whether we actually fail when adding the device, or when trying to launch the VM.

Backing up, I am a little bit surprised that the vfkit Go code compiles on older MacOS too...I guess this must be because it wraps objective C which is more "dynamically send a message" than "call this C function which may not exist on older OSes".

[cfergeau]

(hmm what length of time do macos releases last actually?).

There's 1 major release per year, with the last 2/3 major releases getting updates at a given time, see https://endoflife.date/macos
I'd guess that most people update relatively quickly, and that by the time macOS 14 is released, most machines have been upgraded to 13.
However, each new major releases drop older hardware, so some people don't have a choice but stick with older macOS versions.

[cfergeau]

Although one thing I don't know is whether we actually fail when adding the device, or when trying to launch the VM.

Backing up, I am a little bit surprised that the vfkit Go code compiles on older MacOS too...I guess this must be because it wraps objective C which is more "dynamically send a message" than "call this C function which may not exist on older OSes".

The magic for this is https://github.com/Code-Hex/vz/blob/4f8ae6d82e477251a3241bb9696b83959947c5fb/virtualization_14.m#L15-L24
In the Code-Hex/vz case, it's complemented by macOS versions checks in the go code to return go errors when appropriate
https://github.com/Code-Hex/vz/blob/4f8ae6d82e477251a3241bb9696b83959947c5fb/storage.go#L301-L303

However, in podman case, none of that code is used/vendored as go code making use of Code-Hex/vz cannot be cross-compiled, it requires macOS. vfkit is split in a pure-go package which is used in podman, and then it runs vfkit which makes use of Code-Hex/vz. At this point, the pure-go config package has no macOS version checks, so you will only get "OS too old" errors when starting vfkit. These could be added though if there's a need for it.

[openshift-merge-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.