unset QEMU_CPU for aml_image_packer

[QwaSeeK]

Description

Fix JetHub burn image build on aarch64 hosts. The extension uses Amlogic tool "aml_image_v2_packer_new" which is an i386 binary, on aarch64 it runs via binfmt_misc/qemu-i386, which inherits QEMU_CPU=cortex-a53 from arm64.conf and fails because cortex-a53 is not a valid x86 CPU model. The uses fix env -u QEMU_CPU to unset it only for the packer process.

Note: run_host_x86_binary_logged added in d8e24d7 already does env -u QEMU_CPU, but it only supports x86_64 binaries not i386.

How Has This Been Tested?

Build works on aarch64 and x86 platforms

Summary by CodeRabbit

• Bug Fixes
  • Fixed an issue with the image burn packaging process affecting build reliability.

[coderabbitai]

📝 Walkthrough

Walkthrough

The shell script extensions/jethub-burn.sh is modified to unset the QEMU_CPU environment variable during the PACKER invocation for burn image packaging. The change prefixes the PACKER command with env -u QEMU_CPU, maintaining all other existing logic and error handling.

Changes

Cohort / File(s)	Summary
Environment Variable Configuration extensions/jethub-burn.sh	Modified PACKER invocation to unset the QEMU_CPU environment variable using env -u QEMU_CPU prefix during burn image packaging step.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A whisker of code, so precise and so small,
Unsetting QEMU_CPU to help burn them all,
One line changed, yet powerful still,
The rabbits hop forward with technical skill! 🔥

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main functional change: unsetting QEMU_CPU specifically for the aml_image_packer binary to fix build failures on aarch64 hosts.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

extensions/jethub-burn.sh (1)

103-103: Fix correctly addresses the root cause with minimal scope.

env -u QEMU_CPU cleanly isolates the environment variable change to the packer subprocess, preventing ARM64-specific CPU models set in config from interfering with x86 binary execution. The fix is safe on native x86 hosts where QEMU_CPU is typically absent, and mirrors the established pattern in run_host_x86_binary_logged. Error handling is preserved, and no other packer invocations require the same treatment.

Optional: Consider a run_host_i386_binary_logged helper in lib/functions/logging/runners.sh (analogous to the x86-64 version) for future consistency if additional i386 host tools are added.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@extensions/jethub-burn.sh` at line 103, The packer subprocess can fail on
hosts that export QEMU_CPU; update the invocation that runs PACKER to run it
with QEMU_CPU removed (use env -u QEMU_CPU "$PACKER" -r "$tmpdir/image.cfg"
"$tmpdir" "$OUT_IMG") so the variable change is scoped to the subprocess,
preserve the existing exit_with_error handling, and mirror the established
pattern used by run_host_x86_binary_logged; optionally consider adding a
run_host_i386_binary_logged wrapper in lib/functions/logging/runners.sh for
future i386 host tools to centralize this behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@extensions/jethub-burn.sh`:
- Line 103: The packer subprocess can fail on hosts that export QEMU_CPU; update
the invocation that runs PACKER to run it with QEMU_CPU removed (use env -u
QEMU_CPU "$PACKER" -r "$tmpdir/image.cfg" "$tmpdir" "$OUT_IMG") so the variable
change is scoped to the subprocess, preserve the existing exit_with_error
handling, and mirror the established pattern used by run_host_x86_binary_logged;
optionally consider adding a run_host_i386_binary_logged wrapper in
lib/functions/logging/runners.sh for future i386 host tools to centralize this
behavior.

[rpardini]

Hmm. I guess i386 needs a runner.sh helper just like x86 has.

[QwaSeeK]

Hmm. I guess i386 needs a runner.sh helper just like x86 has.

I can do that instead of this lazy fix lol

[tabrisnet]

Hmm. I guess i386 needs a runner.sh helper just like x86 has.

sure, but can we try to keep some code-reuse too? that is, the only difference is in which qemu binary gets called...
or even... can we do a file [or readelf -h |grep Machine:] call to see which architecture it is?
caveat... we pass in qemu_invocation which doesn't exactly say what positional argument is the binary...

@coderabbitai

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the current implementation of run_host_x86_binary_logged
cat lib/functions/logging/runners.sh

Length of output: 12763

---

🏁 Script executed:

#!/bin/bash
# Read the extension file to see how it's currently being used
cat extensions/jethub-burn.sh

Length of output: 6031

---

🏁 Script executed:

#!/bin/bash
# Search for other places that might be running x86 binaries in the codebase
rg -n -C3 'qemu.*i386|i386.*binary' --type sh

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Check what x86-related qemu binaries might be available in typical systems
# and look at how the current function determines architecture
echo "=== Checking for i386 qemu binary patterns in the codebase ==="
rg -n 'qemu-i386' --type sh

echo ""
echo "=== Looking for any x86 or i386 library paths ==="
rg -n 'i386-linux-gnu|i686-linux-gnu' --type sh

Length of output: 113

---

🏁 Script executed:

#!/bin/bash
# Let's examine what 'file' command outputs for different architectures
# This will help understand how to parse the output for auto-detection
echo "Example file output formats for different architectures:"
echo ""
echo "For x86_64 binary:"
echo "ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked"
echo ""
echo "For i386/i686 binary:"
echo "ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV), dynamically linked"
echo ""
echo "For ARM64 binary:"
echo "ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), dynamically linked"

Length of output: 344

---

🏁 Script executed:

