Support for Virtualization.Framework for macOS 13 by balajiv113 · Pull Request #1147 · lima-vm/lima

balajiv113 · 2022-11-07T05:17:33Z

This PR provides support for using Virtualization.Framework as a optional driver.

The following are the changes done related to using drivers,

Driver interface
Migrate current QEMU implementation to driver
New driver vz for Virtualization.Framework

The following are the features of lima, the vz driver should provide support for below,

Running VM (Using both disk img and iso)*
Slirp network for guest to host communication (uses gvisor-tap-vsock)
Host to guest network (uses vz NAT gateway)*
Directory sharing, supports reverse-sshfs and virtiofs (newly added)
Port forwarding
Host DNS resolver
Display (Deferring it for now as it requires runtime.LockOsThread() to be called in the beginning of hostagent start cmd)

Notes

Vz driver internally converts qcow to raw image using (qemu-img convert). This is because vz only supports raw disk
Need to provide yaml configuration for NAT

Know Issues

Serial log doesn't contain system boot logs
Gvisor-tap-vsock will throw some errors like [e.connection](tcpproxy:) during start-up (This is because of port 22 Forwards being tried before VM is ready) [Not a failure/blocker just info message thrown, but we can look into fixing in a follow-up to call ssh forward manually]
When vz vm stops, it doesn't trigger hostagent stop [Fixed]

Testing
Tested the following templates with driver: vz. All test are done on macOS 13 intel as of now.

AkihiroSuda · 2022-11-07T06:37:26Z

examples/docker.yaml

 # $ docker ...

 # This example requires Lima v0.8.0 or later
+driver: "vz"


The default must remain qemu

You can create examples/vz.yaml for the VZ example. (Maybe under experimental dir?)

Yes yes that make sense. Will put it under experimental directory.

AkihiroSuda · 2022-11-07T07:05:25Z

pkg/store/filenames/filenames.go

 	HostAgentSock      = "ha.sock"
 	HostAgentStdoutLog = "ha.stdout.log"
 	HostAgentStderrLog = "ha.stderr.log"
+	Identifier         = "identifier"


Please describe in https://github.com/lima-vm/lima/blob/master/docs/internal.md

Also, perhaps the filename should be something like vz-identifier?

AkihiroSuda · 2022-11-07T07:05:31Z

pkg/store/filenames/filenames.go

 	HostAgentStdoutLog = "ha.stdout.log"
 	HostAgentStderrLog = "ha.stderr.log"
+	Identifier         = "identifier"
+	EFI                = "efi"


AkihiroSuda · 2022-11-07T07:07:42Z

pkg/vz/vm_darwin.go

