Deep Review: 20260408-175512-pr-280

	return ln, nil
}

// getVMGUID discovers the Hyper-V VM running our distro by polling the
// registry for VM GUIDs and handshaking with each in parallel. The registry
// is rescanned every second so that VMs starting after the initial scan
// (e.g., the WSL2 utility VM on a fresh system) are discovered.
//
// The signature-based discovery assumes only one opensuse WSL2 instance
// runs at a time. With multiple instances, the first match wins and the
// others get no host-switch networking.
func getVMGUID(ctx context.Context, logger logr.Logger) (hvsock.GUID, error) {
	ctx, cancel := context.WithCancel(ctx)
	defer cancel()

	found := make(chan hvsock.GUID, 1)
	seen := make(map[hvsock.GUID]bool)

	scanRegistry := func() {
		key, err := registry.OpenKey(
			registry.LOCAL_MACHINE,
			`SOFTWARE\Microsoft\Windows NT\CurrentVersion\HostComputeService\VolatileStore\ComputeSystem`,
			registry.ENUMERATE_SUB_KEYS)
		if err != nil {
			return // Registry key not present yet; retry on next tick.
		}
		names, err := key.ReadSubKeyNames(0)
		key.Close()
		if err != nil {
			return
		}
		for _, name := range names {
			vmGUID, err := hvsock.GUIDFromString(name)
			if err != nil {
				logger.V(1).Info("Skipping invalid VM GUID", "name", name, "error", err)
				continue
			}
			if !seen[vmGUID] {
				seen[vmGUID] = true
				go attemptHandshake(ctx, logger, vmGUID, found)
			}
		}
	}

	ticker := time.NewTicker(time.Second)
	defer ticker.Stop()

	// Immediate first scan, then rescan on each tick.
	scanRegistry()
	for {
		select {
		case vmGUID := <-found:
			return vmGUID, nil
		case <-ctx.Done():
			return hvsock.GUIDZero, fmt.Errorf("VM GUID discovery timed out: %w", ctx.Err())
		case <-ticker.C:
			scanRegistry()
		}
	}
}

// attemptHandshake polls a single VM GUID once per second, trying to match
// the signature phrase. Each probe runs synchronously to avoid goroutine
// leaks and panics from sending on a closed channel.
func attemptHandshake(ctx context.Context, logger logr.Logger, vmGUID hvsock.GUID, found chan<- hvsock.GUID) {
	ticker := time.NewTicker(time.Second)
	defer ticker.Stop()

	for {
		conn, err := getVsockConnection(vmGUID, vsockHandshakePort)
		if ctx.Err() != nil {
			if conn != nil {
				conn.Close()
			}
			return
		}
		if err != nil {
			logger.V(1).Info("Handshake dial failed", "vmGUID", vmGUID.String(), "error", err)
		} else {
			sig, err := readSignature(conn)
			conn.Close()
			if err != nil {
				logger.V(1).Info("Handshake read failed", "vmGUID", vmGUID.String(), "error", err)
			} else if sig == signaturePhrase {
				logger.V(1).Info("Signature matched", "vmGUID", vmGUID.String())
				select {
				case found <- vmGUID:
				default:
				}
				return
			} else {
				// Valid signature from a different distro; no point retrying.
				logger.V(1).Info("Signature mismatch", "vmGUID", vmGUID.String())
				return
			}
		}

		select {
		case <-ctx.Done():
			return
		case <-ticker.C:
		}

getVMGUID matches the first VM that sends the fixed signaturePhrase (line 98). If two WSL2 VMs run the opensuse distro simultaneously, two host-switch goroutines could bind to the same guest while the other gets no networking. In the current architecture (one App, one LimaVM, one WSL2 instance), this cannot happen. But the code's structure — one host-switch per instance name, map-based state tracking — implies multi-instance support.

	handshakeTimeout   = 5 * time.Minute

	// signaturePhrase identifies our distro among all running Hyper-V VMs.
	// This value is a protocol contract with the guest-side network-setup
	// binary and must not be changed independently.
	signaturePhrase = "github.com/rancher-sandbox/rancher-desktop/src/go/networking"
	readySignal     = "READY"

	gatewayMacAddr = "5a:94:ef:e4:0c:dd"
	defaultMTU     = 1500
)

Fix: Add a comment documenting the single-instance constraint.

 // getVMGUID enumerates running Hyper-V VMs from the registry and handshakes
-// with each in parallel to find the one running our distro.
+// with each in parallel to find the one running our distro. This assumes
+// only one opensuse WSL2 instance runs at a time; with multiple instances,
+// the signature-based discovery would be ambiguous.

	if inst.VMType != limatype.WSL2 {
		return
	}

	r.stopHostSwitch(name)

	hsCtx, hsCancel := context.WithCancel(ctx)
	done := make(chan struct{})

	r.hostSwitchMu.Lock()
	r.hostSwitchStates[name] = &hostSwitchState{
		cancel: hsCancel,
		done:   done,
	}
	r.hostSwitchMu.Unlock()

	logger := logr.FromContextOrDiscard(ctx).WithValues("instance", name, "component", "host-switch")
	go r.runHostSwitch(hsCtx, logger, done)
}

