feat(minvmd): extend libkrun support to Linux/KVM - #414

.github/workflows/ci.yml (1)

261-263: ⚡ Quick win

Avoid one-off checkout hardening in a single step.

persist-credentials: false is introduced only here; keep this policy consistent repo-wide (either adopt it everywhere or omit it in this one job).

Based on learnings, this repository prefers not introducing persist-credentials: false on only one actions/checkout usage; apply consistently across workflows if adopted.

🤖 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.yml around lines 261 - 263, Remove the
persist-credentials: false configuration from the actions/checkout@v6 step in
this workflow. This parameter is only used in this single location across the
repository, and the repo prefers consistent policies applied either everywhere
or nowhere. Since this is an isolated one-off usage, remove this line to
maintain consistency with the rest of the codebase.

Source: Learnings

Fix all unresolved CodeRabbit comments on this PR:

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

.github/workflows/ci.yml (1)

261-263: ⚡ Quick win

Keep checkout credential policy consistent across workflows.

This introduces a one-off persist-credentials: false policy in a single checkout step. Either remove it here or roll it out consistently as a repo-wide workflow hardening change to avoid policy drift.

Based on learnings, this repository prefers avoiding single-step checkout hardening unless applied consistently across all workflow checkout steps.

🤖 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.yml around lines 261 - 263, The checkout step using
actions/checkout@v6 in the ci.yml workflow file includes a persist-credentials:
false configuration that is not applied consistently across other checkout steps
in the repository's workflows. Either remove the persist-credentials: false
parameter from this checkout step to maintain the existing checkout policy, or
apply this configuration consistently across all checkout steps throughout all
workflow files as a repository-wide hardening change. Based on the repository's
preference for consistency, removing this single-step configuration is the
recommended approach.

Source: Learnings

Fix all unresolved CodeRabbit comments on this PR:

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

.github/workflows/ci-linux-kvm.yml (1)

106-113: ⚡ Quick win

Include the build script in the libkrun cache key to prevent stale reuse.

The cache key only tracks pinned version strings, so changes in scripts/build-libkrun.sh can incorrectly hit old artifacts and skip the intended rebuild.

Suggested diff

       - name: Cache libkrun (libkrunfw v5.5.0 + libkrun v1.19.0)
         # Keyed on the pinned versions: the kernel compile only runs on a cache
         # miss. Bump the suffix when scripts/build-libkrun.sh changes pins.
         uses: actions/cache@v5
         with:
           path: ${{ env.KRUN_PREFIX }}
-          key: x86_64-libkrun-fw5.5.0-krun1.19.0-blk
+          key: x86_64-libkrun-fw5.5.0-krun1.19.0-${{ hashFiles('scripts/build-libkrun.sh') }}

🤖 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-linux-kvm.yml around lines 106 - 113, The cache key for
the libkrun cache action named "Cache libkrun" only includes pinned version
strings and does not reference the build script, so changes to
scripts/build-libkrun.sh won't invalidate the cache and will cause stale
artifacts to be reused. Update the key field in the cache action to include a
hash of the build script using GitHub's hashFiles function
(hashFiles('scripts/build-libkrun.sh')) so that any modifications to the script
will automatically change the cache key and force a rebuild instead of reusing
old cached artifacts.

.github/workflows/ci-linux-kvm.yml (1)

59-62: ⚡ Quick win

Align actions/checkout credential setting with repo-wide convention.

Line [61] sets persist-credentials: false in this workflow only. In this repo, that hardening should be applied consistently across all workflows (or not at all) to avoid policy drift.

Based on learnings, “don’t harden individual actions/checkout steps by adding persist-credentials: false only in one workflow/step; if needed, apply it repo-wide.”

🤖 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-linux-kvm.yml around lines 59 - 62, The
`persist-credentials: false` setting in the `actions/checkout@v6` action in this
workflow is inconsistently applied compared to the rest of the repository.
Review all other GitHub workflow files in the `.github/workflows/` directory to
determine the repo-wide convention for this security hardening setting. If other
workflows do not include this setting, remove it from the `actions/checkout`
step in this file to maintain consistency. If this is a desired security policy,
ensure it is applied uniformly across all workflows that use `actions/checkout`
to avoid policy drift.

Source: Learnings

Fix all unresolved CodeRabbit comments on this PR:

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

.github/workflows/ci-linux-kvm.yml (1)

59-62: ⚡ Quick win

Align actions/checkout credential setting with repo-wide convention.

