From bd9b2bef9e0e67edcaeb0ff0685d50d0e8a5769e Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 13 May 2026 14:34:53 +0800 Subject: [PATCH 1/5] feat(network): always provision per-VM netns under CNI, even with 0 NICs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CH/FC born from `vm run --nics 0` (or `vm clone --nics 0`) used to land in the host netns because initNetwork short-circuited and no per-VM netns was created. A later `vm net resize --nics N` was rejected by plumbingForVM's zero-NICs guard, because the CH process couldn't see NICs added into a netns it doesn't live in. The interface implied 0→N was supported by symmetry but it wasn't — that asymmetry is the bug. Move netns ownership onto the network module via a new Prepare step: - network.Network gains Prepare(ctx, vmID, vmCfg) (netnsPath, error). - CNI.Prepare ensureNetns idempotently and returns the path; "" when no conflist is configured. - Bridge.Prepare is a no-op (bridge stays in host netns by design). - initNetwork always calls Prepare regardless of NIC count. - Add is only called when nics > 0. Carry the result via a single types.NetSetup{Backend, NetnsPath, BridgeDev, NICs} struct. Hypervisor.Create/Clone/DirectClone now take NetSetup in place of `[]*NetworkConfig` so the same value flows from the network module to the persisted VM record without a transient duplicate slot on VMConfig. Persist NetBackend / NetnsPath / NetBridgeDev at types.VM level (per VM, not per NIC), with ResolvedNetX() fallbacks for old records that still carry these on NetworkConfigs[0]. start.go / restore.go / clone.go read rec.ResolvedNetnsPath() directly; the withNetwork len(rec.NetworkConfigs) > 0 gate is gone — it conflated "has NICs" with "needs netns entry", same bug one layer down. netresize/plumbingForVM and lifecycle/providerForVM dispatch via vm.ResolvedNetBackend() so a 0-NIC VM picks the right provider on resize-up; the "zero NICs; resize up not supported" hard error is gone. Bridge mode was always mechanically capable of 0→N (no netns concept); it now shares the same code path as CNI without the spurious guard. README updated to drop the "cannot resize up from zero" caveat. Adds TestVMResolvedNetFields covering VM-level, NIC[0]-fallback, and empty cases. --- KNOWN_ISSUES.md | 2 +- README.md | 4 +- cmd/vm/lifecycle.go | 38 ++++++++++------ cmd/vm/netresize.go | 16 ++++--- cmd/vm/run.go | 58 +++++++++++++++---------- hypervisor/backend.go | 2 +- hypervisor/clone.go | 12 ++--- hypervisor/cloudhypervisor/clone.go | 18 +++++--- hypervisor/cloudhypervisor/create.go | 4 +- hypervisor/cloudhypervisor/direct.go | 4 +- hypervisor/cloudhypervisor/netresize.go | 4 +- hypervisor/cloudhypervisor/restore.go | 3 +- hypervisor/cloudhypervisor/start.go | 10 ++--- hypervisor/create.go | 5 ++- hypervisor/firecracker/clone.go | 19 +++++--- hypervisor/firecracker/create.go | 4 +- hypervisor/firecracker/direct.go | 4 +- hypervisor/firecracker/restore.go | 3 +- hypervisor/firecracker/start.go | 12 ++--- hypervisor/hypervisor.go | 6 +-- network/bridge/bridge_linux.go | 5 +++ network/bridge/bridge_other.go | 5 +++ network/cni/create.go | 13 ++++++ network/network.go | 2 + types/vm.go | 49 +++++++++++++++++++++ types/vm_test.go | 55 +++++++++++++++++++++++ 26 files changed, 257 insertions(+), 100 deletions(-) diff --git a/KNOWN_ISSUES.md b/KNOWN_ISSUES.md index 737913d8..0a514177 100644 --- a/KNOWN_ISSUES.md +++ b/KNOWN_ISSUES.md @@ -244,7 +244,7 @@ Control-plane traffic from cocoon-managed hosts (vk-cocoon, `cocoon vm exec`) go ## NIC hot-remove leaves the PCI slot pending on Cloud Hypervisor -`cocoon vm net --nics N` waits for CH's `device_tree` to drop the removed device (polled via `vm.info` until the entry disappears, max 10 s) before tearing down host plumbing. CH only frees the PCI slot — and unregisters the device's ioeventfds — when the guest writes to the ACPI hot-plug controller (B0EJ) in response to the SCI raised by `remove-device`. If the guest never ACKs (driver wedged, paused, NDIS halted), the wait times out, the cocoon record is left intact, and the command returns an error so the user can quiesce the guest and retry. +`cocoon vm net --nics N` waits for CH's `device_tree` to drop the removed device (polled via `vm.info` until the entry disappears, max 30 s) before tearing down host plumbing. CH only frees the PCI slot — and unregisters the device's ioeventfds — when the guest writes to the ACPI hot-plug controller (B0EJ) in response to the SCI raised by `remove-device`. If the guest never ACKs (driver wedged, paused, NDIS halted), the wait times out, the cocoon record is left intact, and the command returns an error so the user can quiesce the guest and retry. If the host plumbing teardown itself fails after a successful eject, cocoon truncates the record anyway and surfaces a warning — the orphan TAP / veth / CNI lease is cleaned up by `cocoon vm rm` or the next gc cycle. The user still has to consider: diff --git a/README.md b/README.md index 2768d261..40eb57ef 100644 --- a/README.md +++ b/README.md @@ -547,9 +547,9 @@ cocoon vm net my-vm --nics 2 cocoon vm net my-vm --nics 1 ``` -Cocoon manages **host-side** plumbing only. CH's `vm.remove-device` marks the slot for ejection but the actual eject only happens when the guest cooperates via ACPI (B0EJ write). The host TAP / veth / CNI lease are torn down immediately after the API call regardless. Quiesce in-guest NIC state (driver unbind, NetworkManager removal, Windows NDIS halt) **before** reducing the count, or the in-guest driver will reference plumbing that no longer exists. +On NIC removal, cocoon waits for the guest to ACK B0EJ (CH polls `device_tree` until the device disappears) before tearing down the host TAP / veth / CNI lease. If the guest never ACKs within the eject timeout, the command fails and leaves the cocoon record + host plumbing intact so the operator can quiesce the guest (driver unbind, NetworkManager removal, Windows NDIS halt) and retry. -A VM started with zero NICs cannot be resized up — CH was launched in the host netns (no `NetworkConfigs` to derive a per-VM netns from), so later plumbing can't reach it. To recover networking on a 0-NIC snapshot, clone with `cocoon vm clone --nics 1 --network ` (or `--bridge `): the clone starts with NICs from the start, putting CH in the right netns from boot. +Resize from zero is supported: under CNI, `--nics 0` still provisions a per-VM netns at boot (CH lives in it from the start), so a later `cocoon vm net --nics N` hot-plugs into the same namespace. Bridge mode keeps CH in the host netns regardless of NIC count, so 0→N adds TAPs onto the configured bridge. ## Windows Support diff --git a/cmd/vm/lifecycle.go b/cmd/vm/lifecycle.go index 9e84aca3..9d794f84 100644 --- a/cmd/vm/lifecycle.go +++ b/cmd/vm/lifecycle.go @@ -373,11 +373,14 @@ func (h Handler) recoverNetwork(ctx context.Context, conf *config.Config, hyper for _, ref := range refs { vm := byID[ref] - if vm == nil || len(vm.NetworkConfigs) == 0 { + if vm == nil || vm.ResolvedNetBackend() == "" { continue } - - netProvider, provErr := providerForVM(conf, cniProvider, bridgeProviders, vm.NetworkConfigs) + // Bridge mode has nothing per-VM to recover when NICs=0 (no TAP, no netns). + if vm.ResolvedNetBackend() == types.BackendBridge && len(vm.NetworkConfigs) == 0 { + continue + } + netProvider, provErr := providerForVM(conf, cniProvider, bridgeProviders, vm) if provErr != nil { logger.Warnf(ctx, "skip recovery for VM %s: %v", vm.ID, provErr) continue @@ -386,31 +389,38 @@ func (h Handler) recoverNetwork(ctx context.Context, conf *config.Config, hyper continue } logger.Warnf(ctx, "network missing for VM %s, recovering", vm.ID) + if _, prepErr := netProvider.Prepare(ctx, vm.ID, &vm.Config); prepErr != nil { + logger.Warnf(ctx, "prepare netns for VM %s: %v (start will fail)", vm.ID, prepErr) + continue + } + if len(vm.NetworkConfigs) == 0 { + continue + } if _, recoverErr := netProvider.Add(ctx, vm.ID, &vm.Config, network.AddRecover(vm.NetworkConfigs)...); recoverErr != nil { logger.Warnf(ctx, "recover network for VM %s: %v (start will fail)", vm.ID, recoverErr) } } } -// providerForVM picks the network provider from persisted NetworkConfig; cniProvider may be nil (lazy-init), bridgeCache must be non-nil. -func providerForVM(conf *config.Config, cniProvider network.Network, bridgeCache map[string]network.Network, configs []*types.NetworkConfig) (network.Network, error) { - if len(configs) == 0 { - return nil, fmt.Errorf("no network configs") +// providerForVM picks the network provider from persisted VM state; +// cniProvider may be nil (lazy-init), bridgeCache must be non-nil. +func providerForVM(conf *config.Config, cniProvider network.Network, bridgeCache map[string]network.Network, vm *types.VM) (network.Network, error) { + if vm == nil { + return nil, fmt.Errorf("no VM record") } - // All NICs on a VM share the same backend. - cfg := configs[0] - if cfg.Backend == types.BackendBridge { - if cfg.BridgeDev == "" { + if vm.ResolvedNetBackend() == types.BackendBridge { + dev := vm.ResolvedNetBridgeDev() + if dev == "" { return nil, fmt.Errorf("bridge backend but no bridge device persisted") } - if cached, ok := bridgeCache[cfg.BridgeDev]; ok { + if cached, ok := bridgeCache[dev]; ok { return cached, nil } - p, err := cmdcore.InitBridgeNetwork(conf, cfg.BridgeDev) + p, err := cmdcore.InitBridgeNetwork(conf, dev) if err != nil { return nil, err } - bridgeCache[cfg.BridgeDev] = p + bridgeCache[dev] = p return p, nil } // "cni" or empty (backward compat). diff --git a/cmd/vm/netresize.go b/cmd/vm/netresize.go index 59677389..09a05fff 100644 --- a/cmd/vm/netresize.go +++ b/cmd/vm/netresize.go @@ -22,7 +22,7 @@ func (h Handler) NetResize(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("vm net: %w", err) } - plumbing, err := plumbingForVM(conf, vm.NetworkConfigs) + plumbing, err := plumbingForVM(conf, vm) if err != nil { return fmt.Errorf("vm net: %w", err) } @@ -43,10 +43,14 @@ func (h Handler) NetResize(cmd *cobra.Command, args []string) error { return nil } -// plumbingForVM picks the provider matching the VM's existing NICs; zero NICs is fatal (use `vm clone --nics N` instead). -func plumbingForVM(conf *config.Config, configs []*types.NetworkConfig) (network.Network, error) { - if len(configs) == 0 { - return nil, fmt.Errorf("zero NICs; resize up not supported (use `vm clone --nics N` instead)") +// plumbingForVM picks the provider from persisted VM state; 0-NIC works because NetBackend persists. +func plumbingForVM(conf *config.Config, vm *types.VM) (network.Network, error) { + backend := vm.ResolvedNetBackend() + if backend == "" { + return nil, fmt.Errorf("no network backend on VM; cannot resize") } - return providerForVM(conf, nil, map[string]network.Network{}, configs) + if backend == types.BackendCNI && vm.ResolvedNetnsPath() == "" { + return nil, fmt.Errorf("CNI VM has no netns; CH is in host netns and resize-up would land in the wrong namespace") + } + return providerForVM(conf, nil, map[string]network.Network{}, vm) } diff --git a/cmd/vm/run.go b/cmd/vm/run.go index a729d294..5f334257 100644 --- a/cmd/vm/run.go +++ b/cmd/vm/run.go @@ -122,14 +122,14 @@ func (h Handler) Clone(cmd *cobra.Command, args []string) error { }) defer stop() - vmCfg, vmID, netProvider, networkConfigs, err := h.prepareClone(ctx, cmd, conf, cfg) + vmCfg, vmID, netProvider, netSetup, err := h.prepareClone(ctx, cmd, conf, cfg) if err != nil { return err } logger.Infof(ctx, "cloning VM from snapshot %s ...", snapRef) - vm, cloneErr := hyper.Clone(ctx, vmID, vmCfg, networkConfigs, &cfg, stream) + vm, cloneErr := hyper.Clone(ctx, vmID, vmCfg, netSetup, &cfg, stream) if cloneErr != nil { rollbackNetwork(ctx, netProvider, vmID) return fmt.Errorf("clone VM: %w", cloneErr) @@ -139,7 +139,7 @@ func (h Handler) Clone(cmd *cobra.Command, args []string) error { return jsonErr } logger.Infof(ctx, "VM cloned: %s (name: %s)", vm.ID, vm.Config.Name) - printPostCloneHints(vm, networkConfigs) + printPostCloneHints(vm, netSetup.NICs) return nil } @@ -285,7 +285,7 @@ func (h Handler) cloneFromDir(ctx context.Context, cmd *cobra.Command, conf *con } func (h Handler) cloneFromSrcDir(ctx context.Context, cmd *cobra.Command, conf *config.Config, dcr hypervisor.Direct, cfg types.SnapshotConfig, srcDir, sourceLabel string, logger *log.Fields) error { - vmCfg, vmID, netProvider, networkConfigs, err := h.prepareClone(ctx, cmd, conf, cfg) + vmCfg, vmID, netProvider, netSetup, err := h.prepareClone(ctx, cmd, conf, cfg) if err != nil { return err } @@ -295,7 +295,7 @@ func (h Handler) cloneFromSrcDir(ctx context.Context, cmd *cobra.Command, conf * logger.Infof(ctx, "cloning VM from %s ...", sourceLabel) } - vm, cloneErr := dcr.DirectClone(ctx, vmID, vmCfg, networkConfigs, &cfg, srcDir) + vm, cloneErr := dcr.DirectClone(ctx, vmID, vmCfg, netSetup, &cfg, srcDir) if cloneErr != nil { rollbackNetwork(ctx, netProvider, vmID) return fmt.Errorf("clone VM: %w", cloneErr) @@ -305,7 +305,7 @@ func (h Handler) cloneFromSrcDir(ctx context.Context, cmd *cobra.Command, conf * return cmdcore.OutputJSON(vm) } logger.Infof(ctx, "VM cloned: %s (name: %s)", vm.ID, vm.Config.Name) - printPostCloneHints(vm, networkConfigs) + printPostCloneHints(vm, netSetup.NICs) return nil } @@ -328,24 +328,24 @@ func snapshotSource(cmd *cobra.Command, args []string, baseArgs int) (string, st return "", args[baseArgs], nil } -func (h Handler) prepareClone(ctx context.Context, cmd *cobra.Command, conf *config.Config, cfg types.SnapshotConfig) (*types.VMConfig, string, network.Network, []*types.NetworkConfig, error) { +func (h Handler) prepareClone(ctx context.Context, cmd *cobra.Command, conf *config.Config, cfg types.SnapshotConfig) (*types.VMConfig, string, network.Network, types.NetSetup, error) { vmCfg, err := cmdcore.CloneVMConfigFromFlags(cmd, cfg) if err != nil { - return nil, "", nil, nil, err + return nil, "", nil, types.NetSetup{}, err } vmID := utils.GenerateID() if vmCfg.Name == "" { vmCfg.Name = "cocoon-clone-" + network.VMIDPrefix(vmID) } if err = vmCfg.Validate(); err != nil { - return nil, "", nil, nil, err + return nil, "", nil, types.NetSetup{}, err } // Auto-pull base image if --pull is set (cross-node clone). if pull, _ := cmd.Flags().GetBool("pull"); pull && vmCfg.Image != "" && vmCfg.ImageType != "" { backends, initErr := cmdcore.InitImageBackends(ctx, conf) if initErr != nil { - return nil, "", nil, nil, fmt.Errorf("init image backends: %w", initErr) + return nil, "", nil, types.NetSetup{}, fmt.Errorf("init image backends: %w", initErr) } cmdcore.EnsureImage(ctx, backends, vmCfg) } @@ -354,16 +354,16 @@ func (h Handler) prepareClone(ctx context.Context, cmd *cobra.Command, conf *con nics := cfg.NICs if cmd.Flags().Changed("nics") { if conf.UseFirecracker { - return nil, "", nil, nil, fmt.Errorf("--nics override on clone is Cloud Hypervisor only (FC network_overrides retargets existing NICs, not resize)") + return nil, "", nil, types.NetSetup{}, fmt.Errorf("--nics override on clone is Cloud Hypervisor only (FC network_overrides retargets existing NICs, not resize)") } nics, _ = cmd.Flags().GetInt("nics") } - netProvider, networkConfigs, err := initNetwork(ctx, conf, vmID, nics, vmCfg, tapQueues(vmCfg.CPU, conf.UseFirecracker), bridgeDev) + netProvider, netSetup, err := initNetwork(ctx, conf, vmID, nics, vmCfg, tapQueues(vmCfg.CPU, conf.UseFirecracker), bridgeDev) if err != nil { - return nil, "", nil, nil, err + return nil, "", nil, types.NetSetup{}, err } - return vmCfg, vmID, netProvider, networkConfigs, nil + return vmCfg, vmID, netProvider, netSetup, nil } func (h Handler) restoreDirect(ctx context.Context, cmd *cobra.Command, snapRef, vmRef string, vmCfg *types.VMConfig, snapBackend snapshot.Snapshot, hyper hypervisor.Hypervisor, logger *log.Fields) (bool, error) { @@ -448,12 +448,12 @@ func (h Handler) createVM(cmd *cobra.Command, image string) (context.Context, *t vmID := utils.GenerateID() nics, _ := cmd.Flags().GetInt("nics") - netProvider, networkConfigs, err := initNetwork(ctx, conf, vmID, nics, vmCfg, tapQueues(vmCfg.CPU, conf.UseFirecracker), bridgeDev) + netProvider, netSetup, err := initNetwork(ctx, conf, vmID, nics, vmCfg, tapQueues(vmCfg.CPU, conf.UseFirecracker), bridgeDev) if err != nil { return nil, nil, nil, err } - info, createErr := hyper.Create(ctx, vmID, vmCfg, storageConfigs, networkConfigs, bootCfg) + info, createErr := hyper.Create(ctx, vmID, vmCfg, storageConfigs, netSetup, bootCfg) if createErr != nil { rollbackNetwork(ctx, netProvider, vmID) return nil, nil, nil, fmt.Errorf("create VM: %w", createErr) @@ -469,10 +469,7 @@ func tapQueues(cpu int, useFC bool) int { return cpu } -func initNetwork(ctx context.Context, conf *config.Config, vmID string, nics int, vmCfg *types.VMConfig, queues int, bridgeDev string) (network.Network, []*types.NetworkConfig, error) { - if nics <= 0 { - return nil, nil, nil - } +func initNetwork(ctx context.Context, conf *config.Config, vmID string, nics int, vmCfg *types.VMConfig, queues int, bridgeDev string) (network.Network, types.NetSetup, error) { var netProvider network.Network var err error if bridgeDev != "" { @@ -481,7 +478,20 @@ func initNetwork(ctx context.Context, conf *config.Config, vmID string, nics int netProvider, err = cmdcore.InitNetwork(conf) } if err != nil { - return nil, nil, fmt.Errorf("init network: %w", err) + return nil, types.NetSetup{}, fmt.Errorf("init network: %w", err) + } + nsPath, err := netProvider.Prepare(ctx, vmID, vmCfg) + if err != nil { + return nil, types.NetSetup{}, fmt.Errorf("prepare network: %w", err) + } + // CNI with no conflist + no NICs degenerates to host-netns mode; record an + // empty backend so a later resize doesn't pick CNI and miss the netns. + if nics <= 0 && netProvider.Type() == types.BackendCNI && nsPath == "" { + return netProvider, types.NetSetup{}, nil + } + setup := types.NetSetup{Backend: netProvider.Type(), NetnsPath: nsPath, BridgeDev: bridgeDev} + if nics <= 0 { + return netProvider, setup, nil } // Override CPU for TAP queue count — FC uses single-queue, CH uses per-vCPU queues. // The network layer derives TAP queues from vmCfg.CPU. @@ -490,9 +500,11 @@ func initNetwork(ctx context.Context, conf *config.Config, vmID string, nics int configs, err := netProvider.Add(ctx, vmID, vmCfg, network.AddRange(0, nics)...) vmCfg.CPU = origCPU if err != nil { - return nil, nil, fmt.Errorf("configure network: %w", err) + rollbackNetwork(ctx, netProvider, vmID) + return nil, types.NetSetup{}, fmt.Errorf("configure network: %w", err) } - return netProvider, configs, nil + setup.NICs = configs + return netProvider, setup, nil } func rollbackNetwork(ctx context.Context, netProvider network.Network, vmID string) { diff --git a/hypervisor/backend.go b/hypervisor/backend.go index 76e16027..917f6df6 100644 --- a/hypervisor/backend.go +++ b/hypervisor/backend.go @@ -101,7 +101,7 @@ type DirectRestoreSpec struct { type CreateSpec struct { VMCfg *types.VMConfig StorageConfigs []*types.StorageConfig - NetworkConfigs []*types.NetworkConfig + Net types.NetSetup BootConfig *types.BootConfig Prepare func(ctx context.Context, vmID string, vmCfg *types.VMConfig, storageConfigs []*types.StorageConfig, networkConfigs []*types.NetworkConfig, boot *types.BootConfig) ([]*types.StorageConfig, error) } diff --git a/hypervisor/clone.go b/hypervisor/clone.go index f3d20927..99884b54 100644 --- a/hypervisor/clone.go +++ b/hypervisor/clone.go @@ -39,9 +39,9 @@ func (b *Backend) CloneSetup(ctx context.Context, vmID string, vmCfg *types.VMCo // snapshot lives on the same host (no tar streaming needed). func (b *Backend) DirectCloneBase( ctx context.Context, vmID string, vmCfg *types.VMConfig, - networkConfigs []*types.NetworkConfig, snapshotConfig *types.SnapshotConfig, srcDir string, + net types.NetSetup, snapshotConfig *types.SnapshotConfig, srcDir string, cloneFiles func(dstDir, srcDir string) error, - afterExtract func(ctx context.Context, vmID string, vmCfg *types.VMConfig, networkConfigs []*types.NetworkConfig, runDir, logDir string, now time.Time) (*types.VM, error), + afterExtract func(ctx context.Context, vmID string, vmCfg *types.VMConfig, net types.NetSetup, runDir, logDir string, now time.Time) (*types.VM, error), ) (_ *types.VM, err error) { runDir, logDir, now, cleanup, err := b.CloneSetup(ctx, vmID, vmCfg, snapshotConfig) if err != nil { @@ -57,15 +57,15 @@ func (b *Backend) DirectCloneBase( return nil, fmt.Errorf("clone snapshot files: %w", err) } - return afterExtract(ctx, vmID, vmCfg, networkConfigs, runDir, logDir, now) + return afterExtract(ctx, vmID, vmCfg, net, runDir, logDir, now) } // CloneFromStream clones from a tar stream into a fresh runDir. Used when // the snapshot arrives over the network (cross-node clone). func (b *Backend) CloneFromStream( ctx context.Context, vmID string, vmCfg *types.VMConfig, - networkConfigs []*types.NetworkConfig, snapshotConfig *types.SnapshotConfig, snapshot io.Reader, - afterExtract func(ctx context.Context, vmID string, vmCfg *types.VMConfig, networkConfigs []*types.NetworkConfig, runDir, logDir string, now time.Time) (*types.VM, error), + net types.NetSetup, snapshotConfig *types.SnapshotConfig, snapshot io.Reader, + afterExtract func(ctx context.Context, vmID string, vmCfg *types.VMConfig, net types.NetSetup, runDir, logDir string, now time.Time) (*types.VM, error), ) (_ *types.VM, err error) { runDir, logDir, now, cleanup, err := b.CloneSetup(ctx, vmID, vmCfg, snapshotConfig) if err != nil { @@ -81,7 +81,7 @@ func (b *Backend) CloneFromStream( return nil, fmt.Errorf("extract snapshot: %w", err) } - return afterExtract(ctx, vmID, vmCfg, networkConfigs, runDir, logDir, now) + return afterExtract(ctx, vmID, vmCfg, net, runDir, logDir, now) } // FinalizeClone updates the cloned VM's record in place after restore-and-resume. diff --git a/hypervisor/cloudhypervisor/clone.go b/hypervisor/cloudhypervisor/clone.go index 51807d49..77d07fba 100644 --- a/hypervisor/cloudhypervisor/clone.go +++ b/hypervisor/cloudhypervisor/clone.go @@ -26,11 +26,12 @@ type cloneResumeOpts struct { snapshotCfg *chVMConfig } -func (ch *CloudHypervisor) Clone(ctx context.Context, vmID string, vmCfg *types.VMConfig, networkConfigs []*types.NetworkConfig, snapshotConfig *types.SnapshotConfig, snapshot io.Reader) (*types.VM, error) { - return ch.CloneFromStream(ctx, vmID, vmCfg, networkConfigs, snapshotConfig, snapshot, ch.cloneAfterExtract) +func (ch *CloudHypervisor) Clone(ctx context.Context, vmID string, vmCfg *types.VMConfig, net types.NetSetup, snapshotConfig *types.SnapshotConfig, snapshot io.Reader) (*types.VM, error) { + return ch.CloneFromStream(ctx, vmID, vmCfg, net, snapshotConfig, snapshot, ch.cloneAfterExtract) } -func (ch *CloudHypervisor) cloneAfterExtract(ctx context.Context, vmID string, vmCfg *types.VMConfig, networkConfigs []*types.NetworkConfig, runDir, logDir string, now time.Time) (*types.VM, error) { +func (ch *CloudHypervisor) cloneAfterExtract(ctx context.Context, vmID string, vmCfg *types.VMConfig, net types.NetSetup, runDir, logDir string, now time.Time) (*types.VM, error) { + networkConfigs := net.NICs logger := log.WithFunc("cloudhypervisor.Clone") chConfigPath := filepath.Join(runDir, "config.json") @@ -105,12 +106,16 @@ func (ch *CloudHypervisor) cloneAfterExtract(ctx context.Context, vmID string, v args := []string{"--api-socket", sockPath} ch.saveCmdline(ctx, &hypervisor.VMRecord{RunDir: runDir}, args) - withNetwork := len(networkConfigs) > 0 pid, err := ch.launchProcess(ctx, &hypervisor.VMRecord{ - VM: types.VM{NetworkConfigs: networkConfigs}, + VM: types.VM{ + NetworkConfigs: networkConfigs, + NetBackend: net.Backend, + NetnsPath: net.NetnsPath, + NetBridgeDev: net.BridgeDev, + }, RunDir: runDir, LogDir: logDir, - }, sockPath, args, withNetwork) + }, sockPath, args) if err != nil { ch.MarkError(ctx, vmID) return nil, fmt.Errorf("launch CH: %w", err) @@ -130,6 +135,7 @@ func (ch *CloudHypervisor) cloneAfterExtract(ctx context.Context, vmID string, v info := &types.VM{ ID: vmID, Hypervisor: typ, State: types.VMStateRunning, Config: *vmCfg, StorageConfigs: storageConfigs, NetworkConfigs: networkConfigs, + NetBackend: net.Backend, NetnsPath: net.NetnsPath, NetBridgeDev: net.BridgeDev, CreatedAt: now, UpdatedAt: now, StartedAt: &now, } if err := ch.FinalizeClone(ctx, vmID, info, bootCfg, nil); err != nil { diff --git a/hypervisor/cloudhypervisor/create.go b/hypervisor/cloudhypervisor/create.go index c6251e48..98b54c2b 100644 --- a/hypervisor/cloudhypervisor/create.go +++ b/hypervisor/cloudhypervisor/create.go @@ -14,11 +14,11 @@ import ( // CowSerial is re-exported for backward compatibility. const CowSerial = hypervisor.CowSerial -func (ch *CloudHypervisor) Create(ctx context.Context, id string, vmCfg *types.VMConfig, storageConfigs []*types.StorageConfig, networkConfigs []*types.NetworkConfig, bootCfg *types.BootConfig) (*types.VM, error) { +func (ch *CloudHypervisor) Create(ctx context.Context, id string, vmCfg *types.VMConfig, storageConfigs []*types.StorageConfig, net types.NetSetup, bootCfg *types.BootConfig) (*types.VM, error) { return ch.CreateSequence(ctx, id, hypervisor.CreateSpec{ VMCfg: vmCfg, StorageConfigs: storageConfigs, - NetworkConfigs: networkConfigs, + Net: net, BootConfig: bootCfg, Prepare: func(ctx context.Context, vmID string, vmCfg *types.VMConfig, sc []*types.StorageConfig, nc []*types.NetworkConfig, boot *types.BootConfig) ([]*types.StorageConfig, error) { if boot != nil && boot.KernelPath != "" { diff --git a/hypervisor/cloudhypervisor/direct.go b/hypervisor/cloudhypervisor/direct.go index 6f922447..85b5c516 100644 --- a/hypervisor/cloudhypervisor/direct.go +++ b/hypervisor/cloudhypervisor/direct.go @@ -12,8 +12,8 @@ import ( // DirectClone clones from a local snapshot dir. Per-type: hardlink memory-range-*, // reflink/copy COW, plain copy metadata; cidata is regenerated. -func (ch *CloudHypervisor) DirectClone(ctx context.Context, vmID string, vmCfg *types.VMConfig, networkConfigs []*types.NetworkConfig, snapshotConfig *types.SnapshotConfig, srcDir string) (*types.VM, error) { - return ch.DirectCloneBase(ctx, vmID, vmCfg, networkConfigs, snapshotConfig, srcDir, cloneSnapshotFiles, ch.cloneAfterExtract) +func (ch *CloudHypervisor) DirectClone(ctx context.Context, vmID string, vmCfg *types.VMConfig, net types.NetSetup, snapshotConfig *types.SnapshotConfig, srcDir string) (*types.VM, error) { + return ch.DirectCloneBase(ctx, vmID, vmCfg, net, snapshotConfig, srcDir, cloneSnapshotFiles, ch.cloneAfterExtract) } func (ch *CloudHypervisor) DirectRestore(ctx context.Context, vmRef string, vmCfg *types.VMConfig, srcDir string) (*types.VM, error) { diff --git a/hypervisor/cloudhypervisor/netresize.go b/hypervisor/cloudhypervisor/netresize.go index 09649d8c..302ab116 100644 --- a/hypervisor/cloudhypervisor/netresize.go +++ b/hypervisor/cloudhypervisor/netresize.go @@ -15,8 +15,8 @@ import ( "github.com/cocoonstack/cocoon/types" ) -// ejectWaitTimeout bounds the wait for guest B0EJ; Linux acks < 1 s, Windows can take a few. -const ejectWaitTimeout = 10 * time.Second +// ejectWaitTimeout bounds the wait for guest B0EJ; Linux acks < 1 s, Windows can take 10–20 s. +const ejectWaitTimeout = 30 * time.Second // NetResize brings the VM's NIC count to spec.Target on a running CH VM. func (ch *CloudHypervisor) NetResize(ctx context.Context, vmRef string, spec netresize.Spec, plumbing netresize.Plumbing) (netresize.Result, error) { diff --git a/hypervisor/cloudhypervisor/restore.go b/hypervisor/cloudhypervisor/restore.go index 00e1349f..de47027a 100644 --- a/hypervisor/cloudhypervisor/restore.go +++ b/hypervisor/cloudhypervisor/restore.go @@ -72,8 +72,7 @@ func (ch *CloudHypervisor) restoreAfterExtract(ctx context.Context, vmID string, args := []string{"--api-socket", sockPath} ch.saveCmdline(ctx, rec, args) - withNetwork := len(rec.NetworkConfigs) > 0 - pid, launchErr := ch.launchProcess(ctx, rec, sockPath, args, withNetwork) + pid, launchErr := ch.launchProcess(ctx, rec, sockPath, args) if launchErr != nil { return nil, fmt.Errorf("launch CH: %w", launchErr) } diff --git a/hypervisor/cloudhypervisor/start.go b/hypervisor/cloudhypervisor/start.go index 231ea245..4de00a3f 100644 --- a/hypervisor/cloudhypervisor/start.go +++ b/hypervisor/cloudhypervisor/start.go @@ -37,15 +37,14 @@ func (ch *CloudHypervisor) startOne(ctx context.Context, id string) error { args := buildCLIArgs(vmCfg, sockPath) ch.saveCmdline(ctx, rec, args) - withNetwork := len(rec.NetworkConfigs) > 0 - if _, err = ch.launchProcess(ctx, rec, sockPath, args, withNetwork); err != nil { + if _, err = ch.launchProcess(ctx, rec, sockPath, args); err != nil { ch.MarkError(ctx, id) return fmt.Errorf("launch VM: %w", err) } return nil } -func (ch *CloudHypervisor) launchProcess(ctx context.Context, rec *hypervisor.VMRecord, socketPath string, args []string, withNetwork bool) (int, error) { +func (ch *CloudHypervisor) launchProcess(ctx context.Context, rec *hypervisor.VMRecord, socketPath string, args []string) (int, error) { processLog := ch.LogFilePath(rec.LogDir) logFile, err := os.Create(processLog) //nolint:gosec if err != nil { @@ -64,10 +63,7 @@ func (ch *CloudHypervisor) launchProcess(ctx context.Context, rec *hypervisor.VM } // CNI mode: enter per-VM netns before fork. Bridge mode: TAP is in host netns. - netnsPath := "" - if withNetwork && rec.NetworkConfigs[0].NetnsPath != "" { - netnsPath = rec.NetworkConfigs[0].NetnsPath - } + netnsPath := rec.ResolvedNetnsPath() pid, err := ch.LaunchVMProcess(ctx, hypervisor.LaunchSpec{ Cmd: cmd, diff --git a/hypervisor/create.go b/hypervisor/create.go index 4a013e26..463c6111 100644 --- a/hypervisor/create.go +++ b/hypervisor/create.go @@ -99,7 +99,7 @@ func (b *Backend) CreateSequence(ctx context.Context, id string, spec CreateSpec bootCopy = &bc } - preparedStorage, err := spec.Prepare(ctx, id, spec.VMCfg, spec.StorageConfigs, spec.NetworkConfigs, bootCopy) + preparedStorage, err := spec.Prepare(ctx, id, spec.VMCfg, spec.StorageConfigs, spec.Net.NICs, bootCopy) if err != nil { return nil, err } @@ -109,7 +109,8 @@ func (b *Backend) CreateSequence(ctx context.Context, id string, spec CreateSpec info := &types.VM{ ID: id, Hypervisor: b.Typ, State: types.VMStateCreated, - Config: *spec.VMCfg, StorageConfigs: preparedStorage, NetworkConfigs: spec.NetworkConfigs, + Config: *spec.VMCfg, StorageConfigs: preparedStorage, NetworkConfigs: spec.Net.NICs, + NetBackend: spec.Net.Backend, NetnsPath: spec.Net.NetnsPath, NetBridgeDev: spec.Net.BridgeDev, CreatedAt: now, UpdatedAt: now, } if err = b.FinalizeCreate(ctx, id, info, bootCopy, blobIDs); err != nil { diff --git a/hypervisor/firecracker/clone.go b/hypervisor/firecracker/clone.go index cc7c3edf..79865b78 100644 --- a/hypervisor/firecracker/clone.go +++ b/hypervisor/firecracker/clone.go @@ -23,11 +23,12 @@ type driveRedirect struct { createdDir bool } -func (fc *Firecracker) Clone(ctx context.Context, vmID string, vmCfg *types.VMConfig, networkConfigs []*types.NetworkConfig, snapshotConfig *types.SnapshotConfig, snapshot io.Reader) (*types.VM, error) { - return fc.CloneFromStream(ctx, vmID, vmCfg, networkConfigs, snapshotConfig, snapshot, fc.cloneAfterExtract) +func (fc *Firecracker) Clone(ctx context.Context, vmID string, vmCfg *types.VMConfig, net types.NetSetup, snapshotConfig *types.SnapshotConfig, snapshot io.Reader) (*types.VM, error) { + return fc.CloneFromStream(ctx, vmID, vmCfg, net, snapshotConfig, snapshot, fc.cloneAfterExtract) } -func (fc *Firecracker) cloneAfterExtract(ctx context.Context, vmID string, vmCfg *types.VMConfig, networkConfigs []*types.NetworkConfig, runDir, logDir string, now time.Time) (*types.VM, error) { +func (fc *Firecracker) cloneAfterExtract(ctx context.Context, vmID string, vmCfg *types.VMConfig, net types.NetSetup, runDir, logDir string, now time.Time) (*types.VM, error) { + networkConfigs := net.NICs logger := log.WithFunc("firecracker.Clone") meta, err := hypervisor.LoadAndValidateMeta(runDir, fc.conf.RootDir, fc.conf.Config.RunDir) @@ -73,7 +74,6 @@ func (fc *Firecracker) cloneAfterExtract(ctx context.Context, vmID string, vmCfg // changed, so symlink-redirect the source path until upstream supports // drive overrides at load time. sockPath := hypervisor.SocketPath(runDir) - withNetwork := len(networkConfigs) > 0 var pid int if cloneErr := withSourceWritableDisksLocked(meta.StorageConfigs, func() error { redirects, redirectErr := createDriveRedirects(meta.StorageConfigs, storageConfigs) @@ -84,10 +84,16 @@ func (fc *Firecracker) cloneAfterExtract(ctx context.Context, vmID string, vmCfg var launchErr error pid, launchErr = fc.launchProcess(ctx, &hypervisor.VMRecord{ - VM: types.VM{ID: vmID, NetworkConfigs: networkConfigs}, + VM: types.VM{ + ID: vmID, + NetworkConfigs: networkConfigs, + NetBackend: net.Backend, + NetnsPath: net.NetnsPath, + NetBridgeDev: net.BridgeDev, + }, RunDir: runDir, LogDir: logDir, - }, sockPath, withNetwork) + }, sockPath) if launchErr != nil { return fmt.Errorf("launch FC: %w", launchErr) } @@ -101,6 +107,7 @@ func (fc *Firecracker) cloneAfterExtract(ctx context.Context, vmID string, vmCfg info := &types.VM{ ID: vmID, Hypervisor: typ, State: types.VMStateRunning, Config: *vmCfg, StorageConfigs: storageConfigs, NetworkConfigs: networkConfigs, + NetBackend: net.Backend, NetnsPath: net.NetnsPath, NetBridgeDev: net.BridgeDev, CreatedAt: now, UpdatedAt: now, StartedAt: &now, } if err := fc.FinalizeClone(ctx, vmID, info, bootCfg, blobIDs); err != nil { diff --git a/hypervisor/firecracker/create.go b/hypervisor/firecracker/create.go index 39c816cc..38cbc9c8 100644 --- a/hypervisor/firecracker/create.go +++ b/hypervisor/firecracker/create.go @@ -17,11 +17,11 @@ import ( "github.com/cocoonstack/cocoon/utils" ) -func (fc *Firecracker) Create(ctx context.Context, id string, vmCfg *types.VMConfig, storageConfigs []*types.StorageConfig, networkConfigs []*types.NetworkConfig, bootCfg *types.BootConfig) (*types.VM, error) { +func (fc *Firecracker) Create(ctx context.Context, id string, vmCfg *types.VMConfig, storageConfigs []*types.StorageConfig, net types.NetSetup, bootCfg *types.BootConfig) (*types.VM, error) { return fc.CreateSequence(ctx, id, hypervisor.CreateSpec{ VMCfg: vmCfg, StorageConfigs: storageConfigs, - NetworkConfigs: networkConfigs, + Net: net, BootConfig: bootCfg, Prepare: fc.prepareOCI, }) diff --git a/hypervisor/firecracker/direct.go b/hypervisor/firecracker/direct.go index a718abe7..c5e47ac5 100644 --- a/hypervisor/firecracker/direct.go +++ b/hypervisor/firecracker/direct.go @@ -10,8 +10,8 @@ import ( // DirectClone clones from a local snapshot dir. Per-type: hardlink mem, // reflink/copy COW, plain copy metadata. -func (fc *Firecracker) DirectClone(ctx context.Context, vmID string, vmCfg *types.VMConfig, networkConfigs []*types.NetworkConfig, snapshotConfig *types.SnapshotConfig, srcDir string) (*types.VM, error) { - return fc.DirectCloneBase(ctx, vmID, vmCfg, networkConfigs, snapshotConfig, srcDir, cloneSnapshotFiles, fc.cloneAfterExtract) +func (fc *Firecracker) DirectClone(ctx context.Context, vmID string, vmCfg *types.VMConfig, net types.NetSetup, snapshotConfig *types.SnapshotConfig, srcDir string) (*types.VM, error) { + return fc.DirectCloneBase(ctx, vmID, vmCfg, net, snapshotConfig, srcDir, cloneSnapshotFiles, fc.cloneAfterExtract) } func (fc *Firecracker) DirectRestore(ctx context.Context, vmRef string, vmCfg *types.VMConfig, srcDir string) (*types.VM, error) { diff --git a/hypervisor/firecracker/restore.go b/hypervisor/firecracker/restore.go index e6e272d2..7c197c01 100644 --- a/hypervisor/firecracker/restore.go +++ b/hypervisor/firecracker/restore.go @@ -62,8 +62,7 @@ func (fc *Firecracker) restoreAfterExtract(ctx context.Context, vmID string, vmC sockPath := hypervisor.SocketPath(rec.RunDir) - withNetwork := len(rec.NetworkConfigs) > 0 - pid, launchErr := fc.launchProcess(ctx, rec, sockPath, withNetwork) + pid, launchErr := fc.launchProcess(ctx, rec, sockPath) if launchErr != nil { return nil, fmt.Errorf("launch FC: %w", launchErr) } diff --git a/hypervisor/firecracker/start.go b/hypervisor/firecracker/start.go index 3b045af2..0b086cfa 100644 --- a/hypervisor/firecracker/start.go +++ b/hypervisor/firecracker/start.go @@ -38,8 +38,7 @@ func (fc *Firecracker) startOne(ctx context.Context, id string) error { sockPath := hypervisor.SocketPath(rec.RunDir) - withNetwork := len(rec.NetworkConfigs) > 0 - pid, err := fc.launchProcess(ctx, rec, sockPath, withNetwork) + pid, err := fc.launchProcess(ctx, rec, sockPath) if err != nil { fc.MarkError(ctx, id) return fmt.Errorf("launch VM: %w", err) @@ -138,7 +137,7 @@ func (fc *Firecracker) configureVM(ctx context.Context, hc *http.Client, rec *hy } // launchProcess starts firecracker, sets up PTY+console relay, waits for socket. -func (fc *Firecracker) launchProcess(ctx context.Context, rec *hypervisor.VMRecord, sockPath string, withNetwork bool) (int, error) { +func (fc *Firecracker) launchProcess(ctx context.Context, rec *hypervisor.VMRecord, sockPath string) (int, error) { fcLog := fc.LogFilePath(rec.LogDir) // FC opens log O_WRONLY|O_APPEND without O_CREATE — touch first. if f, createErr := os.Create(fcLog); createErr == nil { //nolint:gosec @@ -163,16 +162,11 @@ func (fc *Firecracker) launchProcess(ctx context.Context, rec *hypervisor.VMReco fcCmd.Stdin = slave fcCmd.Stdout = slave - netnsPath := "" - if withNetwork && rec.NetworkConfigs[0].NetnsPath != "" { - netnsPath = rec.NetworkConfigs[0].NetnsPath - } - pid, err := fc.LaunchVMProcess(ctx, hypervisor.LaunchSpec{ Cmd: fcCmd, PIDPath: fc.PIDFilePath(rec.RunDir), SockPath: sockPath, - NetnsPath: netnsPath, + NetnsPath: rec.ResolvedNetnsPath(), OnFail: func() { _ = master.Close() }, }) if err != nil { diff --git a/hypervisor/hypervisor.go b/hypervisor/hypervisor.go index 256e6c90..3c311b7f 100644 --- a/hypervisor/hypervisor.go +++ b/hypervisor/hypervisor.go @@ -19,7 +19,7 @@ var ( type Hypervisor interface { Type() string - Create(ctx context.Context, vmID string, vmCfg *types.VMConfig, storage []*types.StorageConfig, network []*types.NetworkConfig, boot *types.BootConfig) (*types.VM, error) + Create(ctx context.Context, vmID string, vmCfg *types.VMConfig, storage []*types.StorageConfig, net types.NetSetup, boot *types.BootConfig) (*types.VM, error) Start(ctx context.Context, refs []string) ([]string, error) Stop(ctx context.Context, refs []string) ([]string, error) Inspect(ctx context.Context, ref string) (*types.VM, error) @@ -28,7 +28,7 @@ type Hypervisor interface { Console(ctx context.Context, ref string) (io.ReadWriteCloser, error) LogPath(ctx context.Context, ref string) (string, error) Snapshot(ctx context.Context, ref string) (*types.SnapshotConfig, io.ReadCloser, error) - Clone(ctx context.Context, vmID string, vmCfg *types.VMConfig, networkConfigs []*types.NetworkConfig, snapshotConfig *types.SnapshotConfig, snapshot io.Reader) (*types.VM, error) + Clone(ctx context.Context, vmID string, vmCfg *types.VMConfig, net types.NetSetup, snapshotConfig *types.SnapshotConfig, snapshot io.Reader) (*types.VM, error) Restore(ctx context.Context, vmRef string, vmCfg *types.VMConfig, snapshot io.Reader) (*types.VM, error) RegisterGC(*gc.Orchestrator) @@ -43,6 +43,6 @@ type Watchable interface { // Direct is an optional interface for hypervisors that support // clone/restore from a local snapshot directory. type Direct interface { - DirectClone(ctx context.Context, vmID string, vmCfg *types.VMConfig, networkConfigs []*types.NetworkConfig, snapshotConfig *types.SnapshotConfig, srcDir string) (*types.VM, error) + DirectClone(ctx context.Context, vmID string, vmCfg *types.VMConfig, net types.NetSetup, snapshotConfig *types.SnapshotConfig, srcDir string) (*types.VM, error) DirectRestore(ctx context.Context, vmRef string, vmCfg *types.VMConfig, srcDir string) (*types.VM, error) } diff --git a/network/bridge/bridge_linux.go b/network/bridge/bridge_linux.go index 6cab195d..38ec83dc 100644 --- a/network/bridge/bridge_linux.go +++ b/network/bridge/bridge_linux.go @@ -67,6 +67,11 @@ func (b *Bridge) Verify(_ context.Context, vmID string) error { return nil } +// Prepare is a no-op: bridge mode keeps CH in the host netns. +func (b *Bridge) Prepare(_ context.Context, _ string, _ *types.VMConfig) (string, error) { + return "", nil +} + // Add allocates TAP devices on the bridge for the given specs. func (b *Bridge) Add(ctx context.Context, vmID string, vmCfg *types.VMConfig, specs ...network.AddSpec) (configs []*types.NetworkConfig, retErr error) { if len(specs) == 0 { diff --git a/network/bridge/bridge_other.go b/network/bridge/bridge_other.go index 031cb2e6..5bdfc38e 100644 --- a/network/bridge/bridge_other.go +++ b/network/bridge/bridge_other.go @@ -29,6 +29,11 @@ func (b *Bridge) Type() string { return "bridge" } // Verify is not supported. func (b *Bridge) Verify(_ context.Context, _ string) error { return errUnsupported } +// Prepare is not supported. +func (b *Bridge) Prepare(_ context.Context, _ string, _ *types.VMConfig) (string, error) { + return "", errUnsupported +} + // Add is not supported. func (b *Bridge) Add(_ context.Context, _ string, _ *types.VMConfig, _ ...network.AddSpec) ([]*types.NetworkConfig, error) { return nil, errUnsupported diff --git a/network/cni/create.go b/network/cni/create.go index 08056c69..60fa99f4 100644 --- a/network/cni/create.go +++ b/network/cni/create.go @@ -17,6 +17,19 @@ import ( "github.com/cocoonstack/cocoon/utils" ) +// Prepare creates the per-VM netns idempotently; returns "" when no CNI conflist is configured. +func (c *CNI) Prepare(_ context.Context, vmID string, _ *types.VMConfig) (string, error) { + if c.cniConf == nil { + return "", nil + } + nsName := netnsName(vmID) + nsPath := netnsPath(vmID) + if _, err := ensureNetns(nsName, nsPath); err != nil { + return "", fmt.Errorf("ensure netns %s: %w", nsName, err) + } + return nsPath, nil +} + // Add creates the netns (if absent) and allocates each NIC's CNI plumbing. func (c *CNI) Add(ctx context.Context, vmID string, vmCfg *types.VMConfig, specs ...network.AddSpec) (configs []*types.NetworkConfig, retErr error) { if c.cniConf == nil { diff --git a/network/network.go b/network/network.go index ef4b9d24..5c7c7d70 100644 --- a/network/network.go +++ b/network/network.go @@ -23,6 +23,8 @@ type AddSpec struct { type Network interface { Type() string Verify(ctx context.Context, vmID string) error + // Prepare provisions per-VM state regardless of NIC count; returns the netns path or "". + Prepare(ctx context.Context, vmID string, vmCfg *types.VMConfig) (string, error) Add(ctx context.Context, vmID string, vmCfg *types.VMConfig, specs ...AddSpec) ([]*types.NetworkConfig, error) Remove(ctx context.Context, vmID string, indices ...int) error Delete(context.Context, []string) ([]string, error) diff --git a/types/vm.go b/types/vm.go index f36ccf5c..c16b10ec 100644 --- a/types/vm.go +++ b/types/vm.go @@ -36,6 +36,14 @@ type VMConfig struct { DataDisks []DataDiskSpec `json:"-"` // populated from --data-disk; consumed by Create } +// NetSetup is what initNetwork hands the hypervisor: backend identity + netns + NICs. +type NetSetup struct { + Backend string + NetnsPath string + BridgeDev string + NICs []*NetworkConfig +} + // VM is the runtime record for a VM, persisted by the hypervisor backend. type VM struct { ID string `json:"id"` @@ -52,6 +60,11 @@ type VM struct { NetworkConfigs []*NetworkConfig `json:"network_configs,omitempty"` StorageConfigs []*StorageConfig `json:"storage_configs,omitempty"` + // Host networking at VM level so 0-NIC VMs still carry backend + netns. + NetBackend string `json:"net_backend,omitempty"` + NetnsPath string `json:"netns_path,omitempty"` + NetBridgeDev string `json:"net_bridge_dev,omitempty"` + // FirstBooted is true after the VM has been started at least once. // Used to skip cidata attachment on subsequent starts (cloudimg only). FirstBooted bool `json:"first_booted"` @@ -98,3 +111,39 @@ func (cfg *VMConfig) Validate() error { } return nil } + +// ResolvedNetnsPath returns the netns where CH runs (NIC[0] fallback for old records). +func (v *VM) ResolvedNetnsPath() string { + if v.NetnsPath != "" { + return v.NetnsPath + } + if len(v.NetworkConfigs) > 0 { + return v.NetworkConfigs[0].NetnsPath + } + return "" +} + +// ResolvedNetBackend returns the host network backend (NIC[0] fallback for old records). +func (v *VM) ResolvedNetBackend() string { + if v.NetBackend != "" { + return v.NetBackend + } + if len(v.NetworkConfigs) > 0 { + if b := v.NetworkConfigs[0].Backend; b != "" { + return b + } + return BackendCNI + } + return "" +} + +// ResolvedNetBridgeDev returns the bridge device (NIC[0] fallback for old records). +func (v *VM) ResolvedNetBridgeDev() string { + if v.NetBridgeDev != "" { + return v.NetBridgeDev + } + if len(v.NetworkConfigs) > 0 { + return v.NetworkConfigs[0].BridgeDev + } + return "" +} diff --git a/types/vm_test.go b/types/vm_test.go index 1f5efc31..54f30f18 100644 --- a/types/vm_test.go +++ b/types/vm_test.go @@ -169,3 +169,58 @@ func TestValidate(t *testing.T) { }) } } + +func TestVMResolvedNetFields(t *testing.T) { + tests := []struct { + name string + vm VM + wantNetnsPath string + wantNetBackend string + wantNetBridgeDev string + }{ + { + name: "VM-level fields populated", + vm: VM{NetBackend: "cni", NetnsPath: "/var/run/netns/cocoon-x", NetBridgeDev: ""}, + wantNetnsPath: "/var/run/netns/cocoon-x", + wantNetBackend: "cni", + }, + { + name: "fallback to NIC[0] when VM-level empty", + vm: VM{NetworkConfigs: []*NetworkConfig{ + {Backend: BackendBridge, NetnsPath: "", BridgeDev: "br0"}, + }}, + wantNetBackend: BackendBridge, + wantNetBridgeDev: "br0", + }, + { + name: "pre-bridge record (empty Backend) maps to CNI", + vm: VM{NetworkConfigs: []*NetworkConfig{ + {NetnsPath: "/var/run/netns/cocoon-y"}, + }}, + wantNetnsPath: "/var/run/netns/cocoon-y", + wantNetBackend: BackendCNI, + }, + { + name: "VM-level wins over NIC[0]", + vm: VM{NetBackend: "cni", NetworkConfigs: []*NetworkConfig{{Backend: BackendBridge}}}, + wantNetBackend: "cni", + }, + { + name: "empty everywhere", + vm: VM{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.vm.ResolvedNetnsPath(); got != tt.wantNetnsPath { + t.Errorf("ResolvedNetnsPath = %q, want %q", got, tt.wantNetnsPath) + } + if got := tt.vm.ResolvedNetBackend(); got != tt.wantNetBackend { + t.Errorf("ResolvedNetBackend = %q, want %q", got, tt.wantNetBackend) + } + if got := tt.vm.ResolvedNetBridgeDev(); got != tt.wantNetBridgeDev { + t.Errorf("ResolvedNetBridgeDev = %q, want %q", got, tt.wantNetBridgeDev) + } + }) + } +} From a07a83083c299c796e12ce6ded04b201a8342edf Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 13 May 2026 16:06:28 +0800 Subject: [PATCH 2/5] refactor(net): IsBridge/IsCNI helpers, nil-safe Resolved*, launchProcess takes netns directly Senior + /simplify pass on PR #44: - VM.{IsBridge,IsCNI} helpers; Resolved*/ApplyNetSetup are nil-safe - launchProcess(netns) directly drops the partial-VM ferry in CH/FC clone - printPostCloneHints reads vm.NetworkConfigs (no second arg) - initNetwork rolls back on Prepare failure; caches netProvider.Type() - recoverNetwork splits nil/empty-backend checks - Tighten godoc and error strings --- cmd/vm/lifecycle.go | 15 ++++++----- cmd/vm/netresize.go | 7 +++--- cmd/vm/run.go | 26 +++++++++---------- hypervisor/cloudhypervisor/clone.go | 12 +++------ hypervisor/cloudhypervisor/restore.go | 2 +- hypervisor/cloudhypervisor/start.go | 7 ++---- hypervisor/create.go | 4 +-- hypervisor/firecracker/clone.go | 14 +++-------- hypervisor/firecracker/restore.go | 2 +- hypervisor/firecracker/start.go | 6 ++--- network/bridge/bridge_linux.go | 2 +- network/cni/create.go | 2 +- network/network.go | 2 +- types/vm.go | 36 +++++++++++++++++++++++---- 14 files changed, 75 insertions(+), 62 deletions(-) diff --git a/cmd/vm/lifecycle.go b/cmd/vm/lifecycle.go index 9d794f84..8514f272 100644 --- a/cmd/vm/lifecycle.go +++ b/cmd/vm/lifecycle.go @@ -373,11 +373,15 @@ func (h Handler) recoverNetwork(ctx context.Context, conf *config.Config, hyper for _, ref := range refs { vm := byID[ref] - if vm == nil || vm.ResolvedNetBackend() == "" { + if vm == nil { continue } - // Bridge mode has nothing per-VM to recover when NICs=0 (no TAP, no netns). - if vm.ResolvedNetBackend() == types.BackendBridge && len(vm.NetworkConfigs) == 0 { + backend := vm.ResolvedNetBackend() + if backend == "" { + continue + } + // Bridge 0-NIC: no TAP, no netns — nothing to recover. + if backend == types.BackendBridge && len(vm.NetworkConfigs) == 0 { continue } netProvider, provErr := providerForVM(conf, cniProvider, bridgeProviders, vm) @@ -402,13 +406,12 @@ func (h Handler) recoverNetwork(ctx context.Context, conf *config.Config, hyper } } -// providerForVM picks the network provider from persisted VM state; -// cniProvider may be nil (lazy-init), bridgeCache must be non-nil. +// providerForVM picks the provider from VM state. cniProvider may be nil; bridgeCache must be non-nil. func providerForVM(conf *config.Config, cniProvider network.Network, bridgeCache map[string]network.Network, vm *types.VM) (network.Network, error) { if vm == nil { return nil, fmt.Errorf("no VM record") } - if vm.ResolvedNetBackend() == types.BackendBridge { + if vm.IsBridge() { dev := vm.ResolvedNetBridgeDev() if dev == "" { return nil, fmt.Errorf("bridge backend but no bridge device persisted") diff --git a/cmd/vm/netresize.go b/cmd/vm/netresize.go index 09a05fff..219f67fc 100644 --- a/cmd/vm/netresize.go +++ b/cmd/vm/netresize.go @@ -45,12 +45,11 @@ func (h Handler) NetResize(cmd *cobra.Command, args []string) error { // plumbingForVM picks the provider from persisted VM state; 0-NIC works because NetBackend persists. func plumbingForVM(conf *config.Config, vm *types.VM) (network.Network, error) { - backend := vm.ResolvedNetBackend() - if backend == "" { + if vm.ResolvedNetBackend() == "" { return nil, fmt.Errorf("no network backend on VM; cannot resize") } - if backend == types.BackendCNI && vm.ResolvedNetnsPath() == "" { - return nil, fmt.Errorf("CNI VM has no netns; CH is in host netns and resize-up would land in the wrong namespace") + if vm.IsCNI() && vm.ResolvedNetnsPath() == "" { + return nil, fmt.Errorf("CNI backend but no netns; resize would target host netns") } return providerForVM(conf, nil, map[string]network.Network{}, vm) } diff --git a/cmd/vm/run.go b/cmd/vm/run.go index 5f334257..e3d2ed39 100644 --- a/cmd/vm/run.go +++ b/cmd/vm/run.go @@ -139,7 +139,7 @@ func (h Handler) Clone(cmd *cobra.Command, args []string) error { return jsonErr } logger.Infof(ctx, "VM cloned: %s (name: %s)", vm.ID, vm.Config.Name) - printPostCloneHints(vm, netSetup.NICs) + printPostCloneHints(vm) return nil } @@ -305,7 +305,7 @@ func (h Handler) cloneFromSrcDir(ctx context.Context, cmd *cobra.Command, conf * return cmdcore.OutputJSON(vm) } logger.Infof(ctx, "VM cloned: %s (name: %s)", vm.ID, vm.Config.Name) - printPostCloneHints(vm, netSetup.NICs) + printPostCloneHints(vm) return nil } @@ -482,19 +482,19 @@ func initNetwork(ctx context.Context, conf *config.Config, vmID string, nics int } nsPath, err := netProvider.Prepare(ctx, vmID, vmCfg) if err != nil { + rollbackNetwork(ctx, netProvider, vmID) return nil, types.NetSetup{}, fmt.Errorf("prepare network: %w", err) } - // CNI with no conflist + no NICs degenerates to host-netns mode; record an - // empty backend so a later resize doesn't pick CNI and miss the netns. - if nics <= 0 && netProvider.Type() == types.BackendCNI && nsPath == "" { + backend := netProvider.Type() + // CNI no-conflist + 0 NICs runs in host netns; empty backend so resize won't mispick CNI. + if nics <= 0 && backend == types.BackendCNI && nsPath == "" { return netProvider, types.NetSetup{}, nil } - setup := types.NetSetup{Backend: netProvider.Type(), NetnsPath: nsPath, BridgeDev: bridgeDev} + setup := types.NetSetup{Backend: backend, NetnsPath: nsPath, BridgeDev: bridgeDev} if nics <= 0 { return netProvider, setup, nil } - // Override CPU for TAP queue count — FC uses single-queue, CH uses per-vCPU queues. - // The network layer derives TAP queues from vmCfg.CPU. + // Override CPU for TAP queue count (FC=1, CH=per-vCPU); network reads vmCfg.CPU. origCPU := vmCfg.CPU vmCfg.CPU = queues configs, err := netProvider.Add(ctx, vmID, vmCfg, network.AddRange(0, nics)...) @@ -516,7 +516,7 @@ func rollbackNetwork(ctx context.Context, netProvider network.Network, vmID stri } } -func printPostCloneHints(vm *types.VM, networkConfigs []*types.NetworkConfig) { +func printPostCloneHints(vm *types.VM) { if vm.Config.Windows { fmt.Println() fmt.Println("Windows clone: NICs hot-swapped with new MAC addresses.") @@ -542,7 +542,7 @@ func printPostCloneHints(vm *types.VM, networkConfigs []*types.NetworkConfig) { // FC clone: guest MAC is baked in vmstate (source VM's MAC). // Must change guest MAC before networkd config takes effect. if vm.Hypervisor == string(config.HypervisorFirecracker) { - printFCMACHints(networkConfigs) + printFCMACHints(vm.NetworkConfigs) } fmt.Println() @@ -552,7 +552,7 @@ func printPostCloneHints(vm *types.VM, networkConfigs []*types.NetworkConfig) { if isCloudimg { printCloudimgNetworkHints() } else { - printOCINetworkHints(vm, networkConfigs) + printOCINetworkHints(vm) } fmt.Println() } @@ -575,14 +575,14 @@ func printCloudimgNetworkHints() { fmt.Println(" cloud-init modules --mode=config && systemctl restart systemd-networkd") } -func printOCINetworkHints(vm *types.VM, networkConfigs []*types.NetworkConfig) { +func printOCINetworkHints(vm *types.VM) { fmt.Println() fmt.Printf(" # Set hostname\n") fmt.Printf(" hostnamectl set-hostname %s\n", vm.Config.Name) var staticNICs []nicHint var dhcpMACs []string - for _, nc := range networkConfigs { + for _, nc := range vm.NetworkConfigs { if nc == nil || nc.MAC == "" { continue } diff --git a/hypervisor/cloudhypervisor/clone.go b/hypervisor/cloudhypervisor/clone.go index 77d07fba..2e010253 100644 --- a/hypervisor/cloudhypervisor/clone.go +++ b/hypervisor/cloudhypervisor/clone.go @@ -107,15 +107,9 @@ func (ch *CloudHypervisor) cloneAfterExtract(ctx context.Context, vmID string, v ch.saveCmdline(ctx, &hypervisor.VMRecord{RunDir: runDir}, args) pid, err := ch.launchProcess(ctx, &hypervisor.VMRecord{ - VM: types.VM{ - NetworkConfigs: networkConfigs, - NetBackend: net.Backend, - NetnsPath: net.NetnsPath, - NetBridgeDev: net.BridgeDev, - }, RunDir: runDir, LogDir: logDir, - }, sockPath, args) + }, sockPath, args, net.NetnsPath) if err != nil { ch.MarkError(ctx, vmID) return nil, fmt.Errorf("launch CH: %w", err) @@ -134,10 +128,10 @@ func (ch *CloudHypervisor) cloneAfterExtract(ctx context.Context, vmID string, v info := &types.VM{ ID: vmID, Hypervisor: typ, State: types.VMStateRunning, - Config: *vmCfg, StorageConfigs: storageConfigs, NetworkConfigs: networkConfigs, - NetBackend: net.Backend, NetnsPath: net.NetnsPath, NetBridgeDev: net.BridgeDev, + Config: *vmCfg, StorageConfigs: storageConfigs, CreatedAt: now, UpdatedAt: now, StartedAt: &now, } + info.ApplyNetSetup(net) if err := ch.FinalizeClone(ctx, vmID, info, bootCfg, nil); err != nil { ch.AbortLaunch(ctx, pid, sockPath, runDir, runtimeFiles) return nil, fmt.Errorf("finalize VM record: %w", err) diff --git a/hypervisor/cloudhypervisor/restore.go b/hypervisor/cloudhypervisor/restore.go index de47027a..0d164cb3 100644 --- a/hypervisor/cloudhypervisor/restore.go +++ b/hypervisor/cloudhypervisor/restore.go @@ -72,7 +72,7 @@ func (ch *CloudHypervisor) restoreAfterExtract(ctx context.Context, vmID string, args := []string{"--api-socket", sockPath} ch.saveCmdline(ctx, rec, args) - pid, launchErr := ch.launchProcess(ctx, rec, sockPath, args) + pid, launchErr := ch.launchProcess(ctx, rec, sockPath, args, rec.ResolvedNetnsPath()) if launchErr != nil { return nil, fmt.Errorf("launch CH: %w", launchErr) } diff --git a/hypervisor/cloudhypervisor/start.go b/hypervisor/cloudhypervisor/start.go index 4de00a3f..b9d025b8 100644 --- a/hypervisor/cloudhypervisor/start.go +++ b/hypervisor/cloudhypervisor/start.go @@ -37,14 +37,14 @@ func (ch *CloudHypervisor) startOne(ctx context.Context, id string) error { args := buildCLIArgs(vmCfg, sockPath) ch.saveCmdline(ctx, rec, args) - if _, err = ch.launchProcess(ctx, rec, sockPath, args); err != nil { + if _, err = ch.launchProcess(ctx, rec, sockPath, args, rec.ResolvedNetnsPath()); err != nil { ch.MarkError(ctx, id) return fmt.Errorf("launch VM: %w", err) } return nil } -func (ch *CloudHypervisor) launchProcess(ctx context.Context, rec *hypervisor.VMRecord, socketPath string, args []string) (int, error) { +func (ch *CloudHypervisor) launchProcess(ctx context.Context, rec *hypervisor.VMRecord, socketPath string, args []string, netnsPath string) (int, error) { processLog := ch.LogFilePath(rec.LogDir) logFile, err := os.Create(processLog) //nolint:gosec if err != nil { @@ -62,9 +62,6 @@ func (ch *CloudHypervisor) launchProcess(ctx context.Context, rec *hypervisor.VM cmd.Stderr = logFile } - // CNI mode: enter per-VM netns before fork. Bridge mode: TAP is in host netns. - netnsPath := rec.ResolvedNetnsPath() - pid, err := ch.LaunchVMProcess(ctx, hypervisor.LaunchSpec{ Cmd: cmd, PIDPath: ch.PIDFilePath(rec.RunDir), diff --git a/hypervisor/create.go b/hypervisor/create.go index 463c6111..fac479d3 100644 --- a/hypervisor/create.go +++ b/hypervisor/create.go @@ -109,10 +109,10 @@ func (b *Backend) CreateSequence(ctx context.Context, id string, spec CreateSpec info := &types.VM{ ID: id, Hypervisor: b.Typ, State: types.VMStateCreated, - Config: *spec.VMCfg, StorageConfigs: preparedStorage, NetworkConfigs: spec.Net.NICs, - NetBackend: spec.Net.Backend, NetnsPath: spec.Net.NetnsPath, NetBridgeDev: spec.Net.BridgeDev, + Config: *spec.VMCfg, StorageConfigs: preparedStorage, CreatedAt: now, UpdatedAt: now, } + info.ApplyNetSetup(spec.Net) if err = b.FinalizeCreate(ctx, id, info, bootCopy, blobIDs); err != nil { return nil, fmt.Errorf("finalize VM record: %w", err) } diff --git a/hypervisor/firecracker/clone.go b/hypervisor/firecracker/clone.go index 79865b78..8cf4cb1e 100644 --- a/hypervisor/firecracker/clone.go +++ b/hypervisor/firecracker/clone.go @@ -84,16 +84,10 @@ func (fc *Firecracker) cloneAfterExtract(ctx context.Context, vmID string, vmCfg var launchErr error pid, launchErr = fc.launchProcess(ctx, &hypervisor.VMRecord{ - VM: types.VM{ - ID: vmID, - NetworkConfigs: networkConfigs, - NetBackend: net.Backend, - NetnsPath: net.NetnsPath, - NetBridgeDev: net.BridgeDev, - }, + VM: types.VM{ID: vmID}, RunDir: runDir, LogDir: logDir, - }, sockPath) + }, sockPath, net.NetnsPath) if launchErr != nil { return fmt.Errorf("launch FC: %w", launchErr) } @@ -106,10 +100,10 @@ func (fc *Firecracker) cloneAfterExtract(ctx context.Context, vmID string, vmCfg info := &types.VM{ ID: vmID, Hypervisor: typ, State: types.VMStateRunning, - Config: *vmCfg, StorageConfigs: storageConfigs, NetworkConfigs: networkConfigs, - NetBackend: net.Backend, NetnsPath: net.NetnsPath, NetBridgeDev: net.BridgeDev, + Config: *vmCfg, StorageConfigs: storageConfigs, CreatedAt: now, UpdatedAt: now, StartedAt: &now, } + info.ApplyNetSetup(net) if err := fc.FinalizeClone(ctx, vmID, info, bootCfg, blobIDs); err != nil { fc.AbortLaunch(ctx, pid, sockPath, runDir, runtimeFiles) return nil, fmt.Errorf("finalize VM record: %w", err) diff --git a/hypervisor/firecracker/restore.go b/hypervisor/firecracker/restore.go index 7c197c01..ac774283 100644 --- a/hypervisor/firecracker/restore.go +++ b/hypervisor/firecracker/restore.go @@ -62,7 +62,7 @@ func (fc *Firecracker) restoreAfterExtract(ctx context.Context, vmID string, vmC sockPath := hypervisor.SocketPath(rec.RunDir) - pid, launchErr := fc.launchProcess(ctx, rec, sockPath) + pid, launchErr := fc.launchProcess(ctx, rec, sockPath, rec.ResolvedNetnsPath()) if launchErr != nil { return nil, fmt.Errorf("launch FC: %w", launchErr) } diff --git a/hypervisor/firecracker/start.go b/hypervisor/firecracker/start.go index 0b086cfa..983aa8b7 100644 --- a/hypervisor/firecracker/start.go +++ b/hypervisor/firecracker/start.go @@ -38,7 +38,7 @@ func (fc *Firecracker) startOne(ctx context.Context, id string) error { sockPath := hypervisor.SocketPath(rec.RunDir) - pid, err := fc.launchProcess(ctx, rec, sockPath) + pid, err := fc.launchProcess(ctx, rec, sockPath, rec.ResolvedNetnsPath()) if err != nil { fc.MarkError(ctx, id) return fmt.Errorf("launch VM: %w", err) @@ -137,7 +137,7 @@ func (fc *Firecracker) configureVM(ctx context.Context, hc *http.Client, rec *hy } // launchProcess starts firecracker, sets up PTY+console relay, waits for socket. -func (fc *Firecracker) launchProcess(ctx context.Context, rec *hypervisor.VMRecord, sockPath string) (int, error) { +func (fc *Firecracker) launchProcess(ctx context.Context, rec *hypervisor.VMRecord, sockPath, netnsPath string) (int, error) { fcLog := fc.LogFilePath(rec.LogDir) // FC opens log O_WRONLY|O_APPEND without O_CREATE — touch first. if f, createErr := os.Create(fcLog); createErr == nil { //nolint:gosec @@ -166,7 +166,7 @@ func (fc *Firecracker) launchProcess(ctx context.Context, rec *hypervisor.VMReco Cmd: fcCmd, PIDPath: fc.PIDFilePath(rec.RunDir), SockPath: sockPath, - NetnsPath: rec.ResolvedNetnsPath(), + NetnsPath: netnsPath, OnFail: func() { _ = master.Close() }, }) if err != nil { diff --git a/network/bridge/bridge_linux.go b/network/bridge/bridge_linux.go index 38ec83dc..20b19fc4 100644 --- a/network/bridge/bridge_linux.go +++ b/network/bridge/bridge_linux.go @@ -67,7 +67,7 @@ func (b *Bridge) Verify(_ context.Context, vmID string) error { return nil } -// Prepare is a no-op: bridge mode keeps CH in the host netns. +// Prepare is a no-op for bridge mode. func (b *Bridge) Prepare(_ context.Context, _ string, _ *types.VMConfig) (string, error) { return "", nil } diff --git a/network/cni/create.go b/network/cni/create.go index 60fa99f4..4c42f275 100644 --- a/network/cni/create.go +++ b/network/cni/create.go @@ -17,7 +17,7 @@ import ( "github.com/cocoonstack/cocoon/utils" ) -// Prepare creates the per-VM netns idempotently; returns "" when no CNI conflist is configured. +// Prepare creates the per-VM netns; returns "" with no conflist. func (c *CNI) Prepare(_ context.Context, vmID string, _ *types.VMConfig) (string, error) { if c.cniConf == nil { return "", nil diff --git a/network/network.go b/network/network.go index 5c7c7d70..9abf7f0f 100644 --- a/network/network.go +++ b/network/network.go @@ -23,7 +23,7 @@ type AddSpec struct { type Network interface { Type() string Verify(ctx context.Context, vmID string) error - // Prepare provisions per-VM state regardless of NIC count; returns the netns path or "". + // Prepare provisions per-VM state; returns the netns path or "". Prepare(ctx context.Context, vmID string, vmCfg *types.VMConfig) (string, error) Add(ctx context.Context, vmID string, vmCfg *types.VMConfig, specs ...AddSpec) ([]*types.NetworkConfig, error) Remove(ctx context.Context, vmID string, indices ...int) error diff --git a/types/vm.go b/types/vm.go index c16b10ec..bbee337e 100644 --- a/types/vm.go +++ b/types/vm.go @@ -36,7 +36,7 @@ type VMConfig struct { DataDisks []DataDiskSpec `json:"-"` // populated from --data-disk; consumed by Create } -// NetSetup is what initNetwork hands the hypervisor: backend identity + netns + NICs. +// NetSetup is the network handoff from initNetwork to the hypervisor. type NetSetup struct { Backend string NetnsPath string @@ -60,7 +60,7 @@ type VM struct { NetworkConfigs []*NetworkConfig `json:"network_configs,omitempty"` StorageConfigs []*StorageConfig `json:"storage_configs,omitempty"` - // Host networking at VM level so 0-NIC VMs still carry backend + netns. + // VM-level so 0-NIC VMs still carry backend + netns. NetBackend string `json:"net_backend,omitempty"` NetnsPath string `json:"netns_path,omitempty"` NetBridgeDev string `json:"net_bridge_dev,omitempty"` @@ -112,8 +112,11 @@ func (cfg *VMConfig) Validate() error { return nil } -// ResolvedNetnsPath returns the netns where CH runs (NIC[0] fallback for old records). +// ResolvedNetnsPath returns NetnsPath, with NIC[0] fallback. func (v *VM) ResolvedNetnsPath() string { + if v == nil { + return "" + } if v.NetnsPath != "" { return v.NetnsPath } @@ -123,8 +126,11 @@ func (v *VM) ResolvedNetnsPath() string { return "" } -// ResolvedNetBackend returns the host network backend (NIC[0] fallback for old records). +// ResolvedNetBackend returns NetBackend, with NIC[0] fallback. func (v *VM) ResolvedNetBackend() string { + if v == nil { + return "" + } if v.NetBackend != "" { return v.NetBackend } @@ -137,8 +143,11 @@ func (v *VM) ResolvedNetBackend() string { return "" } -// ResolvedNetBridgeDev returns the bridge device (NIC[0] fallback for old records). +// ResolvedNetBridgeDev returns NetBridgeDev, with NIC[0] fallback. func (v *VM) ResolvedNetBridgeDev() string { + if v == nil { + return "" + } if v.NetBridgeDev != "" { return v.NetBridgeDev } @@ -147,3 +156,20 @@ func (v *VM) ResolvedNetBridgeDev() string { } return "" } + +// IsBridge reports whether the VM uses the bridge backend. +func (v *VM) IsBridge() bool { return v.ResolvedNetBackend() == BackendBridge } + +// IsCNI reports whether the VM uses the CNI backend. +func (v *VM) IsCNI() bool { return v.ResolvedNetBackend() == BackendCNI } + +// ApplyNetSetup copies network fields from net into v. +func (v *VM) ApplyNetSetup(net NetSetup) { + if v == nil { + return + } + v.NetworkConfigs = net.NICs + v.NetBackend = net.Backend + v.NetnsPath = net.NetnsPath + v.NetBridgeDev = net.BridgeDev +} From 3811e1835826e0fb6380dd210734181f22c90ea1 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 13 May 2026 16:09:19 +0800 Subject: [PATCH 3/5] test(types): cover nil receiver + IsBridge/IsCNI --- types/vm_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/types/vm_test.go b/types/vm_test.go index 54f30f18..1e3015c5 100644 --- a/types/vm_test.go +++ b/types/vm_test.go @@ -224,3 +224,35 @@ func TestVMResolvedNetFields(t *testing.T) { }) } } + +func TestVMResolvedNetNilReceiver(t *testing.T) { + var v *VM + if got := v.ResolvedNetnsPath(); got != "" { + t.Errorf("nil ResolvedNetnsPath = %q, want \"\"", got) + } + if got := v.ResolvedNetBackend(); got != "" { + t.Errorf("nil ResolvedNetBackend = %q, want \"\"", got) + } + if got := v.ResolvedNetBridgeDev(); got != "" { + t.Errorf("nil ResolvedNetBridgeDev = %q, want \"\"", got) + } + if v.IsBridge() || v.IsCNI() { + t.Errorf("nil IsBridge/IsCNI both want false") + } + v.ApplyNetSetup(NetSetup{Backend: "cni"}) // must not panic +} + +func TestVMIsBridgeIsCNI(t *testing.T) { + bridge := VM{NetBackend: BackendBridge} + cni := VM{NetBackend: BackendCNI} + empty := VM{} + if !bridge.IsBridge() || bridge.IsCNI() { + t.Errorf("bridge: IsBridge=true, IsCNI=false") + } + if !cni.IsCNI() || cni.IsBridge() { + t.Errorf("cni: IsCNI=true, IsBridge=false") + } + if empty.IsBridge() || empty.IsCNI() { + t.Errorf("empty: both false") + } +} From 8cedaaa413dcd05a219ed22dded55ffe87952067 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 13 May 2026 16:20:08 +0800 Subject: [PATCH 4/5] =?UTF-8?q?refactor(types):=20drop=20IsBridge/IsCNI=20?= =?UTF-8?q?=E2=80=94=20cmd=20routing=20leak?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/vm/lifecycle.go | 2 +- cmd/vm/netresize.go | 5 +++-- types/vm.go | 6 ------ types/vm_test.go | 18 ------------------ 4 files changed, 4 insertions(+), 27 deletions(-) diff --git a/cmd/vm/lifecycle.go b/cmd/vm/lifecycle.go index 8514f272..53b380d0 100644 --- a/cmd/vm/lifecycle.go +++ b/cmd/vm/lifecycle.go @@ -411,7 +411,7 @@ func providerForVM(conf *config.Config, cniProvider network.Network, bridgeCache if vm == nil { return nil, fmt.Errorf("no VM record") } - if vm.IsBridge() { + if vm.ResolvedNetBackend() == types.BackendBridge { dev := vm.ResolvedNetBridgeDev() if dev == "" { return nil, fmt.Errorf("bridge backend but no bridge device persisted") diff --git a/cmd/vm/netresize.go b/cmd/vm/netresize.go index 219f67fc..7e0805a4 100644 --- a/cmd/vm/netresize.go +++ b/cmd/vm/netresize.go @@ -45,10 +45,11 @@ func (h Handler) NetResize(cmd *cobra.Command, args []string) error { // plumbingForVM picks the provider from persisted VM state; 0-NIC works because NetBackend persists. func plumbingForVM(conf *config.Config, vm *types.VM) (network.Network, error) { - if vm.ResolvedNetBackend() == "" { + backend := vm.ResolvedNetBackend() + if backend == "" { return nil, fmt.Errorf("no network backend on VM; cannot resize") } - if vm.IsCNI() && vm.ResolvedNetnsPath() == "" { + if backend == types.BackendCNI && vm.ResolvedNetnsPath() == "" { return nil, fmt.Errorf("CNI backend but no netns; resize would target host netns") } return providerForVM(conf, nil, map[string]network.Network{}, vm) diff --git a/types/vm.go b/types/vm.go index bbee337e..9a67023d 100644 --- a/types/vm.go +++ b/types/vm.go @@ -157,12 +157,6 @@ func (v *VM) ResolvedNetBridgeDev() string { return "" } -// IsBridge reports whether the VM uses the bridge backend. -func (v *VM) IsBridge() bool { return v.ResolvedNetBackend() == BackendBridge } - -// IsCNI reports whether the VM uses the CNI backend. -func (v *VM) IsCNI() bool { return v.ResolvedNetBackend() == BackendCNI } - // ApplyNetSetup copies network fields from net into v. func (v *VM) ApplyNetSetup(net NetSetup) { if v == nil { diff --git a/types/vm_test.go b/types/vm_test.go index 1e3015c5..98b262c7 100644 --- a/types/vm_test.go +++ b/types/vm_test.go @@ -236,23 +236,5 @@ func TestVMResolvedNetNilReceiver(t *testing.T) { if got := v.ResolvedNetBridgeDev(); got != "" { t.Errorf("nil ResolvedNetBridgeDev = %q, want \"\"", got) } - if v.IsBridge() || v.IsCNI() { - t.Errorf("nil IsBridge/IsCNI both want false") - } v.ApplyNetSetup(NetSetup{Backend: "cni"}) // must not panic } - -func TestVMIsBridgeIsCNI(t *testing.T) { - bridge := VM{NetBackend: BackendBridge} - cni := VM{NetBackend: BackendCNI} - empty := VM{} - if !bridge.IsBridge() || bridge.IsCNI() { - t.Errorf("bridge: IsBridge=true, IsCNI=false") - } - if !cni.IsCNI() || cni.IsBridge() { - t.Errorf("cni: IsCNI=true, IsBridge=false") - } - if empty.IsBridge() || empty.IsCNI() { - t.Errorf("empty: both false") - } -} From fa303b75a5bc2a7bf21cdbe8fe74999ebc66b13d Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 13 May 2026 16:25:53 +0800 Subject: [PATCH 5/5] refactor(types): embed NetSetup in VM, drop ApplyNetSetup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single source of truth for VM network state. Anonymous embed preserves JSON wire format (net_backend, netns_path, net_bridge_dev, network_configs all stay flat on VM). Old records load unchanged. - NetSetup gains JSON tags matching VM's existing field names - VM drops the four duplicate fields; embeds NetSetup - info.ApplyNetSetup(net) → info.NetSetup = net (direct assignment) - setup.NICs → setup.NetworkConfigs (rename in NetSetup) --- cmd/vm/run.go | 4 ++-- cmd/vm/status_test.go | 8 +++++--- hypervisor/cloudhypervisor/clone.go | 4 ++-- hypervisor/create.go | 4 ++-- hypervisor/firecracker/clone.go | 4 ++-- types/vm.go | 32 ++++++++--------------------- types/vm_test.go | 13 ++++++------ 7 files changed, 28 insertions(+), 41 deletions(-) diff --git a/cmd/vm/run.go b/cmd/vm/run.go index e3d2ed39..86c1c589 100644 --- a/cmd/vm/run.go +++ b/cmd/vm/run.go @@ -490,7 +490,7 @@ func initNetwork(ctx context.Context, conf *config.Config, vmID string, nics int if nics <= 0 && backend == types.BackendCNI && nsPath == "" { return netProvider, types.NetSetup{}, nil } - setup := types.NetSetup{Backend: backend, NetnsPath: nsPath, BridgeDev: bridgeDev} + setup := types.NetSetup{NetBackend: backend, NetnsPath: nsPath, NetBridgeDev: bridgeDev} if nics <= 0 { return netProvider, setup, nil } @@ -503,7 +503,7 @@ func initNetwork(ctx context.Context, conf *config.Config, vmID string, nics int rollbackNetwork(ctx, netProvider, vmID) return nil, types.NetSetup{}, fmt.Errorf("configure network: %w", err) } - setup.NICs = configs + setup.NetworkConfigs = configs return netProvider, setup, nil } diff --git a/cmd/vm/status_test.go b/cmd/vm/status_test.go index f6a5a6ab..6ca76ecd 100644 --- a/cmd/vm/status_test.go +++ b/cmd/vm/status_test.go @@ -62,9 +62,11 @@ func TestVMIPsAndSort(t *testing.T) { }, }, CreatedAt: now.Add(-time.Minute), - NetworkConfigs: []*types.NetworkConfig{ - {Network: &types.Network{IP: "10.0.0.2"}}, - {Network: &types.Network{IP: "10.0.0.3"}}, + NetSetup: types.NetSetup{ + NetworkConfigs: []*types.NetworkConfig{ + {Network: &types.Network{IP: "10.0.0.2"}}, + {Network: &types.Network{IP: "10.0.0.3"}}, + }, }, }, } diff --git a/hypervisor/cloudhypervisor/clone.go b/hypervisor/cloudhypervisor/clone.go index 2e010253..801ed0d7 100644 --- a/hypervisor/cloudhypervisor/clone.go +++ b/hypervisor/cloudhypervisor/clone.go @@ -31,7 +31,7 @@ func (ch *CloudHypervisor) Clone(ctx context.Context, vmID string, vmCfg *types. } func (ch *CloudHypervisor) cloneAfterExtract(ctx context.Context, vmID string, vmCfg *types.VMConfig, net types.NetSetup, runDir, logDir string, now time.Time) (*types.VM, error) { - networkConfigs := net.NICs + networkConfigs := net.NetworkConfigs logger := log.WithFunc("cloudhypervisor.Clone") chConfigPath := filepath.Join(runDir, "config.json") @@ -129,9 +129,9 @@ func (ch *CloudHypervisor) cloneAfterExtract(ctx context.Context, vmID string, v info := &types.VM{ ID: vmID, Hypervisor: typ, State: types.VMStateRunning, Config: *vmCfg, StorageConfigs: storageConfigs, + NetSetup: net, CreatedAt: now, UpdatedAt: now, StartedAt: &now, } - info.ApplyNetSetup(net) if err := ch.FinalizeClone(ctx, vmID, info, bootCfg, nil); err != nil { ch.AbortLaunch(ctx, pid, sockPath, runDir, runtimeFiles) return nil, fmt.Errorf("finalize VM record: %w", err) diff --git a/hypervisor/create.go b/hypervisor/create.go index fac479d3..61c2413a 100644 --- a/hypervisor/create.go +++ b/hypervisor/create.go @@ -99,7 +99,7 @@ func (b *Backend) CreateSequence(ctx context.Context, id string, spec CreateSpec bootCopy = &bc } - preparedStorage, err := spec.Prepare(ctx, id, spec.VMCfg, spec.StorageConfigs, spec.Net.NICs, bootCopy) + preparedStorage, err := spec.Prepare(ctx, id, spec.VMCfg, spec.StorageConfigs, spec.Net.NetworkConfigs, bootCopy) if err != nil { return nil, err } @@ -110,9 +110,9 @@ func (b *Backend) CreateSequence(ctx context.Context, id string, spec CreateSpec info := &types.VM{ ID: id, Hypervisor: b.Typ, State: types.VMStateCreated, Config: *spec.VMCfg, StorageConfigs: preparedStorage, + NetSetup: spec.Net, CreatedAt: now, UpdatedAt: now, } - info.ApplyNetSetup(spec.Net) if err = b.FinalizeCreate(ctx, id, info, bootCopy, blobIDs); err != nil { return nil, fmt.Errorf("finalize VM record: %w", err) } diff --git a/hypervisor/firecracker/clone.go b/hypervisor/firecracker/clone.go index 8cf4cb1e..841ebdd1 100644 --- a/hypervisor/firecracker/clone.go +++ b/hypervisor/firecracker/clone.go @@ -28,7 +28,7 @@ func (fc *Firecracker) Clone(ctx context.Context, vmID string, vmCfg *types.VMCo } func (fc *Firecracker) cloneAfterExtract(ctx context.Context, vmID string, vmCfg *types.VMConfig, net types.NetSetup, runDir, logDir string, now time.Time) (*types.VM, error) { - networkConfigs := net.NICs + networkConfigs := net.NetworkConfigs logger := log.WithFunc("firecracker.Clone") meta, err := hypervisor.LoadAndValidateMeta(runDir, fc.conf.RootDir, fc.conf.Config.RunDir) @@ -101,9 +101,9 @@ func (fc *Firecracker) cloneAfterExtract(ctx context.Context, vmID string, vmCfg info := &types.VM{ ID: vmID, Hypervisor: typ, State: types.VMStateRunning, Config: *vmCfg, StorageConfigs: storageConfigs, + NetSetup: net, CreatedAt: now, UpdatedAt: now, StartedAt: &now, } - info.ApplyNetSetup(net) if err := fc.FinalizeClone(ctx, vmID, info, bootCfg, blobIDs); err != nil { fc.AbortLaunch(ctx, pid, sockPath, runDir, runtimeFiles) return nil, fmt.Errorf("finalize VM record: %w", err) diff --git a/types/vm.go b/types/vm.go index 9a67023d..063e4211 100644 --- a/types/vm.go +++ b/types/vm.go @@ -36,12 +36,13 @@ type VMConfig struct { DataDisks []DataDiskSpec `json:"-"` // populated from --data-disk; consumed by Create } -// NetSetup is the network handoff from initNetwork to the hypervisor. +// NetSetup is the VM's host networking state: backend, netns, bridge, and attached NICs. +// Embedded into VM and also used as the initNetwork → hypervisor handoff. type NetSetup struct { - Backend string - NetnsPath string - BridgeDev string - NICs []*NetworkConfig + NetBackend string `json:"net_backend,omitempty"` + NetnsPath string `json:"netns_path,omitempty"` + NetBridgeDev string `json:"net_bridge_dev,omitempty"` + NetworkConfigs []*NetworkConfig `json:"network_configs,omitempty"` } // VM is the runtime record for a VM, persisted by the hypervisor backend. @@ -56,14 +57,10 @@ type VM struct { SocketPath string `json:"socket_path,omitempty"` // CH API Unix socket VsockSocket string `json:"vsock_socket,omitempty"` // hybrid vsock UDS for cocoon-agent - // Attached resources — promoted into VMRecord via embedding. - NetworkConfigs []*NetworkConfig `json:"network_configs,omitempty"` - StorageConfigs []*StorageConfig `json:"storage_configs,omitempty"` + // Network — embedded; fields promote (vm.NetBackend, vm.NetworkConfigs, ...). + NetSetup - // VM-level so 0-NIC VMs still carry backend + netns. - NetBackend string `json:"net_backend,omitempty"` - NetnsPath string `json:"netns_path,omitempty"` - NetBridgeDev string `json:"net_bridge_dev,omitempty"` + StorageConfigs []*StorageConfig `json:"storage_configs,omitempty"` // FirstBooted is true after the VM has been started at least once. // Used to skip cidata attachment on subsequent starts (cloudimg only). @@ -156,14 +153,3 @@ func (v *VM) ResolvedNetBridgeDev() string { } return "" } - -// ApplyNetSetup copies network fields from net into v. -func (v *VM) ApplyNetSetup(net NetSetup) { - if v == nil { - return - } - v.NetworkConfigs = net.NICs - v.NetBackend = net.Backend - v.NetnsPath = net.NetnsPath - v.NetBridgeDev = net.BridgeDev -} diff --git a/types/vm_test.go b/types/vm_test.go index 98b262c7..97e2370d 100644 --- a/types/vm_test.go +++ b/types/vm_test.go @@ -180,29 +180,29 @@ func TestVMResolvedNetFields(t *testing.T) { }{ { name: "VM-level fields populated", - vm: VM{NetBackend: "cni", NetnsPath: "/var/run/netns/cocoon-x", NetBridgeDev: ""}, + vm: VM{NetSetup: NetSetup{NetBackend: "cni", NetnsPath: "/var/run/netns/cocoon-x"}}, wantNetnsPath: "/var/run/netns/cocoon-x", wantNetBackend: "cni", }, { name: "fallback to NIC[0] when VM-level empty", - vm: VM{NetworkConfigs: []*NetworkConfig{ + vm: VM{NetSetup: NetSetup{NetworkConfigs: []*NetworkConfig{ {Backend: BackendBridge, NetnsPath: "", BridgeDev: "br0"}, - }}, + }}}, wantNetBackend: BackendBridge, wantNetBridgeDev: "br0", }, { name: "pre-bridge record (empty Backend) maps to CNI", - vm: VM{NetworkConfigs: []*NetworkConfig{ + vm: VM{NetSetup: NetSetup{NetworkConfigs: []*NetworkConfig{ {NetnsPath: "/var/run/netns/cocoon-y"}, - }}, + }}}, wantNetnsPath: "/var/run/netns/cocoon-y", wantNetBackend: BackendCNI, }, { name: "VM-level wins over NIC[0]", - vm: VM{NetBackend: "cni", NetworkConfigs: []*NetworkConfig{{Backend: BackendBridge}}}, + vm: VM{NetSetup: NetSetup{NetBackend: "cni", NetworkConfigs: []*NetworkConfig{{Backend: BackendBridge}}}}, wantNetBackend: "cni", }, { @@ -236,5 +236,4 @@ func TestVMResolvedNetNilReceiver(t *testing.T) { if got := v.ResolvedNetBridgeDev(); got != "" { t.Errorf("nil ResolvedNetBridgeDev = %q, want \"\"", got) } - v.ApplyNetSetup(NetSetup{Backend: "cni"}) // must not panic }