feat(minvmd,minimald): Stage 2 — minimald as pid-1 via initramfs - #368

crates/minimald/src/guest.rs (1)

82-94: 💤 Low value

Consider logging the initial mount error for diagnostics.

The function formats the disk on any mount failure, which is pragmatic for the controlled VM environment. However, logging the specific error from the first mount attempt would aid debugging when the failure isn't due to a blank disk (e.g., device missing, wrong device type).

💡 Optional improvement

 pub fn mount_state_disk(device: &str, target: &str) -> std::io::Result<()> {
     std::fs::create_dir_all(target)?;
-    if raw_mount_ext4(device, target).is_ok() {
+    if let Err(e) = raw_mount_ext4(device, target) {
+        tracing::info!(device, error = %e, "state disk mount failed; formatting ext4");
+        format_ext4(device)?;
+        raw_mount_ext4(device, target)?;
+        tracing::info!(device, target, "formatted + mounted state disk");
+    } else {
         tracing::info!(device, target, "mounted state disk");
-        return Ok(());
     }
-    // Likely blank on first boot — format ext4, then retry.
-    tracing::info!(device, "state disk mount failed; formatting ext4");
-    format_ext4(device)?;
-    raw_mount_ext4(device, target)?;
-    tracing::info!(device, target, "formatted + mounted state disk");
     Ok(())
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/minimald/src/guest.rs` around lines 82 - 94, In mount_state_disk,
capture the Result/error returned from the first raw_mount_ext4(device, target)
attempt and include that error in the log before formatting; specifically,
replace the current is_ok() check by matching or binding the Err to a variable
(from the first raw_mount_ext4 call) and pass that error into tracing::info/warn
so the message includes the actual mount failure (referencing mount_state_disk
and raw_mount_ext4), then proceed to format_ext4(device) and retry as before.

crates/minimald/src/main.rs (1)

297-303: 💤 Low value

Consider checking for specific errors when removing stale socket.

let _ = std::fs::remove_file(&guest_uds) ignores all errors, which could mask issues like permission denied or the path being a directory. While unlikely at early boot, checking for NotFound specifically is slightly safer.

💡 Optional improvement

-    let _ = std::fs::remove_file(&guest_uds);
+    if let Err(e) = std::fs::remove_file(&guest_uds) {
+        if e.kind() != std::io::ErrorKind::NotFound {
+            tracing::warn!(error = %e, "failed to remove stale guest UDS");
+        }
+    }

This same pattern appears at line 349 in run_guest.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/minimald/src/main.rs` around lines 297 - 303, The removal of the stale
UDS currently swallows all errors via let _ = std::fs::remove_file(&guest_uds),
which can hide real issues; update the code around guest_uds (the remove_file
call used before UnixListener::bind and the analogous call in run_guest) to
explicitly handle errors by matching the result: treat
std::io::ErrorKind::NotFound as OK (ignore) but propagate or return other errors
(e.g., wrap them with MainError::IO) so permission/invalid-path errors are not
silently ignored before calling UnixListener::bind or guest::spawn_vsock_relay.

scripts/build-initramfs.sh (1)

27-27: 💤 Low value

Consider removing unused staging directories.

The script creates dev, proc, sys, and newroot directories but only uses the staging root to place the init binary. These empty directories are included in the cpio archive but aren't referenced by the initramfs code.

🧹 Suggested simplification

 STAGE="$(mktemp -d)"
 trap 'rm -rf "$STAGE"' EXIT
-mkdir -p "$STAGE/dev" "$STAGE/proc" "$STAGE/sys" "$STAGE/newroot"
 cp "$BIN" "$STAGE/init"
 chmod +x "$STAGE/init"

If these directories are planned for future use (e.g., pre-creating mount points), consider adding a comment explaining their purpose.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/build-initramfs.sh` at line 27, Remove or document the unused staging
directories created by the mkdir call: if dev, proc, sys, and newroot under the
STAGE variable are not used by the initramfs (they only add empty entries to the
cpio archive), delete them from the mkdir invocation so only the necessary
staging root and any required directories (e.g., where the init binary is
placed) are created; alternatively, if you intend to keep them as pre-created
mount points, add a short comment next to the mkdir "$STAGE/..." line explaining
their purpose (reference: the mkdir invocation that creates "$STAGE/dev"
"$STAGE/proc" "$STAGE/sys" "$STAGE/newroot" and the STAGE variable).

.github/workflows/ci-macos.yml (1)

78-81: 💤 Low value

Consider pinning the cross version for reproducible CI builds.

The taiki-e/install-action will install the latest cross by default. If cross releases a breaking change, CI builds could become non-reproducible or fail unexpectedly.

📌 Suggested change to pin cross version

       - name: Install cross
         uses: taiki-e/install-action@v2
         with:
-          tool: cross
+          tool: cross@0.2.5  # or latest stable version

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci-macos.yml around lines 78 - 81, The GitHub Actions step
named "Install cross" currently uses taiki-e/install-action@v2 with only "tool:
cross", which installs the latest cross and risks non-reproducible CI; update
that step to pin the cross tool to a specific release by adding a version input
(e.g., add a "version: 'X.Y.Z'" field) to the taiki-e/install-action invocation
so the step installs a fixed cross version instead of the latest.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes