Skip to content

Commit 831fe9c

Browse files
committed
refactor: reduce parameter sprawl and complete docs
- printCHDebug: replace 14 positional params with vmCfg struct - restoreAndResumeClone: replace 12 positional params with cloneResumeOpts - README: add --bridge and --disk-queue-size to flags tables - KNOWN_ISSUES: document clone disk num_queues immutability
1 parent 17fa530 commit 831fe9c

4 files changed

Lines changed: 48 additions & 18 deletions

File tree

KNOWN_ISSUES.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,16 @@ OCI VMs use the kernel `ip=` boot parameter for network configuration. While mul
2727

2828
**Workaround**: the post-clone setup hints write persistent MAC-based systemd-networkd configs for **all** NICs. These survive reboots and correctly configure every interface regardless of the kernel `ip=` limitation.
2929

30+
## Clone/restore disk queue count is immutable
31+
32+
When cloning or restoring a VM with a different `--cpu` value, the disk `num_queues` (one queue per vCPU) retains the snapshot's original value. This is because `num_queues` is part of the virtio-blk device state baked into the binary snapshot — changing it in `config.json` causes Cloud Hypervisor to crash on `vm.restore`.
33+
34+
`--disk-queue-size` (ring depth per queue) **is** correctly patched on clone/restore, since it is a software-layer configuration that CH re-reads from `config.json` during restore.
35+
36+
**Consequence**: a VM cloned with `--cpu 8` from a 2-vCPU snapshot still runs disks with 2 queues (not 8). Disk I/O is functional but does not scale to the new vCPU count.
37+
38+
**Workaround**: none — this is a CH architectural limitation. Create a fresh VM with the desired CPU count instead of cloning if disk multi-queue scaling is critical.
39+
3040
## Cloud image UEFI boot compatibility
3141

3242
Cocoon uses [rust-hypervisor-firmware](https://github.com/cloud-hypervisor/rust-hypervisor-firmware) (`CLOUDHV.fd`) for cloud image UEFI boot. This firmware implements a minimal EFI specification and does **not** support the `InstallMultipleProtocolInterfaces()` call required by newer distributions.

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ Applies to `cocoon vm create`, `cocoon vm run`, and `cocoon vm debug`:
181181
| `--queue-size` | `0` (default 512) | Virtio-net ring depth per queue (larger = better bulk throughput, smaller = better RPC latency; CH only, ignored by FC) |
182182
| `--disk-queue-size` | `0` (default 512) | Virtio-blk ring depth per device (CH only, ignored by FC) |
183183
| `--network` | empty (default) | CNI conflist name (empty = first conflist) |
184+
| `--bridge` | empty | TAP-on-bridge mode (value is bridge device, e.g. `cni0`); mutually exclusive with `--network` |
184185
| `--windows` | `false` | Windows guest (UEFI boot, kvm_hyperv=on, no cidata) |
185186

186187
### Clone Flags
@@ -197,6 +198,7 @@ Applies to `cocoon vm clone`:
197198
| `--queue-size` | `0` (inherit) | Virtio-net ring depth per queue (0 = inherit from snapshot) |
198199
| `--disk-queue-size` | `0` (inherit) | Virtio-blk ring depth per device (0 = inherit from snapshot; CH only) |
199200
| `--network` | empty (inherit) | CNI conflist name (empty = inherit from source VM) |
201+
| `--bridge` | empty | TAP-on-bridge mode (value is bridge device); mutually exclusive with `--network` |
200202

201203
### Snapshot Flags
202204

cmd/vm/debug.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (h Handler) Debug(cmd *cobra.Command, args []string) error {
6161
if balloon == 0 {
6262
balloon = memoryMB / 2 //nolint:mnd
6363
}
64-
printCHDebug(storageConfigs, boot, vmCfg.Name, vmCfg.Image, cowPath, chBin, vmCfg.CPU, maxCPU, memoryMB, balloon, cowSizeGB, vmCfg.DiskQueueSize, vmCfg.Windows, boot.KernelPath != "")
64+
printCHDebug(storageConfigs, boot, vmCfg, cowPath, chBin, maxCPU, memoryMB, balloon, cowSizeGB, boot.KernelPath != "")
6565
}
6666
return nil
6767
}
@@ -137,10 +137,13 @@ func printFCDebug(configs []*types.StorageConfig, boot *types.BootConfig, vmCfg
137137
fmt.Printf(" -d '{\"action_type\": \"InstanceStart\"}'\n")
138138
}
139139