Line [61] sets persist-credentials: false in this workflow only. In this repo, that hardening should be applied consistently across all workflows (or not at all) to avoid policy drift.

Based on learnings, “don’t harden individual actions/checkout steps by adding persist-credentials: false only in one workflow/step; if needed, apply it repo-wide.”

🤖 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-linux-kvm.yml around lines 59 - 62, The
`persist-credentials: false` setting in the `actions/checkout@v6` action in this
workflow is inconsistently applied compared to the rest of the repository.
Review all other GitHub workflow files in the `.github/workflows/` directory to
determine the repo-wide convention for this security hardening setting. If other
workflows do not include this setting, remove it from the `actions/checkout`
step in this file to maintain consistency. If this is a desired security policy,
ensure it is applied uniformly across all workflows that use `actions/checkout`
to avoid policy drift.

Source: Learnings

Fix all unresolved CodeRabbit comments on this PR:

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

scripts/bench-minvmd-boot.sh (2)

28-31: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Sync usage docs with the new required initramfs input.

Line 28 makes MINVMD_INITRAMFS mandatory, but the usage example still documents only two env vars, which causes avoidable first-run confusion.

Suggested patch

-#   MINVMD_KERNEL_PATH=... MINVMD_ROOTFS_PATH=... scripts/bench-minvmd-boot.sh [N] [minvmd-binary]
+#   MINVMD_KERNEL_PATH=... MINVMD_ROOTFS_PATH=... MINVMD_INITRAMFS=... \
+#     scripts/bench-minvmd-boot.sh [N] [minvmd-binary]

🤖 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/bench-minvmd-boot.sh` around lines 28 - 31, The usage documentation
or example in the script file does not reflect the new requirement for
MINVMD_INITRAMFS environment variable that was added in the validation block
starting at line 28. Update the usage example or documentation section at the
beginning of the script to include all three required environment variables:
MINVMD_KERNEL_PATH, MINVMD_ROOTFS_PATH, and MINVMD_INITRAMFS, so users
understand all three inputs are mandatory before running the script.

---

34-35: ⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify timeout command availability shape on target host
set -u
command -v timeout >/dev/null && echo "timeout found: $(command -v timeout)" || echo "timeout not found"
command -v gtimeout >/dev/null && echo "gtimeout found: $(command -v gtimeout)" || echo "gtimeout not found"

Repository: gominimal/minimal

Length of output: 98

---

🏁 Script executed:

cat -n scripts/bench-minvmd-boot.sh | head -80

Repository: gominimal/minimal

Length of output: 4439

---

Add gtimeout fallback to maintain macOS portability.

Line 34 and calls at lines 61 and 71 assume timeout exists. On macOS with Homebrew coreutils, the binary is gtimeout unless gnubin is on PATH, causing the script to fail despite GNU coreutils being installed.

Suggested patch

-command -v timeout >/dev/null || { echo "timeout required (install GNU coreutils)" >&2; exit 1; }
+if command -v timeout >/dev/null; then
+  TIMEOUT_BIN=timeout
+elif command -v gtimeout >/dev/null; then
+  TIMEOUT_BIN=gtimeout
+else
+  echo "timeout required (install GNU coreutils)" >&2
+  exit 1
+fi
@@
-if ! timeout "$BOOT_TIMEOUT" "$BIN" boot </dev/null >"$OUT" 2>&1 || ! grep -qx vm-up "$OUT"; then
+if ! "$TIMEOUT_BIN" "$BOOT_TIMEOUT" "$BIN" boot </dev/null >"$OUT" 2>&1 || ! grep -qx vm-up "$OUT"; then
@@
-  if timeout "$BOOT_TIMEOUT" "$BIN" boot </dev/null >"$OUT" 2>/dev/null && grep -qx vm-up "$OUT"; then
+  if "$TIMEOUT_BIN" "$BOOT_TIMEOUT" "$BIN" boot </dev/null >"$OUT" 2>/dev/null && grep -qx vm-up "$OUT"; then

Also applies to: lines 61–63, 71–72

🤖 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/bench-minvmd-boot.sh` around lines 34 - 35, The script checks only
for the `timeout` command, but on macOS with Homebrew coreutils the GNU
coreutils binary is named `gtimeout` unless the gnubin directory is on PATH.
Modify the command check at line 34 to test for both `timeout` and `gtimeout`,
storing the result in a variable. Then replace the hardcoded `timeout` calls at
lines 61 and 71 with this variable reference so the script uses whichever binary
is available on the system.