Actions to install latest QEMU support on GitHub runners - #31
RamakrishnanPK wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Direct shell interpolation, insecure KVM permissions, and incomplete cache identity create blocking security and reliability risks.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a reusable action to build, cache, install, and validate modern QEMU with Raspberry Pi 4 support.
Changes:
- Adds configurable QEMU source installation and caching.
- Adds documentation and an integration test workflow.
- Adds the test to the PR CI gate.
File summaries
| File | Description |
|---|---|
setup-qemu-latest/action.yml |
Implements the composite action. |
setup-qemu-latest/README.md |
Documents configuration and usage. |
.github/workflows/test-setup-qemu-latest.yml |
Tests installation, outputs, and KVM configuration. |
.github/workflows/_local_on_pr.yml |
Adds the test to PR validation. |
Review details
Suppressed comments (3)
setup-qemu-latest/action.yml:70
- Action inputs are interpolated directly into generated Bash source. A quoted value containing a quote, command separator, or newline can escape these assignments and execute arbitrary commands; the same pattern recurs for the dependency, download, build, cleanup, PATH, and validation steps. Pass every input through
env, validate structured values such as versions and package names, and consume argument lists via arrays rather than embedding${{ inputs.* }}in scripts.
install_prefix="${{ inputs.install-prefix }}/${{ inputs.qemu-version }}"
setup-qemu-latest/action.yml:49
- Defaulting
/dev/kvmto 0666 grants every local account access to the KVM kernel interface, and the udev rule persists after jobs on compatible self-hosted runners. Use the standard 0660root:kvmpolicy and grant only the runner account group membership, or make this persistent host modification explicitly opt-in.
default: "0666"
setup-qemu-latest/action.yml:72
- The cache can restore incompatible or stale binaries:
runner.osis identical for x64 and ARM64 runners, and the hash omits the hard-coded build recipe. This can produce an exec-format failure across architectures and lets later action revisions skip their updated build logic. Includerunner.archand a recipe hash (or explicitly maintained cache schema) in the key.
config_hash="$(printf '%s' "${{ inputs.qemu-targets }}|${{ inputs.configure-args }}|${{ inputs.extra-build-deps }}" | sha256sum | cut -d' ' -f1)"
cache_key="qemu-${{ runner.os }}-${{ inputs.qemu-version }}-${config_hash}"
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
d83d48f to
b102594
Compare
| && dpkg --compare-versions "$installed_version" ge "$QEMU_VERSION" \ | ||
| && { [[ "$ALLOW_DOWNGRADE" != "true" ]] || dpkg --compare-versions "$installed_version" eq "$QEMU_VERSION"; }; then |
There was a problem hiding this comment.
I am confused. Is the preinstalled version allowed to be higher than what is requested or do they have to match? It is one time ge and once eq
| - name: Enable KVM group permissions | ||
| shell: bash | ||
| env: | ||
| KVM_MODE: ${{ inputs.kvm-mode }} | ||
| run: | | ||
| if [[ ! "$KVM_MODE" =~ ^[0-7]{4}$ ]]; then | ||
| echo "Invalid kvm-mode value: '$KVM_MODE'. Expected a four-digit octal mode." >&2 | ||
| exit 1 | ||
| fi | ||
| echo "KERNEL==\"kvm\", GROUP=\"kvm\", MODE=\"$KVM_MODE\", OPTIONS+=\"static_node=kvm\"" | sudo tee /etc/udev/rules.d/99-kvm4all.rules | ||
| sudo udevadm control --reload-rules | ||
| sudo udevadm trigger --name-match=kvm | ||
| # Group membership changes don't apply to the current session, so chown | ||
| # directly to guarantee access without requiring a new login. | ||
| if [[ -e /dev/kvm ]]; then | ||
| sudo chown "$(id -u)":kvm /dev/kvm | ||
| fi |
There was a problem hiding this comment.
IMHO this is way more complicated than necessary. It is very unlikely that we will see a new login when running github actions
| - name: Enable KVM group permissions | |
| shell: bash | |
| env: | |
| KVM_MODE: ${{ inputs.kvm-mode }} | |
| run: | | |
| if [[ ! "$KVM_MODE" =~ ^[0-7]{4}$ ]]; then | |
| echo "Invalid kvm-mode value: '$KVM_MODE'. Expected a four-digit octal mode." >&2 | |
| exit 1 | |
| fi | |
| echo "KERNEL==\"kvm\", GROUP=\"kvm\", MODE=\"$KVM_MODE\", OPTIONS+=\"static_node=kvm\"" | sudo tee /etc/udev/rules.d/99-kvm4all.rules | |
| sudo udevadm control --reload-rules | |
| sudo udevadm trigger --name-match=kvm | |
| # Group membership changes don't apply to the current session, so chown | |
| # directly to guarantee access without requiring a new login. | |
| if [[ -e /dev/kvm ]]; then | |
| sudo chown "$(id -u)":kvm /dev/kvm | |
| fi | |
| - name: Enable KVM | |
| shell: bash | |
| run: | | |
| sudo chmod 666 /dev/kvm |
| qemu-targets: | ||
| description: Comma-separated list passed to QEMU's --target-list configure option. | ||
| required: false | ||
| default: "aarch64-softmmu" |
There was a problem hiding this comment.
Because this action removes all QEMU installations previously installed via apt, I prefer that it also builds the x86_64 variant. E.g. we currently use that for running tests using QNX.
Adds configurable QEMU source installation and caching.
Adds documentation and an integration test workflow.
Adds the test to the PR CI gate.
Fixes eclipse-score/score#1837