fix: restore ipv6_prefix to vm_state.config during snapshot run#560
Closed
claude-claude[bot] wants to merge 7 commits intorouted-ipv6-prefixfrom
Closed
fix: restore ipv6_prefix to vm_state.config during snapshot run#560claude-claude[bot] wants to merge 7 commits intorouted-ipv6-prefixfrom
claude-claude[bot] wants to merge 7 commits intorouted-ipv6-prefixfrom
Conversation
Add --ipv6-prefix flag for routed mode VM addressing. When set, VMs get addresses in the specified /64 prefix via NDP proxy, and MASQUERADE is skipped (the prefix is directly routable and covered by the machine cert's IP SANs, so VM source IPs pass IP binding checks). Without --ipv6-prefix, detect_host_ipv6() auto-detects from interfaces, skipping deprecated addresses. For hosts where all /64s are deprecated, --ipv6-prefix is required. Also: preflight_check is now an instance method (&self) so the ipv6_prefix configuration cannot be mismatched between preflight and setup.
- cargo fmt: line wrapping for .context() chains - clippy: collapse nested if in detect_host_ipv6() - Add missing ipv6_prefix field to RunArgs in test helpers Tested: make lint (fmt, clippy, audit, deny all pass)
Three issues found by code review: 1. ipv6_prefix not propagated through snapshot metadata — clones of routed-mode VMs with --ipv6-prefix would fail preflight (no auto-detect) or incorrectly add MASQUERADE. Added ipv6_prefix to VmConfig, SnapshotMetadata, and the snapshot restore path. 2. No validation of --ipv6-prefix input — invalid values like "foobar" produced nonsense addresses. Added validate_ipv6_prefix() that checks for 4 colon-separated hex groups. 3. Bare .await on output_connected_rx could hang forever if Firecracker crashes before fc-agent connects. Replaced with tokio::select! loop that polls vm_manager.try_wait() every 5s as a liveness check. Tested: make lint (fmt, clippy, audit, deny all pass)
Keep ipv6_prefix as a builder method (with_ipv6_prefix), consistent with loopback_ip (with_loopback_ip). Both are optional config set after construction — loopback_ip because it's allocated async after preflight_check, ipv6_prefix because it's genuinely optional. The real compile-time safety comes from storing ipv6_prefix in SnapshotMetadata, so both call sites (podman run + snapshot run) have it available and use symmetric code. Tested: cargo fmt --check + clippy pass
- README: ip6tables not required with --ipv6-prefix, add to CLI Reference - DESIGN: add ipv6_prefix field to RoutedNetwork, note conditional checks - CLAUDE.md: note MASQUERADE skipped with --ipv6-prefix
- 10 tests in routed.rs: prefix validation (valid/invalid/hex/full-addr), VM IPv6 generation (deterministic/format), parse_host_ipv6 (deprecated address filtering, link-local/ULA skipping, prefix extraction) - 2 tests in snapshot.rs: ipv6_prefix roundtrip through SnapshotMetadata serde, backward compatibility with old snapshots missing the field - 3 tests in test_state_manager.rs: load_state_by_pid found/not-found, stale state cleanup on retry (verifies dead PID files are removed) Extracted parse_host_ipv6(output, check_onlink) from detect_host_ipv6() for testability without shelling out to `ip addr`.
The snapshot restore path passed ipv6_prefix to RoutedNetwork for networking setup but didn't write it back to vm_state.config. This meant cascaded snapshots (snapshot of a clone) would lose the prefix, and the clone's state file showed ipv6_prefix: null. Also fix README CLI reference: --ipv6-prefix takes a PREFIX, not CIDR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
ejc3
force-pushed
the
routed-ipv6-prefix
branch
2 times, most recently
from
ca2ef8c to
8feec2e
Compare
Owner
|
Closing: will incorporate ipv6_prefix fix directly into routed-ipv6-prefix branch |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auto-Fix for PR #559
Issues Fixed
ipv6_prefixnot restored tovm_state.configduringsnapshot run: The snapshot restore path passedipv6_prefixtoRoutedNetwork::with_ipv6_prefix()for networking setup, but never wrote it back tovm_state.config. This meant cascaded snapshots (creating a snapshot from a clone) would lose the prefix viabuild_snapshot_config(), and the clone's state file showedipv6_prefix: null.<CIDR>instead of<PREFIX>: The--ipv6-prefixflag takes 4 colon-separated hex groups (e.g.,2803:6084:7058:46f6), not CIDR notation.Changes
src/commands/snapshot.rs: Addvm_state.config.ipv6_prefix = snapshot_config.metadata.ipv6_prefix.clone()alongside the other config restorations in the snapshot run path.README.md: Change--ipv6-prefix <CIDR>to--ipv6-prefix <PREFIX>.Generated by Claude | Review Run