The inline cancel-and-wait duplicates stopHostSwitch's logic. The unlock-wait-relock pattern also opens a narrow window where a concurrent shutdownAllHostagents call could delete the old entry from the map, wait on it, and then startHostSwitch would insert a new running entry — leaving a host-switch the manager believes it stopped. In practice this requires a shutdown racing with a startup, which is extremely unlikely.

Fix: Delegate to the existing stopHostSwitch method.

-	r.hostSwitchMu.Lock()
-	if old, exists := r.hostSwitchStates[name]; exists {
-		r.hostSwitchMu.Unlock()
-		old.cancel()
-		<-old.done
-		r.hostSwitchMu.Lock()
-	}
+	r.stopHostSwitch(name)
+
+	r.hostSwitchMu.Lock()
 	r.hostSwitchStates[name] = &hostSwitchState{

	ctx, cancel := context.WithCancel(ctx)
	defer cancel()

	found := make(chan hvsock.GUID, 1)
	seen := make(map[hvsock.GUID]bool)

	scanRegistry := func() {
		key, err := registry.OpenKey(
			registry.LOCAL_MACHINE,
			`SOFTWARE\Microsoft\Windows NT\CurrentVersion\HostComputeService\VolatileStore\ComputeSystem`,
			registry.ENUMERATE_SUB_KEYS)
		if err != nil {
			return // Registry key not present yet; retry on next tick.
		}
		names, err := key.ReadSubKeyNames(0)
		key.Close()
		if err != nil {
			return
		}
		for _, name := range names {
			vmGUID, err := hvsock.GUIDFromString(name)
			if err != nil {
				logger.V(1).Info("Skipping invalid VM GUID", "name", name, "error", err)
				continue
			}
			if !seen[vmGUID] {
				seen[vmGUID] = true
				go attemptHandshake(ctx, logger, vmGUID, found)
			}

The comment at lines 316-318 says the select "prevents an indefinite hang," but it prevents only the caller from hanging. If hvsock.Dial blocks forever, the background goroutine leaks. This is a known limitation of the hvsock library (no cancelable dial), already documented in the code. The goroutine typically leaks only during process shutdown, when it is about to be cleaned up anyway.

// runs at a time. With multiple instances, the first match wins and the
// others get no host-switch networking.
func getVMGUID(ctx context.Context, logger logr.Logger) (hvsock.GUID, error) {
	ctx, cancel := context.WithCancel(ctx)
	defer cancel()

	found := make(chan hvsock.GUID, 1)
	seen := make(map[hvsock.GUID]bool)

	scanRegistry := func() {
		key, err := registry.OpenKey(
			registry.LOCAL_MACHINE,
			`SOFTWARE\Microsoft\Windows NT\CurrentVersion\HostComputeService\VolatileStore\ComputeSystem`,

Fix: Correct the comment to avoid implying the goroutine is canceled.

 // signalListenerReady tells the guest that the data listener is ready.
-// The dial is wrapped in a goroutine because hvsock.Dial does not accept
-// a context; the select on ctx.Done prevents an indefinite hang if the
-// VM becomes unresponsive between discovery and signaling.
+// The dial is wrapped in a goroutine because hvsock.Dial does not accept
+// a context. The select on ctx.Done prevents the caller from hanging, but
+// the dial goroutine itself may leak if the VM becomes unresponsive.

validateSubnet and newVirtualNetworkConfig use only net.ParseCIDR and net.IPv4 — no Windows-specific APIs. Because they reside in hostswitch_windows.go, the tests run only on Windows CI. Extracting these functions and their types into a hostswitch_config.go (no build constraint) would let the tests run on all platforms.

AcceptStdio blocks until the connection closes, so the accept goroutine handles one connection at a time. This is correct because each VM runs a single vm-switch process — reconnections are serial (old connection EOF, then accept new connection). A clarifying comment would prevent a future contributor from "fixing" this with go vn.AcceptStdio(...).

+		// AcceptStdio blocks until the connection closes, so the loop
+		// handles one connection at a time. This is correct: each VM
+		// runs a single vm-switch process, and reconnections are serial.
 		if err := vn.AcceptStdio(ctx, conn); err != nil {

time.NewTicker fires its first event after the specified duration, so the first handshake probe is delayed by 1 second. Adding an immediate attempt before entering the ticker loop would shave a second off networking availability on every boot.

The doc describes the signature phrase as sufficient to identify "the correct" VM, which holds only while at most one matching opensuse guest is running. The text should document this single-instance constraint, consistent with S1.

validateSubnet hardcodes the fourth octet to .1, .2, and .254 without checking the mask. For a /25 subnet, .254 would fall outside the valid range. Since the function is called only with the hardcoded defaultSubnet = "192.168.127.0/24", this cannot happen today. A defensive assertion (ones != 24 → error) would guard against future misuse.