140-
func printCHDebug(configs []*types.StorageConfig, boot *types.BootConfig, vmName, image, cowPath, chBin string, cpu, maxCPU, memory, balloon, cowSize, diskQueueSize int, windows, directBoot bool) {
140+
func printCHDebug(configs []*types.StorageConfig, boot *types.BootConfig, vmCfg *types.VMConfig, cowPath, chBin string, maxCPU, memory, balloon, cowSize int, directBoot bool) {
141+
cpu := vmCfg.CPU
142+
diskQueueSize := vmCfg.DiskQueueSize
143+
141144
if directBoot {
142145
if cowPath == "" {
143-
cowPath = fmt.Sprintf("cow-%s.raw", vmName)
146+
cowPath = fmt.Sprintf("cow-%s.raw", vmCfg.Name)
144147
}
145148
debugConfigs := append(append([]*types.StorageConfig(nil), configs...),
146149
&types.StorageConfig{Path: cowPath, RO: false, Serial: hypervisor.CowSerial})
@@ -154,7 +157,7 @@ func printCHDebug(configs []*types.StorageConfig, boot *types.BootConfig, vmName
154157
fmt.Printf("truncate -s %dG %s\n", cowSize, cowPath)
155158
fmt.Printf("mkfs.ext4 -F -m 0 -q -E lazy_itable_init=1,lazy_journal_init=1,discard %s\n", cowPath)
156159
fmt.Println()
157-
fmt.Printf("# Launch VM: %s (image: %s, boot: direct kernel)\n", vmName, image)
160+
fmt.Printf("# Launch VM: %s (image: %s, boot: direct kernel)\n", vmCfg.Name, vmCfg.Image)
158161
fmt.Printf("%s \\\n", chBin)
159162
fmt.Printf(" --kernel %s \\\n", boot.KernelPath)
160163
fmt.Printf(" --initramfs %s \\\n", boot.InitrdPath)
@@ -166,7 +169,7 @@ func printCHDebug(configs []*types.StorageConfig, boot *types.BootConfig, vmName
166169
fmt.Printf(" --cmdline \"%s\" \\\n", cmdline)
167170
} else {
168171
if cowPath == "" {
169-
cowPath = fmt.Sprintf("cow-%s.qcow2", vmName)
172+
cowPath = fmt.Sprintf("cow-%s.qcow2", vmCfg.Name)
170173
}
171174
basePath := configs[0].Path
172175
fmt.Println("# Prepare COW overlay")
@@ -175,14 +178,14 @@ func printCHDebug(configs []*types.StorageConfig, boot *types.BootConfig, vmName
175178
fmt.Printf("qemu-img resize %s %dG\n", cowPath, cowSize)
176179
}
177180
fmt.Println()
178-
fmt.Printf("# Launch VM: %s (image: %s, boot: UEFI firmware)\n", vmName, image)
181+
fmt.Printf("# Launch VM: %s (image: %s, boot: UEFI firmware)\n", vmCfg.Name, vmCfg.Image)
179182
fmt.Printf("%s \\\n", chBin)
180183
fmt.Printf(" --firmware %s \\\n", boot.FirmwarePath)
181184
fmt.Printf(" --disk \\\n")
182185
diskArgs := cloudhypervisor.DebugDiskCLIArgs([]*types.StorageConfig{{Path: cowPath, RO: false}}, cpu, diskQueueSize)
183186
fmt.Printf(" \"%s\" \\\n", diskArgs[0])
184187
}
185-
printCommonCHArgs(cpu, maxCPU, memory, balloon, windows)
188+
printCommonCHArgs(cpu, maxCPU, memory, balloon, vmCfg.Windows)
186189
}
187190

188191
func printCommonCHArgs(cpu, maxCPU, memory, balloon int, windows bool) {

hypervisor/cloudhypervisor/clone.go

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,16 @@ func (ch *CloudHypervisor) cloneAfterExtract(ctx context.Context, vmID string, v
123123
return nil, fmt.Errorf("launch CH: %w", err)
124124
}
125125

126-
if err := ch.restoreAndResumeClone(ctx, pid, sockPath, runDir, directBoot, vmCfg.Windows, hadCidataInSnapshot, storageConfigs, networkConfigs, chCfg, vmCfg.CPU, vmCfg.DiskQueueSize); err != nil {
126+
if err := ch.restoreAndResumeClone(ctx, pid, sockPath, runDir, &cloneResumeOpts{
127+
directBoot: directBoot,
128+
windows: vmCfg.Windows,
129+
hadCidataInSnapshot: hadCidataInSnapshot,
130+
storageConfigs: storageConfigs,
131+
networkConfigs: networkConfigs,
132+
snapshotCfg: chCfg,
133+
cpu: vmCfg.CPU,
134+
diskQueueSize: vmCfg.DiskQueueSize,
135+
}); err != nil {
127136
return nil, err
128137
}
129138

@@ -158,15 +167,22 @@ func (ch *CloudHypervisor) cloneAfterExtract(ctx context.Context, vmID string, v
158167
return &info, nil
159168
}
160169

170+
type cloneResumeOpts struct {
171+
directBoot bool
172+
windows bool
173+
hadCidataInSnapshot bool
174+
storageConfigs []*types.StorageConfig
175+
networkConfigs []*types.NetworkConfig
176+
snapshotCfg *chVMConfig
177+
cpu int
178+
diskQueueSize int
179+
}
180+
161181
func (ch *CloudHypervisor) restoreAndResumeClone(
162182
ctx context.Context,
163183
pid int,
164184
sockPath, runDir string,
165-
directBoot, windows, hadCidataInSnapshot bool,
166-
storageConfigs []*types.StorageConfig,
167-
networkConfigs []*types.NetworkConfig,
168-
snapshotCfg *chVMConfig,
169-
cpu, diskQueueSize int,
185+
opts *cloneResumeOpts,
170186
) (err error) {
171187
defer func() {
172188
if err != nil {
@@ -179,16 +195,15 @@ func (ch *CloudHypervisor) restoreAndResumeClone(
179195
}
180196
hc := utils.NewSocketHTTPClient(sockPath)
181197

182-
// Replace snapshot NICs so the guest resumes with the clone's MACs.
183-
if err = hotSwapNets(ctx, hc, snapshotCfg.Nets, networkConfigs); err != nil {
198+
if err = hotSwapNets(ctx, hc, opts.snapshotCfg.Nets, opts.networkConfigs); err != nil {
184199
return fmt.Errorf("hot-swap NICs: %w", err)
185200
}
186201

187-
if !directBoot && !windows && !hadCidataInSnapshot {
188-
if len(storageConfigs) == 0 {
202+
if !opts.directBoot && !opts.windows && !opts.hadCidataInSnapshot {
203+
if len(opts.storageConfigs) == 0 {
189204
return fmt.Errorf("vm.add-disk (cidata): missing storage config")
190205
}
191-
cidataDisk := storageConfigToDisk(storageConfigs[len(storageConfigs)-1], cpu, diskQueueSize)
206+
cidataDisk := storageConfigToDisk(opts.storageConfigs[len(opts.storageConfigs)-1], opts.cpu, opts.diskQueueSize)
192207
if err = addDiskVM(ctx, hc, cidataDisk); err != nil {
193208
return fmt.Errorf("vm.add-disk (cidata): %w", err)
194209
}

0 commit comments

Comments
 (0)