Deprecate network.vde and create networks fields in lima.yaml

Signed-off-by: Jan Dubois <[email protected]>

Author: jandubois

pkg/cidata/cidata.TEMPLATE.d/network-config

@@ -1,12 +1,12 @@
 version: 2
 ethernets:
   {{- range $nw := .Networks}}
-  {{$nw.Name}}:
+  {{$nw.Interface}}:
     match:
       macaddress: '{{$nw.MACAddress}}'
     dhcp4: true
-    set-name: {{$nw.Name}}
-    {{- if and (eq $nw.Name $.SlirpNICName) (gt (len $.DNSAddresses) 0) }}
+    set-name: {{$nw.Interface}}
+    {{- if and (eq $nw.Interface $.SlirpNICName) (gt (len $.DNSAddresses) 0) }}
     nameservers:
       addresses:
       {{- range $ns := $.DNSAddresses }}

pkg/cidata/cidata.go

@@ -77,9 +77,9 @@ func GenerateISO9660(instDir, name string, y *limayaml.LimaYAML) error {
 	}
 
 	slirpMACAddress := limayaml.MACAddress(instDir)
-	args.Networks = append(args.Networks, Network{MACAddress: slirpMACAddress, Name: qemuconst.SlirpNICName})
-	for _, vde := range y.Network.VDE {
-		args.Networks = append(args.Networks, Network{MACAddress: vde.MACAddress, Name: vde.Name})
+	args.Networks = append(args.Networks, Network{MACAddress: slirpMACAddress, Interface: qemuconst.SlirpNICName})
+	for _, nw := range y.Networks {
+		args.Networks = append(args.Networks, Network{MACAddress: nw.MACAddress, Interface: nw.Interface})
 	}
 
 	if len(y.DNS) > 0 {

pkg/cidata/template.go

@@ -25,7 +25,7 @@ type Containerd struct {
 }
 type Network struct {
 	MACAddress string
-	Name       string
+	Interface  string
 }
 type TemplateArgs struct {
 	Name         string // instance name

pkg/limayaml/defaults.go

@@ -72,14 +72,27 @@ func FillDefault(y *LimaYAML, filePath string) {
 		FillPortForwardDefaults(&y.PortForwards[i])
 		// After defaults processing the singular HostPort and GuestPort values should not be used again.
 	}
-	for i := range y.Network.VDE {
-		vde := &y.Network.VDE[i]
-		if vde.MACAddress == "" {
+
+	if len(y.Network.VDEDeprecated) > 0 && len(y.Networks) == 0 {
+		for _, vde := range y.Network.VDEDeprecated {
+			network := Network{
+				Interface: vde.Name,
+				MACAddress: vde.MACAddress,
+				SwitchPort: vde.SwitchPort,
+				VNL: vde.VNL,
+			}
+			y.Networks = append(y.Networks, network)
+		}
+		y.Network.migrated = true
+	}
+	for i := range y.Networks {
+		nw := &y.Networks[i]
+		if nw.MACAddress == "" {
 			// every interface in every limayaml file must get its own unique MAC address
-			vde.MACAddress = MACAddress(fmt.Sprintf("%s#%d", filePath, i))
+			nw.MACAddress = MACAddress(fmt.Sprintf("%s#%d", filePath, i))
 		}
-		if vde.Name == "" {
-			vde.Name = "vde" + strconv.Itoa(i)
+		if nw.Interface == "" {
+			nw.Interface = "lima" + strconv.Itoa(i)
 		}
 	}
 }

pkg/limayaml/limayaml.go

@@ -20,8 +20,9 @@ type LimaYAML struct {
 	Containerd   Containerd         `yaml:"containerd,omitempty"`
 	Probes       []Probe            `yaml:"probes,omitempty"`
 	PortForwards []PortForward      `yaml:"portForwards,omitempty"`
-	Network      Network            `yaml:"network,omitempty"`
-	Env          map[string]*string `yaml:"env,omitempty"` // EXPERIMENAL, see default.yaml
+	Networks     []Network          `yaml:"networks,omitempty"`
+	Network      NetworkDeprecated  `yaml:"network,omitempty"` // DEPRECATED, use `networks` instead
+	Env          map[string]*string `yaml:"env,omitempty"`     // EXPERIMENAL, see default.yaml
 	DNS          []net.IP           `yaml:"dns,omitempty"`
 }
 
@@ -110,11 +111,28 @@ type PortForward struct {
 }
 
 type Network struct {
-	VDE []VDE `yaml:"vde,omitempty"`
-}
-type VDE struct {
+	// `Lima` and `VNL` are mutually exclusive; exactly one is required
+	Lima string `yaml:"lima,omitempty"`
 	// VNL is a Virtual Network Locator (https://github.com/rd235/vdeplug4/commit/089984200f447abb0e825eb45548b781ba1ebccd).
 	// On macOS, only VDE2-compatible form (optionally with vde:// prefix) is supported.
+	VNL        string `yaml:"vnl,omitempty"`
+	SwitchPort uint16 `yaml:"switchPort,omitempty"` // VDE Switch port, not TCP/UDP port (only used by VDE networking)
+	MACAddress string `yaml:"macAddress,omitempty"`
+	Interface  string `yaml:"interface,omitempty"`
+}
+
+// DEPRECATED types below
+
+// Types have been renamed to turn all references to the old names into compiler errors,
+// and to avoid accidental usage in new code.
+
+type NetworkDeprecated struct {
+	VDEDeprecated []VDEDeprecated `yaml:"vde,omitempty"`
+	// migrate will be true when `network.VDE` has been copied to `networks` by FillDefaults()
+	migrated bool
+}
+
+type VDEDeprecated struct {
 	VNL        string `yaml:"vnl,omitempty"`
 	SwitchPort uint16 `yaml:"switchPort,omitempty"` // VDE Switch port, not TCP/UDP port
 	MACAddress string `yaml:"macAddress,omitempty"`

pkg/limayaml/validate.go

@@ -160,86 +160,96 @@ func Validate(y LimaYAML) error {
 		// processed sequentially and the first matching rule for a guest port determines forwarding behavior.
 	}
 
-	if err := validateNetwork(y.Network); err != nil {
+	if err := validateNetwork(y); err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func validateNetwork(yNetwork Network) error {
-	networkName := make(map[string]int)
-	for i, vde := range yNetwork.VDE {
-		field := fmt.Sprintf("network.vde[%d]", i)
-		if vde.VNL == "" {
-			return fmt.Errorf("field `%s.vnl` must not be empty", field)
+func validateNetwork(y LimaYAML) error {
+	if len(y.Network.VDEDeprecated) > 0 {
+		if y.Network.migrated {
+			logrus.Warnf("field `network.VDE` is deprecated; please use `networks` instead")
+		} else {
+			return fmt.Errorf("you cannot use deprecated field `network.VDE` together with replacement field `networks`")
 		}
-		// The field is called VDE.VNL in anticipation of QEMU upgrading VDE2 to VDEplug4,
-		// but right now the only valid value on macOS is a path to the vde_switch socket directory,
-		// optionally with vde:// prefix.
-		// TODO: use networks.LimaSchema after solving circular dependency
-		if strings.HasPrefix(vde.VNL, "lima://") {
-			// TODO: validate network names? Problem is we can't use "networks" or "store" packages here...
-			//name := strings.TrimPrefix(vde.VNL, networks.LimaScheme)
-			//if _, ok := networkConfig.Networks[name]; !ok {
-			//	return fmt.Errorf("field `%s.vnl` references undefined network %q", field, name)
-			//}
-		} else if !strings.Contains(vde.VNL, "://") || strings.HasPrefix(vde.VNL, "vde://") {
-			vdeSwitch := strings.TrimPrefix(vde.VNL, "vde://")
-			if fi, err := os.Stat(vdeSwitch); err != nil {
-				// negligible when the instance is stopped
-				logrus.WithError(err).Debugf("field `%s.vnl` %q failed stat", field, vdeSwitch)
-			} else {
-				if fi.IsDir() {
-					/* Switch mode (vdeSwitch is dir, port != 65535) */
-					ctlSocket := filepath.Join(vdeSwitch, "ctl")
-					// ErrNotExist during os.Stat(ctlSocket) can be ignored. ctlSocket does not need to exist until actually starting the VM
-					if fi, err = os.Stat(ctlSocket); err == nil {
+	}
+	interfaceName := make(map[string]int)
+	for i, nw := range y.Networks {
+		field := fmt.Sprintf("networks[%d]", i)
+		if nw.Lima != "" {
+			if nw.VNL != "" {
+				return fmt.Errorf("field `%s.lima` and field `%s.vnl` are mutually exclusive", field, field)
+			}
+			if nw.SwitchPort != 0 {
+				return fmt.Errorf("field `%s.switchPort` cannot be used with field `%s.lima`", field, field)
+			}
+			// TODO: validate network name (we can't use networks and store packages right now)
+		} else {
+			if nw.VNL == "" {
+				return fmt.Errorf("field `%s.lima` or field `%s.vnl` must be set", field, field)
+			}
+			// The field is called VDE.VNL in anticipation of QEMU upgrading VDE2 to VDEplug4,
+			// but right now the only valid value on macOS is a path to the vde_switch socket directory,
+			// optionally with vde:// prefix.
+			if !strings.Contains(nw.VNL, "://") || strings.HasPrefix(nw.VNL, "vde://") {
+				vdeSwitch := strings.TrimPrefix(nw.VNL, "vde://")
+				if fi, err := os.Stat(vdeSwitch); err != nil {
+					// negligible when the instance is stopped
+					logrus.WithError(err).Debugf("field `%s.vnl` %q failed stat", field, vdeSwitch)
+				} else {
+					if fi.IsDir() {
+						/* Switch mode (vdeSwitch is dir, port != 65535) */
+						ctlSocket := filepath.Join(vdeSwitch, "ctl")
+						// ErrNotExist during os.Stat(ctlSocket) can be ignored. ctlSocket does not need to exist until actually starting the VM
+						if fi, err = os.Stat(ctlSocket); err == nil {
+							if fi.Mode()&os.ModeSocket == 0 {
+								return fmt.Errorf("field `%s.vnl` file %q is not a UNIX socket", field, ctlSocket)
+							}
+						}
+						if nw.SwitchPort == 65535 {
+							return fmt.Errorf("field `%s.vnl` points to a non-PTP switch, so the port number must not be 65535", field)
+						}
+					} else {
+						/* PTP mode (vdeSwitch is socket, port == 65535) */
 						if fi.Mode()&os.ModeSocket == 0 {
-							return fmt.Errorf("field `%s.vnl` file %q is not a UNIX socket", field, ctlSocket)
+							return fmt.Errorf("field `%s.vnl` %q is not a directory nor a UNIX socket", field, vdeSwitch)
+						}
+						if nw.SwitchPort != 65535 {
+							return fmt.Errorf("field `%s.vnl` points to a PTP (switchless) socket %q, so the port number has to be 65535 (got %d)",
+								field, vdeSwitch, nw.SwitchPort)
 						}
-					}
-					if vde.SwitchPort == 65535 {
-						return fmt.Errorf("field `%s.vnl` points to a non-PTP switch, so the port number must not be 65535", field)
-					}
-				} else {
-					/* PTP mode (vdeSwitch is socket, port == 65535) */
-					if fi.Mode()&os.ModeSocket == 0 {
-						return fmt.Errorf("field `%s.vnl` %q is not a directory nor a UNIX socket", field, vdeSwitch)
-					}
-					if vde.SwitchPort != 65535 {
-						return fmt.Errorf("field `%s.vnl` points to a PTP (switchless) socket %q, so the port number has to be 65535 (got %d)",
-							field, vdeSwitch, vde.SwitchPort)
 					}
 				}
+			} else if runtime.GOOS != "linux" {
+				logrus.Warnf("field `%s.vnl` is unlikely to work for %s (unless libvdeplug4 has been ported to %s and is installed)",
+					field, runtime.GOOS, runtime.GOOS)
 			}
-		} else if runtime.GOOS != "linux" {
-			logrus.Warnf("field `%s.vnl` is unlikely to work for %s (unless libvdeplug4 has been ported to %s and is installed)",
-				field, runtime.GOOS, runtime.GOOS)
 		}
-		if vde.MACAddress != "" {
-			hw, err := net.ParseMAC(vde.MACAddress)
+		if nw.MACAddress != "" {
+			hw, err := net.ParseMAC(nw.MACAddress)
 			if err != nil {
 				return fmt.Errorf("field `vmnet.mac` invalid: %w", err)
 			}
 			if len(hw) != 6 {
-				return fmt.Errorf("field `%s.macAddress` must be a 48 bit (6 bytes) MAC address; actual length of %q is %d bytes", field, vde.MACAddress, len(hw))
+				return fmt.Errorf("field `%s.macAddress` must be a 48 bit (6 bytes) MAC address; actual length of %q is %d bytes", field, nw.MACAddress, len(hw))
 			}
 		}
-		// FillDefault() will make sure that vde.Name is not the empty string
-		if len(vde.Name) >= 16 {
-			return fmt.Errorf("field `%s.name` must be less than 16 bytes, but is %d bytes: %q", field, len(vde.Name), vde.Name)
+		// FillDefault() will make sure that nw.Interface is not the empty string
+		if len(nw.Interface) >= 16 {
+			return fmt.Errorf("field `%s.interface` must be less than 16 bytes, but is %d bytes: %q", field, len(nw.Interface), nw.Interface)
 		}
-		if strings.ContainsAny(vde.Name, " \t\n/") {
-			return fmt.Errorf("field `%s.name` must not contain whitespace or slashes", field)
+		if strings.ContainsAny(nw.Interface, " \t\n/") {
+			return fmt.Errorf("field `%s.interface` must not contain whitespace or slashes", field)
 		}
-		if vde.Name == qemuconst.SlirpNICName {
-			return fmt.Errorf("field `%s.name` must not be set to %q because it is reserved for slirp", field, qemuconst.SlirpNICName)
+		if nw.Interface == qemuconst.SlirpNICName {
+			return fmt.Errorf("field `%s.interface` must not be set to %q because it is reserved for slirp", field, qemuconst.SlirpNICName)
 		}
-		if prev, ok := networkName[vde.Name]; ok {
-			return fmt.Errorf("field `%s.name` value %q has already been used by field `network.vde[%d].name`", field, vde.Name, prev)
+		if prev, ok := interfaceName[nw.Interface]; ok {
+			return fmt.Errorf("field `%s.interface` value %q has already been used by field `network.vde[%d].name`", field, nw.Interface, prev)
 		}
-		networkName[vde.Name] = i
+		interfaceName[nw.Interface] = i
 	}
 	return nil
 }

pkg/networks/config.go

@@ -7,7 +7,6 @@ import (
 	"gopkg.in/yaml.v2"
 	"os"
 	"path/filepath"
-	"strings"
 	"sync"
 
 	"github.com/lima-vm/lima/pkg/store"
@@ -66,16 +65,12 @@ func Config() (NetworksConfig, error) {
 }
 
 func VDESock(name string) (string, error) {
-	if strings.HasPrefix(name, LimaScheme) {
-		load()
-		if cache.err != nil {
-			return "", cache.err
-		}
-		name = strings.TrimPrefix(name, LimaScheme)
-		if err := cache.config.Check(name); err != nil {
-			return "", err
-		}
-		return cache.config.VDESock(name), nil
+	load()
+	if cache.err != nil {
+		return "", cache.err
+	}
+	if err := cache.config.Check(name); err != nil {
+		return "", err
 	}
-	return name, nil
+	return cache.config.VDESock(name), nil
 }

pkg/networks/reconcile.go

@@ -34,15 +34,15 @@ func Reconcile(ctx context.Context, newInst string) error {
 		if instance.Status != store.StatusRunning && instName != newInst {
 			continue
 		}
-		for _, vde := range instance.Network.VDE {
-			if strings.HasPrefix(vde.VNL, LimaScheme) {
-				name := strings.TrimPrefix(vde.VNL, LimaScheme)
-				if _, ok := config.Networks[name]; !ok {
-					logrus.Errorf("network %q (used by instance %q) is missing from networks.yaml", name, instName)
-					continue
-				}
-				activeNetwork[name] = true
+		for _, nw := range instance.Networks {
+			if nw.Lima == "" {
+				continue
 			}
+			if _, ok := config.Networks[nw.Lima]; !ok {
+				logrus.Errorf("network %q (used by instance %q) is missing from networks.yaml", nw.Lima, instName)
+				continue
+			}
+			activeNetwork[nw.Lima] = true
 		}
 	}
 	for name := range config.Networks {

pkg/qemu/qemu.go

@@ -260,36 +260,47 @@ func Cmdline(cfg Config) (string, []string, error) {
 	args = append(args, "-netdev", fmt.Sprintf("user,id=net0,net=%s,dhcpstart=%s,hostfwd=tcp:127.0.0.1:%d-:22",
 		qemuconst.SlirpNetwork, qemuconst.SlirpIPAddress, y.SSH.LocalPort))
 	args = append(args, "-device", "virtio-net-pci,netdev=net0,mac="+limayaml.MACAddress(cfg.InstanceDir))
-	if len(y.Network.VDE) > 0 && !strings.Contains(string(features.NetdevHelp), "vde") {
+	usingVDE := false
+	for _, nw := range y.Networks {
+		if nw.VNL != "" {
+			usingVDE = true
+		}
+	}
+	if usingVDE && !strings.Contains(string(features.NetdevHelp), "vde") {
 		return "", nil, fmt.Errorf("netdev \"vde\" is not supported by %s ( Hint: recompile QEMU with `configure --enable-vde` )", exe)
 	}
-	for i, vde := range y.Network.VDE {
-		// Replace internal lima:// network name with proper socket path
-		vdeSock, err := networks.VDESock(vde.VNL)
-		if err != nil {
-			return "", nil, err
-		}
-		// VDE4 accepts VNL like vde:///var/run/vde.ctl as well as file path like /var/run/vde.ctl .
-		// VDE2 only accepts the latter form.
-		// VDE2 supports macOS but VDE4 does not yet, so we trim vde:// prefix here for VDE2 compatibility.
-		vdeSock = strings.TrimPrefix(vdeSock, "vde://")
-		if !strings.Contains(vdeSock, "://") {
-			if _, err := os.Stat(vdeSock); err != nil {
-				return "", nil, fmt.Errorf("cannot use VNL %q: %w", vde.VNL, err)
-			}
-			// vdeSock is a directory, unless vde.SwitchPort == 65535 (PTP)
-			actualSocket := filepath.Join(vdeSock, "ctl")
-			if vde.SwitchPort == 65535 { // PTP
-				actualSocket = vdeSock
+	for i, nw := range y.Networks {
+		var vdeSock string
+		if nw.Lima != "" {
+			vdeSock, err = networks.VDESock(nw.Lima)
+			if err != nil {
+				return "", nil, err
 			}
-			if st, err := os.Stat(actualSocket); err != nil {
-				return "", nil, fmt.Errorf("cannot use VNL %q: failed to stat %q: %w", vde.VNL, actualSocket, err)
-			} else if st.Mode()&fs.ModeSocket == 0 {
-				return "", nil, fmt.Errorf("cannot use VNL %q: %q is not a socket: %w", vde.VNL, actualSocket, err)
+			// TODO: should we also validate that the socket exists, or do we rely on the
+			// networks reconciler to throw an error when the network cannot start?
+		} else {
+			// VDE4 accepts VNL like vde:///var/run/vde.ctl as well as file path like /var/run/vde.ctl .
+			// VDE2 only accepts the latter form.
+			// VDE2 supports macOS but VDE4 does not yet, so we trim vde:// prefix here for VDE2 compatibility.
+			vdeSock = strings.TrimPrefix(nw.VNL, "vde://")
+			if !strings.Contains(vdeSock, "://") {
+				if _, err := os.Stat(vdeSock); err != nil {
+					return "", nil, fmt.Errorf("cannot use VNL %q: %w", nw.VNL, err)
+				}
+				// vdeSock is a directory, unless vde.SwitchPort == 65535 (PTP)
+				actualSocket := filepath.Join(vdeSock, "ctl")
+				if nw.SwitchPort == 65535 { // PTP
+					actualSocket = vdeSock
+				}
+				if st, err := os.Stat(actualSocket); err != nil {
+					return "", nil, fmt.Errorf("cannot use VNL %q: failed to stat %q: %w", nw.VNL, actualSocket, err)
+				} else if st.Mode()&fs.ModeSocket == 0 {
+					return "", nil, fmt.Errorf("cannot use VNL %q: %q is not a socket: %w", nw.VNL, actualSocket, err)
+				}
 			}
 		}
 		args = append(args, "-netdev", fmt.Sprintf("vde,id=net%d,sock=%s", i+1, vdeSock))
-		args = append(args, "-device", fmt.Sprintf("virtio-net-pci,netdev=net%d,mac=%s", i+1, vde.MACAddress))
+		args = append(args, "-device", fmt.Sprintf("virtio-net-pci,netdev=net%d,mac=%s", i+1, nw.MACAddress))
 	}
 
 	// virtio-rng-pci accelerates starting up the OS, according to https://wiki.gentoo.org/wiki/QEMU/Options