Repository navigation
Conversation
-->
✱ stlc build✅ go code · compare
✅ python code · compare
✅ typescript code · compare
Diagnostics: ❗ 0 new / 1 total error, 💡 0 new / 5 total note
Build metadata
This comment is auto-generated by stlc and is kept up to date as you push. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4849059. Configure here.
| meta, err := m.loadMetadata(entry.Name()) | ||
| if err != nil { | ||
| return fmt.Errorf("check Mac identity admission: %w", err) | ||
| } |
There was a problem hiding this comment.
Identity admission fails on metadata errors
Medium Severity
checkMacOSIdentityAvailable treats any loadMetadata failure as fatal, including missing metadata for an in-progress create. A concurrent Linux or macOS create that has already made a guest directory but not yet written metadata causes an unrelated macOS start or create to fail admission.
Reviewed by Cursor Bugbot for commit 4849059. Configure here.
| } | ||
| if exit, ok := e.(*exec.ExitError); !ok || exit.ExitCode() != 1 { | ||
| return nil, fmt.Errorf("cannot verify storage is closed: %v", e) | ||
| } |
There was a problem hiding this comment.
lsof probe leaks process groups
Medium Severity
The import lsof probe runs through exec.CommandContext without a dedicated process group and without SIGKILLing that group on every completion path. Context cancel only stops the wrapper, so lsof descendants can leak after success, ErrWaitDelay, or a wrapper that exits first.
Triggered by learned rule: Subprocess binary probes must use process groups and kill on all completion paths
Reviewed by Cursor Bugbot for commit 4849059. Configure here.
chruffins
changed the title
|
Live QA of the reviewed combined PR1–3 sources plus PR4 patches on Apple silicon passed normal authenticated HTTP create from a digest-pinned, previously HTTP-pulled installed macOS OCI artifact (registry offline). Verified actual macOS 27.0.1/26A434 cold boot over pinned-key SSH, inherited 4CPU/8GiB template defaults, duplicate-identity HTTP409, live-guest API restart recovery with unchanged guest boot time, HTTP stop/start with fresh boot time and durable guest-only marker, and unchanged full SHA256 of the immutable materialized base disk+aux afterward. Stop/start here used system-agent-disabled fallback, not verified graceful root-agent shutdown. QA used an isolated data dir/slot; original instance/API/storage were untouched and the QA VM/API ended stopped. Runtime issue found: the long QA data directory generated an overlong AF_UNIX socket path. VZ reported VM started, then control listener failed |
|
Simplification update at e6e6ab7: shared bounded complete-machine bundle decoding and one macOS request-default preparation; shared concrete create/start network + boot-config preparation. macOS preserves template MAC and avoids Linux allocation/proxy/config-disk dependencies; Linux rollback and request preservation have synthetic tests. Focused instance/API/image race tests passed 3 runs. Identity leases and stopped-storage/export requirements remain. All PRs remain draft. This is source-level/synthetic validation, not a new live boot or production build proof. Default Codex independent review was attempted but is still blocked by authentication (HTTP401). Published atomically after checking reviewer heads with explicit per-ref force-with-lease; prior heads preserved locally. |
chruffins
changed the title
614d302 to
c221190
Compare
8d8e740 to
0a99bcb
Compare
Experimental, administrator-provisioned macOS guests on Apple silicon through the normal image and instance APIs. Unsupported operations are rejected explicitly. Linux create, start and API paths are unchanged; macOS enters through hooks. Per-operation status: docs/macos-support.md.
0a99bcb to
93803d8
Compare
hiroTamada
left a comment
hiroTamada
left a comment
There was a problem hiding this comment.
reviewed at 93803d8. three findings:
-
P1 —
lib/instances/macos.go:301–305,lib/instances/process_identity.go:147–149: discover live shims before treating an instance as stopped. if the API crashes after spawning the shim but before recording its PID, the shim can survive without a listening control socket:cmd/vz-shim/main.go:80–93starts the VM before opening that socket.query.go:76–83classifies a missing socket as stopped; the newmacOSShimGonealso treats a refusing socket plus a missing PID as stopped.StopInstancethen returns success without termination (manager.go:685–693), and delete skips termination (delete.go:128) before removing storage/metadata. the new PID-discovery fallback is never reached through those shortcuts. please discover the shim before deciding it is stopped, require termination before destructive cleanup, and fail closed on discovery errors. add a regression for a live pre-listener shim with no recorded PID. this is a source-path finding, not a native crash reproduction. -
P2 —
cmd/api/api/macos.go:82–98: preserve explicit resource fields on oversized requests. bodies above 1 MiB pass through without field-presence tracking, butignoreLinuxCreateDefaultstreats missing tracking as proof that default-valued fields were omitted. reproduced with the actual validator and extracted helpers: the same JSON padded with whitespace loses explicitvcpus: 2,size: "1GB", andoverlay_size: "10GB". a two-CPU request can therefore use the image's CPU count instead; invalid memory and unsupported overlay settings can be replaced with image defaults rather than rejected. please reject oversized creates with 413 or preserve explicitness instead of guessing omission. add a boundary test checking downstream field values, not just body pass-through. -
P2 —
lib/instances/macos.go:207–208: check network MAC uniqueness separately from machine identity. admission skips existing guests with a different machine identifier, even if both network-enabled guests have the same preserved MAC. the shim lock is also keyed only by machine identifier, and this path bypasses Linux MAC allocation. reproduced the admission gap with different identifiers, the same MAC, and an existing running guest; the same-identifier control was rejected. duplicate MACs could cause DHCP/connectivity conflicts or a later native boot failure. please compare normalized MACs for active network-enabled guests as well as machine identifiers, with a distinct-ID/same-MAC rejection test. native duplicate-MAC behavior has not been verified.
validation: focused image validation, API config, hypervisor parser/validation, VM metrics/parser, and macOS reclaim-exclusion tests passed. API/instance package tests could not compile because required embedded binaries were missing. native VZ/APFS behavior was not tested.
This branch has not been deployed
2 participants
Summary
PR 1 of 6. Experimental macOS guests on Apple silicon through the existing image and instance APIs, using Virtualization.framework via the existing
vz-shim. Base:main.cmd/import-macosimports a stopped macOS VM bundle (disk, auxiliary storage, platform config) as adarwin/arm64machine image, using APFS clones.alwaysornever(on_failureis rejected: there is no workload exit status yet).macos_onlymode: a server that runs only macOS guests skips Linux kernel/initrd downloads and ingress, and refuses Linux creates.Size
About 950 lines of logic. Of the 4,000 added lines in the diff, 1,554 are non-test, non-doc, non-generated Go. Removing comments, blank lines and imports leaves about 1,215. A further ~265 are mechanical:
if !MacOSOnly/if stored.MacOS == nil, struct-tag realignment, and a smalllegacyLayoutrefactor (~50)The 1,554 is exact; the split of the rest is a per-hunk classification, about ±30 lines.
lib/instances/macos.go,macos_darwin.go; small parts oflogs.go,query.go)cloneMacOSStorage).macOSLeaseIP).checkMacOSIdentityAvailable).macOSShimGone,macOSStateWithoutShim).macos_bundle.govalidates a fixed-layout VM bundle (disk, auxiliary storage, config) and rejects unknown files and links.macos_import_darwin.goclones the bundle into image storage and writes metadata;cmd/import-macosis an 18-line wrapper.MacOSImageand itsValidate,darwin/arm64platform acceptance, the raw-disk filename in storage, and image resolution at create.cmd/vz-shim/macos_*.go)macos_identity.goholds a file lock on the machine identifier for the shim's lifetime, the second guard against two VMs sharing an identity.collector_darwin.go,vm_metrics/manager.go)ps, since macOS has no/proc.cmd/api/api/macos.go:RecordCreateFieldsandignoreLinuxCreateDefaults, so injectedvcpus: 2,1GBand10GBdon't override the image; a rejection reason for ingress rules targeting a macOS guest.instances.go: macOS skips default bandwidth and disk I/O limits.config.go:macos_only(VZ on Apple silicon only) andlisten_address.BootModeMacOSand its config.starter.govalidates one writable raw disk, at least 2 vCPUs and 4 GiB in whole MiB within host VZ limits, and no balloon, hotplug, PCI or Rosetta. The shim config carries the machine identity.vz_shim_pids*.go)vz-shimby the socket path in its command line, so stop and delete can kill a shim whose PID was never recorded (for example after an API crash mid-boot).Linux code touched
Linux behavior is unchanged; these are dispatch hooks.
lib/instances: create, start, stop, delete, query (state derivation), logs, update, fork/snapshot/standby admission, admission allocations, process identity.lib/hypervisor: a macOS boot mode in VM config and validation; the QEMU profile rejects it.lib/images: theMacOSImagetype, platform parsing, boot storage paths, tagging.cmd/api:macos_onlyconfig and startup gates; create, ingress, exec and image guards.vcpus: 2,size: 1GB,overlay_size: 10GB) into every create body. On Darwin hosts only, a middleware records which fields the client actually sent (bodies over 1 MiB pass through unrecorded), so a macOS create ignores only the injected defaults.lib/vm_metrics,lib/resources,lib/guestmemory,lib/paths: macOS stats hook, and skipping balloon and memory reclaim.Not supported in this PR
Volumes, env/commands, credentials, egress policy, bandwidth and disk I/O shaping, auto-standby, ingress, GPU/PCI devices and memory hotplug are rejected. Guest-agent features arrive in #500; snapshots and clones in #506. Current status is in
docs/macos-support.md.Testing
main(tests needing KVM,mkfs.erofsor Docker Hub).vcpus: 2, restart policy after killing the shim, identity and MAC admission, and API crash recovery.Review focus
cmd/api/api/macos.goand its registration incmd/api/main.go.lib/instances/query.go(macOSShimGone).lib/instances/process_identity.go: when no live PID is recorded, it finds the shim by the control socket in its command line.Stack
Each PR is one commit on the one below it. macOS logic lives in its own files (
macos*.go,*_darwin.go,exec_lifecycle.go,port_forward.go); shared Linux files get small hooks such asif stored.MacOS != nil.