tests/iface_options: add pp64 compatibility for command line test#6880
tests/iface_options: add pp64 compatibility for command line test#6880aniket-sahu-ibmx wants to merge 1 commit into
Conversation
The command line used for the test explicitly states "qemu-kvm", which is not used for ppc64le systems. Hence, this patch adds qemu-system-ppc64 to the command line after checking host architecture.
WalkthroughThe change makes the QEMU process detection in Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libvirt/tests/src/virtual_network/iface_options.py (1)
287-287: 💤 Low valueArchitecture detection works correctly for ppc64le; consider adding ppc64 (big-endian) support.
The change correctly handles ppc64le by using
qemu-system-ppc64instead ofqemu-kvm. The condition and binary name align with how libvirt launches QEMU on ppc64 systems. Based on learnings from relevant code snippets showing qemu-system-ppc64 is the standard binary for ppc64 architecture.However, the condition only checks for
'ppc64le'(little-endian). Big-endian ppc64 systems whereplatform.machine()returns'ppc64'would still incorrectly search forqemu-kvminstead ofqemu-system-ppc64.♻️ Optional enhancement to support both ppc64 variants
- cmd = ("ps -ef | grep %s | grep %s | grep -v grep " % (vm_name, "qemu-system-ppc64" if host_arch == 'ppc64le' else "qemu-kvm")) + cmd = ("ps -ef | grep %s | grep %s | grep -v grep " % (vm_name, "qemu-system-ppc64" if host_arch in ['ppc64le', 'ppc64'] else "qemu-kvm"))🤖 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 `@libvirt/tests/src/virtual_network/iface_options.py` at line 287, The cmd assembly in iface_options.py currently treats only host_arch == 'ppc64le' as using qemu-system-ppc64; update the condition used when building cmd (the tuple assigned to cmd that references vm_name and host_arch) to treat both 'ppc64' and 'ppc64le' as using "qemu-system-ppc64" (for example check host_arch in ('ppc64','ppc64le') or host_arch.startswith('ppc64')) so big-endian ppc64 hosts also search for qemu-system-ppc64 instead of qemu-kvm.
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@libvirt/tests/src/virtual_network/iface_options.py`:
- Line 287: The cmd assembly in iface_options.py currently treats only host_arch
== 'ppc64le' as using qemu-system-ppc64; update the condition used when building
cmd (the tuple assigned to cmd that references vm_name and host_arch) to treat
both 'ppc64' and 'ppc64le' as using "qemu-system-ppc64" (for example check
host_arch in ('ppc64','ppc64le') or host_arch.startswith('ppc64')) so big-endian
ppc64 hosts also search for qemu-system-ppc64 instead of qemu-kvm.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d20a7f0f-f03a-446c-ab01-315753ad865b
📒 Files selected for processing (1)
libvirt/tests/src/virtual_network/iface_options.py
The command line used for the test explicitly states "qemu-kvm", which is not used for ppc64le systems. Hence, this patch adds qemu-system-ppc64 to the command line after checking host architecture.
Summary by CodeRabbit