+
+	err = machine.Start()
+
+	networks.StartGVisor(ctx, &networks.GVisorOpts{


This looks a misnomer.
This starts the network stack that originates from gvisor, but does not start gvisor (sandboxed kernel) actually

Maybe StartGVisorNetstack? StartNetstack? StartGvisorNet?

AkihiroSuda · 2022-11-07T07:08:53Z

pkg/limayaml/limayaml.go

 )

 type LimaYAML struct {
+	Driver            *Driver           `yaml:"driver,omitempty" json:"driver,omitempty"`


Should this be called like VMType for consistency with MountType?

Also please create a document like docs/vmtype.md or docs/vz.md to explain the usage, requirements, and limitations of vz

Isn't VMType kind of gives a wrong meaning (Type of VM like linux etc).

How about DriverType ?

Type of VM like linux etc

We only support Linux.
Eventually we may support FreeBSD (if somebody contributes), but that will be probably called "type of guest", not "type of VM"

Done the VMType change only for yaml config related places.
Do let me know if you want to change the word driver everywhere (like driver interface, implementations etc).

Will add doc related things as next step.

Thanks, "driver" in the interface and impl is ok

pkg/limayaml/limayaml.go

AkihiroSuda · 2022-11-07T07:11:19Z

pkg/networks/gvisor.go

+}
+
+func searchDomains() []string {
+	if runtime.GOOS == "darwin" || runtime.GOOS == "linux" {


Perhaps just runtime.GOOS != "windows"

pkg/qemu/imgutil/imgutil.go

AkihiroSuda · 2022-11-07T07:14:05Z

pkg/vz/vm_darwin.go

+					logrus.Errorf("Error while stopping the VM %q", err)
+				}
+			case newState := <-machine.StateChangedNotify():
+				if newState == vz.VirtualMachineStateRunning {


Can we use switch{} for newState comparisons?

AkihiroSuda · 2022-11-07T07:14:45Z

pkg/vz/vm_darwin.go

+}
+
+func attachNetwork(driver *driver.BaseDriver, vmConfig *vz.VirtualMachineConfiguration, networkConn *os.File) error {
+	//Slirp network


Did you mean gvisor network?

Yes, updated the comments

AkihiroSuda · 2022-11-07T07:17:59Z

pkg/vz/vm_darwin.go

+	return vz.NewEFIVariableStore(efi)
+}
+
+func createFileAndWriteTo(data []byte, path string) error {


os.WriteFile may suffice

AkihiroSuda · 2022-11-08T00:11:02Z

examples/experimental/vz.yaml

+    set -eux -o pipefail
+    command -v docker >/dev/null 2>&1 && exit 0
+    export DEBIAN_FRONTEND=noninteractive
+    curl -fsSL https://get.docker.com | sh


vz.yaml should not have Docker.
You may create docker-vz.yaml separately though

But anyway no need to keep docker-vz.yaml separately when we have limactl start --set K=V or maybe limactl start --vm-type vz

limactl: start: introduce --set <stringArray> flag to pin static config values instead of creating custom config file #1145

Sure. Then i will keep a default example alone in vz.yaml

balajiv113 · 2022-11-08T04:23:09Z

pkg/vz/vm_darwin.go

+	//Other networks
+	for _, network := range driver.Yaml.Networks {
+		//TODO - Handle NAT network type
+		if network.Socket != "" {


@AkihiroSuda
For NAT network type, shall i go ahead with network.NAT: true/false or network.socket: NAT ?

What do you mean by "NAT"?

Anyway probably it should be a separate PR

For host to guest network, the below code uses https://developer.apple.com/documentation/virtualization/vznatnetworkdeviceattachment provided from virtualization.framework itself.

This doesn't require sudo access.

Would it be possible to just use vznat by default and drop gvisor netstack ?

If vznat has to be optional, probably it should be like ~~network.VZNAT: true/false~~(EDIT: see #1161 (comment)) (separate PR would be preferrable)

Yes that should work but with vzNAT there will be other issues like VPN won't be working.

Also i faced issues with calls to host dns resolver via UDP didn't work

Thanks, let's keep using gvisor netstack by default, and revisit VZNAT in a separate PR

AkihiroSuda · 2022-11-08T04:29:17Z

pkg/vz/vm_darwin.go

+	//Other networks
+	for _, network := range driver.Yaml.Networks {
+		//TODO - Handle NAT network type
+		if network.Socket != "" {


The network.Socket value does not seem consumed

I expected it could be supported via VZFileHandleNetworkDeviceAttachment , but seems incompatible with QEMU:
https://developer.apple.com/documentation/virtualization/vzfilehandlenetworkdeviceattachment?language=objc

So we have to error out when network.Socket is non-empty

I think we can support it, maybe some extra support might be needed from socker_vmnet as well. I will try to support this in a different PR.

AkihiroSuda · 2022-11-08T04:31:53Z

pkg/vz/vm_darwin.go

+				case vz.VirtualMachineStateStopped:
+					logrus.Println("[VZ] - VM state change: stopped")
+					cancelHa()
+				}


Every switch{} should have default:

Done. By default doing a debug log.

AkihiroSuda · 2022-11-08T04:46:25Z

pkg/vz/vz_driver_darwin.go

+	if *l.Yaml.MountType == limayaml.NINEP {
+		return fmt.Errorf("field `mountType` must be %q or %q for VZ driver , got %q", limayaml.REVSSHFS, limayaml.VIRTIOFS, *l.Yaml.MountType)
+	}
+	return nil


Warnings should be printed for ignored YAML propertries: https://github.com/containerd/nerdctl/blob/c1a4159035b77a4a739a503b98139a17ac3440b8/pkg/composer/serviceparser/serviceparser.go#L38

Can be another PR though

Sure. Will take care of this in follow-up PR

Added errors for not supported one like legacyBios. As vm will not even start with this as true.
For other things will take it up in a follow-up PR like networks, mount as 9p etc.

Code-Hex · 2022-11-08T12:00:37Z

pkg/vz/vm_darwin.go

+
+func getEFI(driver *driver.BaseDriver) (*vz.EFIVariableStore, error) {
+	efi := filepath.Join(driver.Instance.Dir, filenames.VzEfi)
+	if _, err := os.Open(efi); os.IsNotExist(err) {


nits: I think Stat is better because if the file exists, it will not be closed until it is GC'd.

Code-Hex · 2022-11-08T12:13:49Z

pkg/vz/vz_driver_darwin.go

+	vm, err := startVM(ctx, l.BaseDriver)
+	if err != nil {
+		return err
+	}


I think error handling would be required to check version compatibility. Given that lima-vm will be built with the latest OS, handling of vz.ErrBuildTargetOSVersion is unnecessary. (maybe)
https://github.com/Code-Hex/vz#version-compatibility-check

@Code-Hex
Lima will only support macOS 13 based EFILoader. Not the old ones. With that, i believe we don't need to do any check right ?
If either the driver is disabled / os version is incompatible, vz already throws a appropriate error. Am i missing something ?

@balajiv113 Right. However, will be returned error might be not helpful for user side. So I meant to return an error with message which is helpful that this driver cannot use below macOS 13. (Or better logging?)

Sure will add that as well

shyim · 2022-11-08T15:37:08Z

pkg/vz/vm_darwin.go

+	return nil
+}
+
+func attachOtherDevices(_ *driver.BaseDriver, vmConfig *vz.VirtualMachineConfiguration) error {


does it make sense to register there also NewLinuxRosettaDirectoryShare

So we have Rosetta inside VZ? https://github.com/Code-Hex/vz/blob/c436fc367107393c6eda419ff0f69903f674c118/shared_directory_arm64.go#L63

Can be another PR

balajiv113 · 2022-11-10T14:47:13Z

Done testing with all the templates,
There are 2 open issues that we might need to address before merging,

templates with legacyBoot: true is not working with vz driver. If any pointer please do suggest.
kernel boot logs are not coming via serial attachment with UEFI. @Code-Hex, please do suggest if am missing something.

AkihiroSuda · 2022-11-10T23:56:21Z

No need to support legacy BIOS (in this PR, at least).
Could you document that restriction in a documentation like docs/vmtype.md ?

Also please consider squashing commits

Code-Hex · 2022-11-11T08:08:21Z

@balajiv113 I believe it's correct behavior that It's started up and it does not appear kernel early messages on the serial attachment because the console is provided via virtio-pci (not UART). This is undocumented officially.

Note that Virtualization.framework provides all IO as virtio-pci, including the console (i.e. not a UART).

https://github.com/evansm7/vftool#kernelsnotes

However, as noted on the wiki, some messages are displayed.

It's not possible to capture early boot messages with it. It starts working around the time /dev is populated. (by using mdev, sdev, etc.)

https://github.com/Code-Hex/vz/wiki#linux-kernel-requirements

AkihiroSuda · 2022-11-14T23:15:44Z

@chancez Thanks, could you open your rosetta PR after merging this base PR?

One consideration, if we're going add VM-type specific configuration (options specific to VZ or Qemu), perhaps vmType should be made into a object with values.

We already have VM-specific configs in the top-level struct, so I guess we are too late to make that change 😞

lima/examples/default.yaml

Lines 212 to 219 in 06a4b33

    
           # Specify desired QEMU CPU type for each arch. 
        
           # You can see what options are available for host emulation with: `qemu-system-$(arch) -cpu help`. 
        
           # Setting of instructions is supported like this: "qemu64,+ssse3". 
        
           cpuType: 
        
             # 🟢 Builtin default: "cortex-a72" (or "host" when running on aarch64 host) 
        
             aarch64: null 
        
             # 🟢 Builtin default: "qemu64" (or "host" when running on x86_64 host) 
        
             x86_64: null

lima/examples/default.yaml

Lines 226 to 233 in 06a4b33

    
           video: 
        
             # QEMU display, e.g., "none", "cocoa", "sdl", "gtk", "default". 
        
             # Choosing "none" will hide the video output, and not show any window. 
        
             # Choosing "default" will pick the first available of: gtk, sdl, cocoa. 
        
             # As of QEMU v6.2, enabling this is known to have negative impact 
        
             # on performance on macOS hosts: https://gitlab.com/qemu-project/qemu/-/issues/334 
        
             # 🟢 Builtin default: "none" 
        
             display: null

chancez · 2022-11-15T00:12:48Z

@chancez Thanks, could you open your rosetta PR after merging this base PR?

Yeah I'll make a PR with some adjustments to make it closer to "ready" and we can discuss the rest a bit further in that PR.

AkihiroSuda · 2022-11-15T04:18:05Z

Sorry, needs another rebase

balajiv113 · 2022-11-15T04:35:40Z

Done the rebase.
Also addressed @chancez comments about QEMU log during delete instance and also updated printf with infof

pkg/limayaml/defaults.go

pkg/qemu/imgutil/imgutil.go

cmd/limactl/stop.go

docs/mount.md

added support for drivers to lima, migrated exiting qemu to drivers modal and support for apple virtualization.framework as new driver Signed-off-by: Balaji Vijayakumar <kuttibalaji.v6@gmail.com>

abiosoft · 2022-11-16T05:40:47Z

docs/vmtype.md

+
+### Known Issues
+- "vz" doesn't support `legacyBoot: true` option, so guest machine like centos-stream, archlinux, oraclelinux will not work
+- Host to guest networking (`networks` section in lima yaml) is not supported


Is this outrightly not supported by the driver, and with no workarounds?

Expected to be added as "vznat" network in a separate PR
#1147 (comment)

jandubois · 2022-11-16T06:00:32Z

I've been testing this PR today on an M1 machine running Ventura, and I saw this error when starting template://experimental/vz:

ERRO[0296] [hostagent] r.CreateEndpoint() = connection was refused

Which seems to originate from https://github.com/containers/gvisor-tap-vsock/blob/main/pkg/services/forwarder/tcp.go#L45.

Is this expected? There was no additional information in ha.stderr.log.

jandubois · 2022-11-16T06:06:37Z

This is being added as an experimental feature, so the implementation can be revised later at any time, but I'd like to make sure that we are fine with the YAML property (Just vmType: "vz")

I'm fine with the vmType property, and fine with merging it now, and refining it later.

I've not had time to review the code in detail, but so far it all looks good, and most importantly: works. I've only tested on M1 so far, but should be able to get remote access to an Intel Ventura machine for testing as well.

balajiv113 force-pushed the driver branch from 155d06d to 2d4a68a Compare November 7, 2022 05:49

AkihiroSuda reviewed Nov 7, 2022

View reviewed changes

pkg/limayaml/limayaml.go Show resolved Hide resolved

AkihiroSuda reviewed Nov 7, 2022

View reviewed changes

pkg/qemu/imgutil/imgutil.go Show resolved Hide resolved

AkihiroSuda reviewed Nov 7, 2022

View reviewed changes

AkihiroSuda added this to the v0.14 (tentative) milestone Nov 7, 2022

AkihiroSuda added component/vz impact/changelog labels Nov 7, 2022

AkihiroSuda reviewed Nov 7, 2022

View reviewed changes

AkihiroSuda mentioned this pull request Nov 7, 2022

Optional support for Virtualization.framework (on macOS 13) #889

Closed

AkihiroSuda linked an issue Nov 7, 2022 that may be closed by this pull request

Optional support for Virtualization.framework (on macOS 13) #889

Closed

AkihiroSuda reviewed Nov 8, 2022

View reviewed changes

balajiv113 commented Nov 8, 2022

View reviewed changes

AkihiroSuda reviewed Nov 8, 2022

View reviewed changes

Code-Hex reviewed Nov 8, 2022

View reviewed changes

shyim reviewed Nov 8, 2022

View reviewed changes

balajiv113 force-pushed the driver branch 2 times, most recently from 575880e to 1300c9c Compare November 11, 2022 14:34

chancez mentioned this pull request Nov 15, 2022

Support Rosetta with VZ driver #1155

Merged

balajiv113 dismissed AkihiroSuda’s stale review via 1352e5f November 15, 2022 04:34

balajiv113 force-pushed the driver branch from 284cd48 to 1352e5f Compare November 15, 2022 04:34

AkihiroSuda previously approved these changes Nov 15, 2022

View reviewed changes

jandubois reviewed Nov 15, 2022

View reviewed changes

pkg/limayaml/defaults.go Outdated Show resolved Hide resolved

jandubois reviewed Nov 15, 2022

View reviewed changes

pkg/qemu/imgutil/imgutil.go Outdated Show resolved Hide resolved

jandubois reviewed Nov 15, 2022

View reviewed changes

cmd/limactl/stop.go Show resolved Hide resolved

jandubois reviewed Nov 15, 2022

View reviewed changes

docs/mount.md Outdated Show resolved Hide resolved

Support for Virtualization.Framework driver

2679326

added support for drivers to lima, migrated exiting qemu to drivers modal and support for apple virtualization.framework as new driver Signed-off-by: Balaji Vijayakumar <kuttibalaji.v6@gmail.com>

balajiv113 dismissed AkihiroSuda’s stale review via 2679326 November 15, 2022 08:18

balajiv113 force-pushed the driver branch from 1352e5f to 2679326 Compare November 15, 2022 08:18

AkihiroSuda approved these changes Nov 15, 2022

View reviewed changes

AkihiroSuda requested a review from jandubois November 15, 2022 22:50

abiosoft reviewed Nov 16, 2022

View reviewed changes

AkihiroSuda merged commit 702261b into lima-vm:master Nov 16, 2022

This was referenced Nov 16, 2022

vz: dismiss [hostagent] r.CreateEndpoint() = connection was refused #1160

Closed

vz: support "vznat" network #1161

Closed

vz: warn ignored YAML fields #1162

Closed

vz: update docs #1163

Merged

balajiv113 deleted the driver branch November 16, 2022 06:51

abiosoft mentioned this pull request Nov 16, 2022

vz driver not mounting data disk lima-vm/alpine-lima#94

Closed

balajiv113 mentioned this pull request Nov 16, 2022

support for driver specific pid files #1171

Merged

abiosoft mentioned this pull request Nov 16, 2022

vz: host resolver not working #1172

Closed

Sebor mentioned this pull request Feb 22, 2023

virtualization.framework option on MacOS rancher-sandbox/rancher-desktop#3537

Closed


		err = machine.Start()

		networks.StartGVisor(ctx, &networks.GVisorOpts{

Conversation

balajiv113 commented Nov 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balajiv113 commented Nov 7, 2022 •

edited

Loading

AkihiroSuda Nov 8, 2022 •

edited

Loading

AkihiroSuda Nov 8, 2022 •

edited

Loading

AkihiroSuda Nov 8, 2022 •

edited

Loading

AkihiroSuda Nov 8, 2022 •

edited

Loading

Code-Hex Nov 8, 2022 •

edited

Loading