#!/bin/bash
# Check if there are other uses of qemu or architecture-specific handling
rg -n -C2 'qemu-.*-static|QEMU_CPU' --type sh | head -100

Length of output: 3636

---

@tabrisnet Great points about code reuse and auto-detection! The good news is that the existing run_host_x86_binary_logged function already solves the caveat you mentioned—it receives the binary path as $1 and already uses file to detect the architecture.

Here's my analysis:

Current implementation already detects architecture:

target_bin_arch="$(file -b "$1" | cut -d "," -f 1,2 | xargs echo -n)"

file command output patterns:

• x86_64: "ELF 64-bit LSB executable, x86-64"
• i386: "ELF 32-bit LSB executable, Intel 80386"

Proposed extension approach:

You could extend run_host_x86_binary_logged to auto-detect and handle both architectures:

function run_host_x86_binary_logged() {
    local -a qemu_invocation target_bin_arch
    target_bin_arch="unknown"
    
    if [[ -f /usr/bin/file ]]; then
        target_bin_arch="$(file -b "$1" | cut -d "," -f 1,2 | xargs echo -n)"
    fi
    
    qemu_invocation=("$@")  # Default: no qemu
    
    if [[ "$(uname -m)" != "x86_64" ]]; then
        # Detect if i386 or x86_64 from file output
        if [[ "${target_bin_arch}" =~ "Intel 80386" ]] || [[ "${target_bin_arch}" =~ "80386" ]]; then
            # i386 binary
            if [[ -f /usr/bin/qemu-i386-static ]]; then
                display_alert "Using qemu-i386-static for running on $(uname -m)" "$1 (${target_bin_arch})" "debug"
                qemu_invocation=("/usr/bin/qemu-i386-static" "-L" "/usr/i386-linux-gnu" "$@")
            elif [[ -f /usr/bin/qemu-i386 ]]; then
                display_alert "Using qemu-i386 (non-static) for running on $(uname -m)" "$1 (${target_bin_arch})" "debug"
                qemu_invocation=("/usr/bin/qemu-i386" "-L" "/usr/i386-linux-gnu" "$@")
            else
                exit_with_error "Can't find qemu-i386 for running '$1' on $(uname -m)"
            fi
        elif [[ "${target_bin_arch}" =~ "x86-64" ]] || [[ "${target_bin_arch}" =~ "x86_64" ]]; then
            # x86_64 binary (existing logic)
            if [[ -f /usr/bin/qemu-x86_64-static ]]; then
                # ... existing x86_64 logic
            fi
        else
            exit_with_error "Unsupported x86 architecture: ${target_bin_arch}"
        fi
    fi
    
    run_host_command_logged env -u QEMU_CPU "${qemu_invocation[@]}"
}

Benefits:

• ✅ Code reuse—extends existing function
• ✅ Auto-detection via file command (already implemented)
• ✅ Same env -u QEMU_CPU pattern
• ✅ Consistent error handling
• ✅ Same logging approach

Considerations:

• The i386 library path might be /usr/i386-linux-gnu or /usr/i686-linux-gnu depending on the system
• The qemu-user-static package provides both qemu-x86_64-static and qemu-i386-static binaries
• You might want to verify the correct library path on actual systems

Would you like me to help prepare a more complete implementation that QwaSeeK could use instead of the current inline fix?

---

🧠 Learnings used

Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2026-02-11T19:39:49.473Z
Learning: In lib/functions/logging/runners.sh, the run_host_x86_binary_logged function unsets QEMU_CPU locally to prevent ARM-specific CPU models (like cortex-a53 set in config/sources/arm64.conf for ARM64 emulation performance) from incorrectly affecting x86-64 binary emulation when building on ARM64 or other non-x86-64 hosts.

---

Learnt from: tparys
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2026-02-07T18:01:37.820Z
Learning: In the Armbian build system's config/sources/arm64.conf, when QEMU_CPU is not explicitly set, it defaults to 'max', which exposes the maximum feature set supported by QEMU. Setting QEMU_CPU=cortex-a53 provides a ~5x performance improvement by using a simpler CPU model with fewer architectural features, reducing virtualization overhead in QEMU's TCG mode.

---

Learnt from: glneo
Repo: armbian/build PR: 8913
File: config/sources/families/include/k3_common.inc:57-60
Timestamp: 2025-11-11T20:56:20.303Z
Learning: In config/sources/families/include/k3_common.inc, the OP-TEE build command at line 59 should be updated in a future PR to explicitly set CROSS_COMPILE64=aarch64-linux-gnu- and CROSS_COMPILE32=arm-linux-gnueabihf- instead of relying on OP-TEE's internal defaults, for better clarity and maintainability. User glneo agreed to address this in a separate PR.

---

Learnt from: djurny
Repo: armbian/build PR: 8287
File: config/bootscripts/boot-sunxi.cmd:148-161
Timestamp: 2025-06-12T21:03:39.686Z
Learning: In Armbian U-Boot bootscripts, the `${cpu}` environment variable is always pre-set by U-Boot (or defaults to a valid value), so additional guards against it being unset are unnecessary.

[igorpecovnik]

If this is not going to be done differently - we must merge as otherwise CI is having problems.

[github-actions]

✅ This PR has been reviewed and approved — all set for merge!

[adeepn]

If this is not going to be done differently - we must merge as otherwise CI is having problems.

Ok. I'l approved. We will research task with qemu-i386 invocation a bit later.