CPI: Add test case to check hypervisor bit of cpi info #6759
Conversation
WalkthroughAdds a new variant Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @libvirt/tests/src/guest_kernel_debugging/cpi_info.py:
- Around line 325-328: The code treats the return of process.run() as a string;
update usages where current_nested (and the other occurrences around lines
350-352) is assigned from process.run(...) to compare the command output string
instead by extracting the CmdResult.stdout (and .stdout.strip() to remove
newlines/whitespace) before comparing to "1" or other expected values; locate
the assignments to current_nested and the subsequent comparisons and replace
them to use current_nested.stdout.strip() (or assign s =
current_nested.stdout.strip() and compare s == "1") so the comparisons work
correctly.
- Around line 379-381: The error message in the conditional that checks
parsed['hypervisor_bit'] uses the wrong expected value; update the message in
the test.fail call inside the if block that evaluates "if
parsed['hypervisor_bit'] != 1" to state "should be 1" (include the actual
parsed['hypervisor_bit'] value as already done) so the text matches the
condition.
- Around line 302-314: test_nested_kvm_cpi opens a session via vm.wait_for_login
but never closes it, risking resource leaks; wrap the logic that uses the
returned session in a try/finally (or try/except/finally) and call
session.close() (or session.logout()/appropriate cleanup method used elsewhere)
in the finally block, mirroring the pattern from test_long_system_name to ensure
session is always cleaned up even on errors.
- Around line 377-378: Re-read the CPI system_level after booting the L3 guest
instead of reusing the earlier system_level value; call whatever method you used
previously to fetch the CPI/system_level (so you obtain an updated
system_level), then pass that fresh value into
checker._parse_system_level(system_level) to get the updated hypervisor_bit
result.
🧹 Nitpick comments (3)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (3)
339-339: Use && instead of ; for command chaining.The semicolon operator continues execution even if
rmmodfails. Use&&to ensurermmodsucceeds before attemptingmodprobe.♻️ Proposed fix
- "rmmod kvm ; modprobe kvm nested=1", timeout=30 + "rmmod kvm && modprobe kvm nested=1", timeout=30Apply the same fix to line 347:
- "rmmod kvm ; modprobe kvm nested=1", timeout=30 + "rmmod kvm && modprobe kvm nested=1", timeout=30Also applies to: 347-347
368-368: Consider using a public API for system_level parsing.The code uses
_parse_system_level(), which is a private method (indicated by the leading underscore). While this may be intentional for testing internal implementation details, consider whether a public API should be exposed byCPICheckerfor this purpose.
372-375: Add cleanup for the L3 guest process.Once the L3 guest boot issue (line 374-375) is fixed, ensure the qemu-kvm process is properly terminated at the end of the test or in the cleanup section to avoid leaving background processes running.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfglibvirt/tests/src/guest_kernel_debugging/cpi_info.py
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-12T10:00:09.383Z
Learnt from: yanglei-rh
Repo: autotest/tp-libvirt PR: 6675
File: libvirt/tests/cfg/virtual_network/qemu/nic_bonding.cfg:16-16
Timestamp: 2025-12-12T10:00:09.383Z
Learning: In tp-libvirt test config files (*.cfg under libvirt/tests/cfg), the cartesian config parser supports variable substitution using the ${variable_name} syntax. Variables defined in the cfg (e.g., filesize = 4000) are substituted into strings that reference them (e.g., count=${filesize}) before the Python test code receives the parameters. Treat this as a general behavior for these cfg files: define variables early, reference them with ${var}, and verify substitutions in tests to avoid hard-coded literals.
Applied to files:
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfg
📚 Learning: 2025-11-18T08:47:14.465Z
Learnt from: smitterl
Repo: autotest/tp-libvirt PR: 6072
File: libvirt/tests/src/passthrough/pci/libvirt_pci_passthrough_hotplug.py:239-241
Timestamp: 2025-11-18T08:47:14.465Z
Learning: In tp-libvirt tests, `utils_test.ping` is a valid function that exists in the avocado-vt framework (virttest.utils_test module). It is an alias to `utils_net.ping` and supports parameters including dest, count, interface, timeout, and session.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2026-01-07T16:04:00.667Z
Learnt from: yanglei-rh
Repo: autotest/tp-libvirt PR: 6754
File: libvirt/tests/cfg/virtual_network/qemu/hotplug_nic_with_operation.cfg:9-10
Timestamp: 2026-01-07T16:04:00.667Z
Learning: In tp-libvirt test configurations, parameters like `master_images_clone` and `remove_image_image1` are processed by the external avocado-vt/virttest framework's image preprocessing logic, not by test files within the tp-libvirt repository itself.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
🪛 Ruff (0.14.10)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
329-329: Do not catch blind exception: Exception
(BLE001)
356-356: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Python 3.9
- GitHub Check: Python 3.12
- GitHub Check: Python 3.11
- GitHub Check: Python 3.8
🔇 Additional comments (3)
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfg (1)
19-20: LGTM! Configuration follows established patterns.The new test variant is correctly configured and follows the same structure as existing test cases in this file.
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (2)
5-5: LGTM! Import is necessary for host command execution.The
avocado.utils.processmodule is correctly imported for executing commands on the L1 host in the nested KVM test.
400-401: LGTM! Test routing is correctly implemented.The new test case is properly wired into the test dispatcher following the established pattern.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @libvirt/tests/src/guest_kernel_debugging/cpi_info.py:
- Around line 387-389: The test currently checks parsed['hypervisor_bit'] != 1
but the failure message incorrectly says "L2 hypervisor_bit should be 0"; update
the test.fail call to reflect the expected value 1 (e.g., "L2 hypervisor_bit
should be 1, but actual is {parsed['hypervisor_bit']}") so the message matches
the condition in cpi_info.py where parsed['hypervisor_bit'] is evaluated.
- Line 316: After calling session = vm.wait_for_login(timeout=60) wrap the
subsequent test logic in a try/finally so the session is always cleaned up; in
the finally block call session.close() (or session.logout() if that is the
correct API in this codebase) to release resources, ensuring session is only
referenced if it was successfully created.
- Around line 327-358: The code assigns CmdResult objects from process.run() to
current_nested and then compares them to the string "1"; change assignments so
current_nested is the command output string by using the CmdResult's
stdout_text.strip() (e.g., set current_nested =
process.run(...).stdout_text.strip()), update the logging.info call that prints
Current nested status to log the extracted string, and likewise replace the
later verification assignment current_nested = process.run(... ) with
current_nested = process.run(...).stdout_text.strip() so the subsequent
comparison to "1" works as intended; leave session.cmd_output usages unchanged
if they already return strings.
🧹 Nitpick comments (1)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (1)
370-376: Consider exposing_parse_system_levelas a public method inCPIChecker.The test relies on the private method
_parse_system_level()(indicated by the_prefix) from the externalCPICheckerclass. Private methods are implementation details that may change without notice. Since the test needs to parse the hypervisor_bit from the system_level field, consider:
- Making
_parse_system_levela public method in theCPICheckerclass if it's intended for external use- Adding a public method like
get_hypervisor_bit(system_level)that returns the parsed bit directly
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfglibvirt/tests/src/guest_kernel_debugging/cpi_info.py
🚧 Files skipped from review as they are similar to previous changes (1)
- libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfg
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-24T08:01:27.899Z
Learnt from: hholoubk
Repo: autotest/tp-libvirt PR: 6579
File: libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py:95-97
Timestamp: 2025-09-24T08:01:27.899Z
Learning: In the libvirt test framework used in tp-libvirt, VM cleanup including destroying running VMs is handled by the teardown method (test_obj.teardown_iommu_test()) called in the finally block, so explicit VM destroy calls on timeout are not necessary according to the maintainers.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2025-11-03T13:30:16.458Z
Learnt from: smitterl
Repo: autotest/tp-libvirt PR: 6531
File: libvirt/tests/src/virtual_network/qemu/pktgen_burst_mode_test.py:64-67
Timestamp: 2025-11-03T13:30:16.458Z
Learning: In tp-libvirt tests, prefer using a list of cleanup lambda functions that are appended when actions requiring cleanup are performed, then executed in a finally block with individual error handling. Example: `cleanup_actions.append(lambda: undo_action())` followed by iterating through the list in reverse order in the finally block.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
🧬 Code graph analysis (1)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (1)
libvirt/tests/src/serial/serial_functional.py (1)
wait_for_login(235-258)
🪛 Ruff (0.14.10)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
333-333: Do not catch blind exception: Exception
(BLE001)
361-361: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Python 3.11
- GitHub Check: Python 3.12
- GitHub Check: Python 3.8
- GitHub Check: Python 3.9
🔇 Additional comments (1)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (1)
408-409: LGTM!The dispatcher entry correctly wires up the new test case following the existing pattern.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @libvirt/tests/src/guest_kernel_debugging/cpi_info.py:
- Around line 323-325: The guest command output from session.cmd_output is not
stripped before comparison, so change usages where output is compared to "1" to
strip whitespace first (e.g., replace current_nested = session.cmd_output(... )
with using current_nested.strip() when checking equality) and do the same for
the later guest-output checks around the other KVM/nested checks (the
comparisons at/near current_nested and the checks around lines 347-349); ensure
any variable derived from session.cmd_output is .strip()'d before comparing to
"1" to avoid trailing-newline mismatches.
- Line 3: The import of the time module is unused; either remove the unused
import statement or add the missing wait where the comment "wait L3 guest to
boot up" appears (after the call that starts the L3 guest, e.g., in the
function/method that performs the L3 boot such as start_l3_guest or the block
that invokes the L3 start). If adding the wait, insert a call to time.sleep(10)
(or an appropriate timeout) immediately after the L3 start call so the guest has
time to boot; otherwise simply delete the import line to avoid the unused
dependency.
- Line 316: The session returned by vm.wait_for_login() is not guaranteed to be
cleaned up if test.fail() is called; wrap the test logic that follows session =
vm.wait_for_login(...) in a try/finally and move the existing session cleanup
statements (the same cleanup currently at the bottom of the function that frees
the session) into the finally block so the session is always released even when
test.fail() or exceptions occur; keep using the existing cleanup calls (e.g.,
session.close()/session.logout() as present in the file) rather than duplicating
new cleanup logic.
- Around line 379-389: After launching the L3 guest with
session.cmd('/usr/libexec/qemu-kvm ...'), add an explicit wait before running
the CPI checks: ensure the code waits for the L3 guest to fully boot (e.g., poll
for a ready condition or sleep with a timeout) prior to calling
checker.get_cpi_field("system_level") and checker._parse_system_level(...); this
prevents the race where parsed['hypervisor_bit'] is checked too early and avoids
spurious test.fail(...) failures.
🧹 Nitpick comments (1)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (1)
372-372: Accessing private method_parse_system_level.The test accesses
checker._parse_system_level(), which is a private method (underscore prefix). This creates coupling to internal implementation details. Consider whetherCPICheckershould expose a public method for this, or document why direct access is necessary here.Also applies to: 386-386
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfglibvirt/tests/src/guest_kernel_debugging/cpi_info.py
🚧 Files skipped from review as they are similar to previous changes (1)
- libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfg
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-24T08:01:27.899Z
Learnt from: hholoubk
Repo: autotest/tp-libvirt PR: 6579
File: libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py:95-97
Timestamp: 2025-09-24T08:01:27.899Z
Learning: In the libvirt test framework used in tp-libvirt, VM cleanup including destroying running VMs is handled by the teardown method (test_obj.teardown_iommu_test()) called in the finally block, so explicit VM destroy calls on timeout are not necessary according to the maintainers.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
🪛 Ruff (0.14.10)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
333-333: Do not catch blind exception: Exception
(BLE001)
361-361: Do not catch blind exception: Exception
(BLE001)
379-379: Function call with shell=True parameter identified, security issue
(S604)
393-393: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Python 3.9
- GitHub Check: Python 3.12
- GitHub Check: Python 3.11
- GitHub Check: Python 3.8
🔇 Additional comments (1)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (1)
413-414: LGTM!The test case dispatcher integration follows the existing pattern correctly.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @libvirt/tests/src/guest_kernel_debugging/cpi_info.py:
- Around line 345-347: In session.cmd_output where we attempt to reload KVM (the
call containing "modprobe -r kvm ; modprobe -r kvm nested=1"), fix the command
so we first remove the module then load it with nested enabled by changing the
second invocation to "modprobe kvm nested=1" (i.e., replace the second "modprobe
-r" with "modprobe"); update the string passed to session.cmd_output accordingly
to ensure nested=1 is applied.
- Around line 315-318: Initialize the local variable session to None before the
try block (above the vm.wait_for_login() call) so that the finally block's check
(if session is not None) cannot raise a NameError if vm.wait_for_login() throws;
update the code around vm.wait_for_login(), session assignment, and the finally
block to rely on this pre-initialized session variable.
🧹 Nitpick comments (3)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (3)
396-401: Remove unnecessaryshell=Trueparameter.The
shell=Trueparameter is flagged as a security concern and is unnecessary here sincecmd_outputhandles command execution without requiring shell interpretation.♻️ Proposed fix
logging.info("Starting L3 guest") start_cmd = "/usr/libexec/qemu-kvm -machine s390-ccw-virtio -no-shutdown" try: - session.cmd_output(start_cmd, - shell=True, timeout=60) + session.cmd_output(start_cmd, timeout=60) except Exception as e: logging.error(f"Failed to start L3 guest: {e}") test.fail(f"Failed to start L3 guest: {e}")
403-405: Consider a more robust wait mechanism.Using a fixed
time.sleep(10)is fragile. Consider polling for the QEMU process or using a condition-based wait to ensure L3 is actually running before checking the hypervisor bit.
375-376: Accessing private method_parse_system_level.Using private methods (prefixed with
_) creates coupling to implementation details. If a public API exists or can be added toCPICheckerfor retrieving the parsedhypervisor_bit, prefer that approach.#!/bin/bash # Check if CPIChecker has a public method for getting hypervisor_bit ast-grep --pattern $'class CPIChecker { $$$ def get_hypervisor_bit($$$) { $$$ } $$$ }' # Also check what public methods are available rg -n "def [^_].*self" --type py -A 2 | grep -i "cpi"
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfglibvirt/tests/src/guest_kernel_debugging/cpi_info.py
🚧 Files skipped from review as they are similar to previous changes (1)
- libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfg
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: yanglei-rh
Repo: autotest/tp-libvirt PR: 6754
File: libvirt/tests/cfg/virtual_network/qemu/boot_with_multiqueue.cfg:19-21
Timestamp: 2026-01-10T16:53:10.863Z
Learning: In tp-libvirt test configurations for Windows guests, the avocado-vt/virttest framework automatically provides the `cdrom_virtio` parameter for accessing virtio-win ISO, even when not explicitly defined in the cfg file. The Python test code can reference `cdrom_virtio` without requiring it to be set via `cdroms += " virtio"` in the configuration.
📚 Learning: 2025-09-24T08:01:27.899Z
Learnt from: hholoubk
Repo: autotest/tp-libvirt PR: 6579
File: libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py:95-97
Timestamp: 2025-09-24T08:01:27.899Z
Learning: In the libvirt test framework used in tp-libvirt, VM cleanup including destroying running VMs is handled by the teardown method (test_obj.teardown_iommu_test()) called in the finally block, so explicit VM destroy calls on timeout are not necessary according to the maintainers.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2025-11-03T13:30:16.458Z
Learnt from: smitterl
Repo: autotest/tp-libvirt PR: 6531
File: libvirt/tests/src/virtual_network/qemu/pktgen_burst_mode_test.py:64-67
Timestamp: 2025-11-03T13:30:16.458Z
Learning: In tp-libvirt tests, prefer using a list of cleanup lambda functions that are appended when actions requiring cleanup are performed, then executed in a finally block with individual error handling. Example: `cleanup_actions.append(lambda: undo_action())` followed by iterating through the list in reverse order in the finally block.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2025-11-18T01:20:50.891Z
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6654
File: libvirt/tests/src/virtual_network/qemu/boot_with_multiqueue.py:56-58
Timestamp: 2025-11-18T01:20:50.891Z
Learning: In the tp-libvirt project, when calling session command methods and the status code is not needed, prefer using `session.cmd_output()` over `session.cmd_status_output()` to avoid unused variables.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
🧬 Code graph analysis (1)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (1)
libvirt/tests/src/serial/serial_functional.py (1)
wait_for_login(235-258)
🪛 Ruff (0.14.10)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
334-334: Do not catch blind exception: Exception
(BLE001)
364-364: Do not catch blind exception: Exception
(BLE001)
397-397: Function call with shell=True parameter identified, security issue
(S604)
399-399: Do not catch blind exception: Exception
(BLE001)
400-400: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
417-417: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Python 3.12
- GitHub Check: Python 3.9
- GitHub Check: Python 3.8
- GitHub Check: Python 3.11
🔇 Additional comments (2)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (2)
3-6: LGTM!Both imports are appropriately used in the new
test_nested_kvm_cpi()function.
437-438: LGTM!The dispatcher wiring follows the established pattern for test case dispatch.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @libvirt/tests/src/guest_kernel_debugging/cpi_info.py:
- Around line 385-392: The code uses session.cmd(...) which returns command
output as a string, so the comparisons like install_result != 0 are always true;
change the calls to use session.cmd_status(...) or
session.cmd_status_output(...) to obtain numeric exit codes (e.g.,
install_status = session.cmd_status('dnf install -y qemu-kvm*', timeout=180) and
check_status = session.cmd_status('which /usr/libexec/qemu-kvm')), then compare
install_status and check_status to 0 and adjust the branches accordingly (update
variables install_result and check_result usages in this block to the new
*_status names).
- Around line 394-404: The start sequence blocks because
session.cmd_output(start_cmd) waits for qemu-kvm to exit; change to launch qemu
in background (modify start_cmd to background it, e.g. append & and proper
stdout/stderr redirection or use nohup) and invoke it with session.sendline(...)
so it doesn't block; then replace the fixed time.sleep(10) with a short polling
loop that verifies the L3 guest is actually up (e.g., poll for the qemu process,
expected serial/console output or a reachable service/port using
session.cmd_output with a timeout and retry) and fail after a total timeout.
- Around line 344-347: The reload command on the L2 guest is removing kvm twice;
update the session.cmd_output call that currently runs "modprobe -r kvm ;
modprobe -r kvm nested=1" so the second invocation loads kvm with nesting
enabled by using "modprobe kvm nested=1" instead of "modprobe -r kvm nested=1"
(i.e., change the string passed to session.cmd_output in the Reload on L2 guest
block).
🧹 Nitpick comments (1)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (1)
398-400: Uselogging.exceptionfor better debugging.Per static analysis,
logging.exceptionincludes the traceback automatically, which aids debugging when failures occur.♻️ Suggested fix
except Exception as e: - logging.error(f"Failed to start L3 guest: {e}") + logging.exception("Failed to start L3 guest") test.fail(f"Failed to start L3 guest: {e}")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfglibvirt/tests/src/guest_kernel_debugging/cpi_info.py
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: yanglei-rh
Repo: autotest/tp-libvirt PR: 6754
File: libvirt/tests/cfg/virtual_network/qemu/boot_with_multiqueue.cfg:19-21
Timestamp: 2026-01-10T16:53:10.863Z
Learning: In tp-libvirt test configurations for Windows guests, the avocado-vt/virttest framework automatically provides the `cdrom_virtio` parameter for accessing virtio-win ISO, even when not explicitly defined in the cfg file. The Python test code can reference `cdrom_virtio` without requiring it to be set via `cdroms += " virtio"` in the configuration.
📚 Learning: 2025-09-24T08:01:27.899Z
Learnt from: hholoubk
Repo: autotest/tp-libvirt PR: 6579
File: libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py:95-97
Timestamp: 2025-09-24T08:01:27.899Z
Learning: In the libvirt test framework used in tp-libvirt, VM cleanup including destroying running VMs is handled by the teardown method (test_obj.teardown_iommu_test()) called in the finally block, so explicit VM destroy calls on timeout are not necessary according to the maintainers.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2025-11-03T13:30:16.458Z
Learnt from: smitterl
Repo: autotest/tp-libvirt PR: 6531
File: libvirt/tests/src/virtual_network/qemu/pktgen_burst_mode_test.py:64-67
Timestamp: 2025-11-03T13:30:16.458Z
Learning: In tp-libvirt tests, prefer using a list of cleanup lambda functions that are appended when actions requiring cleanup are performed, then executed in a finally block with individual error handling. Example: `cleanup_actions.append(lambda: undo_action())` followed by iterating through the list in reverse order in the finally block.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2025-11-18T01:20:50.891Z
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6654
File: libvirt/tests/src/virtual_network/qemu/boot_with_multiqueue.py:56-58
Timestamp: 2025-11-18T01:20:50.891Z
Learning: In the tp-libvirt project, when calling session command methods and the status code is not needed, prefer using `session.cmd_output()` over `session.cmd_status_output()` to avoid unused variables.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2025-12-12T10:00:09.383Z
Learnt from: yanglei-rh
Repo: autotest/tp-libvirt PR: 6675
File: libvirt/tests/cfg/virtual_network/qemu/nic_bonding.cfg:16-16
Timestamp: 2025-12-12T10:00:09.383Z
Learning: In tp-libvirt test config files (*.cfg under libvirt/tests/cfg), the cartesian config parser supports variable substitution using the ${variable_name} syntax. Variables defined in the cfg (e.g., filesize = 4000) are substituted into strings that reference them (e.g., count=${filesize}) before the Python test code receives the parameters. Treat this as a general behavior for these cfg files: define variables early, reference them with ${var}, and verify substitutions in tests to avoid hard-coded literals.
Applied to files:
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfg
📚 Learning: 2026-01-10T16:53:06.842Z
Learnt from: yanglei-rh
Repo: autotest/tp-libvirt PR: 6754
File: libvirt/tests/cfg/virtual_network/qemu/boot_with_multiqueue.cfg:19-21
Timestamp: 2026-01-10T16:53:06.842Z
Learning: In Windows guest test configurations under libvirt/tests, the avocado-vt/virttest framework auto-provides the cdrom_virtio parameter for accessing the virtio-win ISO. Test code can reference cdrom_virtio directly without requiring it to be added via cdroms += " virtio" in the cfg. This should apply to all relevant Windows guest test cfg files; avoid adding separate cdroms entries for this parameter. If a test relies on cdrom_virtio, assume its presence unless explicitly overridden.
Applied to files:
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfg
🧬 Code graph analysis (1)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (1)
libvirt/tests/src/serial/serial_functional.py (1)
wait_for_login(235-258)
🪛 Ruff (0.14.10)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
334-334: Do not catch blind exception: Exception
(BLE001)
364-364: Do not catch blind exception: Exception
(BLE001)
397-397: Function call with shell=True parameter identified, security issue
(S604)
398-398: Do not catch blind exception: Exception
(BLE001)
399-399: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
416-416: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Python 3.9
- GitHub Check: Python 3.12
- GitHub Check: Python 3.11
- GitHub Check: Python 3.8
🔇 Additional comments (5)
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfg (1)
19-20: LGTM!The new variant follows the established pattern and correctly maps to the
test_nested_kvm_cpifunction in the test implementation.libvirt/tests/src/guest_kernel_debugging/cpi_info.py (4)
3-6: LGTM!The imports are appropriate for the new nested KVM test functionality.
436-437: LGTM!The dispatch correctly routes to the new test function following the established pattern.
339-361: The code correctly uses the"1"format for checking the nested virtualization parameter on s390. According to s390 KVM documentation, the nested parameter on s390 uses numeric format (nested=0ornested=1), notY/N. The comparisons on lines 339 and 360 are appropriate for the s390 architecture this test targets.
374-378: Consider avoiding private method_parse_system_level.Accessing
checker._parse_system_level()couples this test to internal implementation details ofCPIChecker. If a public method exists for parsing system_level data, prefer using that instead.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @libvirt/tests/src/guest_kernel_debugging/cpi_info.py:
- Around line 399-401: In the except block that catches Exception as e (inside
the L3 guest start logic), replace logging.error(...) with
logging.exception(...) so the traceback is preserved in the logs; keep the
existing test.fail(...) call unchanged to surface the failure. Ensure you call
logging.exception with a similar descriptive message (e.g., "Failed to start L3
guest: %s") combined with the exception info so the stack trace is recorded.
- Around line 339-367: The current check only treats current_nested == "1" and
later verifies new_status == "1", which misses kernels that report "Y"; update
the logic around current_nested and new_status (used with symbols
current_nested, new_status, session.cmd_output, process.run, test.fail) to
accept both "1" and "Y" (e.g., compare against a set or normalize the value) so
you don't reload unnecessarily and don't fail when kernel returns "Y"; also make
the unload/load command consistent (use "modprobe -r kvm" in both the is_guest
and non-guest branches instead of mixing rmmod) and keep the same verification
flow and failure messages.
- Around line 315-318: Initialize session to None before calling
vm.wait_for_login() to avoid a NameError if login raises an exception; in the
block containing session = vm.wait_for_login(timeout=60) (same pattern used in
test_long_system_name), add session = None above the try and ensure the finally
block only calls session.close() or session.logout() if session is not None.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfglibvirt/tests/src/guest_kernel_debugging/cpi_info.py
🚧 Files skipped from review as they are similar to previous changes (1)
- libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfg
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: yanglei-rh
Repo: autotest/tp-libvirt PR: 6754
File: libvirt/tests/cfg/virtual_network/qemu/boot_with_multiqueue.cfg:19-21
Timestamp: 2026-01-10T16:53:10.863Z
Learning: In tp-libvirt test configurations for Windows guests, the avocado-vt/virttest framework automatically provides the `cdrom_virtio` parameter for accessing virtio-win ISO, even when not explicitly defined in the cfg file. The Python test code can reference `cdrom_virtio` without requiring it to be set via `cdroms += " virtio"` in the configuration.
📚 Learning: 2025-09-24T08:01:27.899Z
Learnt from: hholoubk
Repo: autotest/tp-libvirt PR: 6579
File: libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py:95-97
Timestamp: 2025-09-24T08:01:27.899Z
Learning: In the libvirt test framework used in tp-libvirt, VM cleanup including destroying running VMs is handled by the teardown method (test_obj.teardown_iommu_test()) called in the finally block, so explicit VM destroy calls on timeout are not necessary according to the maintainers.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2025-11-03T13:30:16.458Z
Learnt from: smitterl
Repo: autotest/tp-libvirt PR: 6531
File: libvirt/tests/src/virtual_network/qemu/pktgen_burst_mode_test.py:64-67
Timestamp: 2025-11-03T13:30:16.458Z
Learning: In tp-libvirt tests, prefer using a list of cleanup lambda functions that are appended when actions requiring cleanup are performed, then executed in a finally block with individual error handling. Example: `cleanup_actions.append(lambda: undo_action())` followed by iterating through the list in reverse order in the finally block.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2025-11-18T01:20:50.891Z
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6654
File: libvirt/tests/src/virtual_network/qemu/boot_with_multiqueue.py:56-58
Timestamp: 2025-11-18T01:20:50.891Z
Learning: In the tp-libvirt project, when calling session command methods and the status code is not needed, prefer using `session.cmd_output()` over `session.cmd_status_output()` to avoid unused variables.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
🧬 Code graph analysis (1)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (1)
libvirt/tests/src/serial/serial_functional.py (1)
wait_for_login(235-258)
🪛 Ruff (0.14.10)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
334-334: Do not catch blind exception: Exception
(BLE001)
364-364: Do not catch blind exception: Exception
(BLE001)
399-399: Do not catch blind exception: Exception
(BLE001)
400-400: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
421-421: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Python 3.11
- GitHub Check: Python 3.9
- GitHub Check: Python 3.8
- GitHub Check: Python 3.12
🔇 Additional comments (3)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (3)
3-6: LGTM!The new imports are appropriate for the functionality being added.
441-442: LGTM!The dispatcher wiring follows the established pattern in this file.
374-376: Consider whether a public API exists for parsing system_level.The test accesses
checker._parse_system_level()(underscore-prefixed) on lines 375 and 413. While private methods are typically fragile, this usage pattern is common in test utilities where underscore-prefixed methods may be intended for test consumption. Ifvirttest.utils_cpi.CPICheckerprovides a documented public method for parsing system_level, prefer that instead. Otherwise, verify with the maintainers of the CPIChecker utility whether this private method usage is an accepted pattern for its intended use in tests.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @libvirt/tests/src/guest_kernel_debugging/cpi_info.py:
- Around line 340-368: The host-path is attempting to forcibly unload/load kvm
using process.run("rmmod kvm ; modprobe kvm nested=1") which will fail if VMs
(including the L2 guest) are using KVM and is inconsistent with the guest path
that uses modprobe -r; modify the logic around is_guest to first read
current_nested and if not "1" attempt to enable nested only via modprobe (not
rmmod) or, if rmmod is required, detect that rmmod failed due to in-use modules
and then skip the test or call test.fail with a clear message; update references
to process.run, session.cmd_output, is_guest, current_nested, new_status and
test.fail accordingly so the host path avoids unconditional rmmod and provides a
clear fallback when modules are busy.
🧹 Nitpick comments (3)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (3)
395-423: Missing cleanup: L3 guest process is not terminated.The L3 qemu-kvm process started on L2 is never explicitly killed. When the session closes, the background process may remain running (orphaned) until the L2 VM is destroyed. Consider adding cleanup in the finally block.
♻️ Suggested fix to cleanup L3 guest
finally: if session is not None: try: + # Kill L3 guest if running + session.cmd_status("pkill -f qemu-kvm", timeout=30) session.close() except Exception as e: logging.warning(f"Failed to close session: {e}")
398-402: Redundant error logging beforetest.fail().The
logging.error()call is redundant sincetest.fail()immediately follows and will capture the failure. Consider simplifying by removing the logging line or usinglogging.exception()if traceback logging is desired.♻️ Simplified version
try: session.sendline(start_cmd) except Exception as e: - logging.error(f"Failed to start L3 guest: {e}") test.fail(f"Failed to start L3 guest: {e}")
384-393: Consider more specific package installation.The wildcard
qemu-kvm*may install unnecessary packages. Consider using justqemu-kvmor the specific packages needed. The fallback logic to check for existing installation is good.♻️ Suggested refinement
# install qemu-kvm on L2 logging.info("Installing qemu-kvm") - install_status = session.cmd_status('dnf install -y qemu-kvm*', + install_status = session.cmd_status('dnf install -y qemu-kvm', timeout=180)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfglibvirt/tests/src/guest_kernel_debugging/cpi_info.py
🚧 Files skipped from review as they are similar to previous changes (1)
- libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfg
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: yanglei-rh
Repo: autotest/tp-libvirt PR: 6754
File: libvirt/tests/cfg/virtual_network/qemu/boot_with_multiqueue.cfg:19-21
Timestamp: 2026-01-10T16:53:10.863Z
Learning: In tp-libvirt test configurations for Windows guests, the avocado-vt/virttest framework automatically provides the `cdrom_virtio` parameter for accessing virtio-win ISO, even when not explicitly defined in the cfg file. The Python test code can reference `cdrom_virtio` without requiring it to be set via `cdroms += " virtio"` in the configuration.
📚 Learning: 2025-09-24T08:01:27.899Z
Learnt from: hholoubk
Repo: autotest/tp-libvirt PR: 6579
File: libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py:95-97
Timestamp: 2025-09-24T08:01:27.899Z
Learning: In the libvirt test framework used in tp-libvirt, VM cleanup including destroying running VMs is handled by the teardown method (test_obj.teardown_iommu_test()) called in the finally block, so explicit VM destroy calls on timeout are not necessary according to the maintainers.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2025-11-03T13:30:16.458Z
Learnt from: smitterl
Repo: autotest/tp-libvirt PR: 6531
File: libvirt/tests/src/virtual_network/qemu/pktgen_burst_mode_test.py:64-67
Timestamp: 2025-11-03T13:30:16.458Z
Learning: In tp-libvirt tests, prefer using a list of cleanup lambda functions that are appended when actions requiring cleanup are performed, then executed in a finally block with individual error handling. Example: `cleanup_actions.append(lambda: undo_action())` followed by iterating through the list in reverse order in the finally block.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2025-11-18T01:20:50.891Z
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6654
File: libvirt/tests/src/virtual_network/qemu/boot_with_multiqueue.py:56-58
Timestamp: 2025-11-18T01:20:50.891Z
Learning: In the tp-libvirt project, when calling session command methods and the status code is not needed, prefer using `session.cmd_output()` over `session.cmd_status_output()` to avoid unused variables.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
🧬 Code graph analysis (1)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (1)
libvirt/tests/src/serial/serial_functional.py (1)
wait_for_login(235-258)
🪛 Ruff (0.14.10)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
335-335: Do not catch blind exception: Exception
(BLE001)
365-365: Do not catch blind exception: Exception
(BLE001)
400-400: Do not catch blind exception: Exception
(BLE001)
401-401: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
422-422: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Python 3.11
- GitHub Check: Python 3.8
- GitHub Check: Python 3.12
- GitHub Check: Python 3.9
🔇 Additional comments (3)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (3)
3-6: LGTM!The
timeandprocessimports are appropriately added and used in the newtest_nested_kvm_cpi()function.
442-443: LGTM!The dispatcher integration follows the existing pattern and correctly routes to the new test function.
374-379: Accessing private method_parse_system_level.The code accesses
checker._parse_system_level()which is a private method (prefixed with_). Verify whetherCPIChecker(from the externalvirttest.utils_cpilibrary) provides a public API for retrieving parsed hypervisor_bit. If available, prefer using the public method to avoid coupling to internal implementation details.
Check hypervisor bit of cpi info when there's nested guest boot up and it will be changed as expected Signed-off-by: bfu <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (4)
340-340: Consider handling "Y"/"N" nested parameter format.On some systems, the nested parameter may report
Y/Ninstead of1/0. The current check only handles the numeric format.Suggested fix
- if current_nested != "1": + if current_nested not in ("1", "Y"):
404-410: Replace hardcoded sleep with polling and add explicit L3 cleanup.The hardcoded
time.sleep(5)is fragile - the L3 guest may take longer to start on slower systems or start faster on others. Consider polling for the qemu-kvm process with a timeout instead.Additionally, the L3 qemu-kvm process is not explicitly cleaned up. While it will be terminated when the session closes, explicit cleanup improves reliability and test hygiene.
Suggested improvements
- # wait L3 guest to boot up - logging.info("waiting for L3 guest to boot") - time.sleep(5) - # Verify qemu-kvm process is running - qemu_status = session.cmd_status("pgrep -f qemu-kvm") - if qemu_status != 0: - test.fail("L3 guest (qemu-kvm) process not found") + # Wait for L3 guest to boot up with polling + logging.info("Waiting for L3 guest to boot") + def check_l3_running(): + return session.cmd_status("pgrep -f qemu-kvm") == 0 + if not utils_misc.wait_for(check_l3_running, timeout=30, step=1): + test.fail("L3 guest (qemu-kvm) process not found after 30s")For cleanup, add before the
finallyblock or in the finally:# Cleanup L3 guest session.cmd_status("pkill -f qemu-kvm", timeout=10)
400-402: Uselogging.exceptionto include traceback.When logging within an exception handler,
logging.exceptionautomatically includes the traceback, which aids debugging. Static analysis flagged this as well.Suggested fix
except Exception as e: - logging.error(f"Failed to start L3 guest: {e}") + logging.exception(f"Failed to start L3 guest: {e}") test.fail(f"Failed to start L3 guest: {e}")
376-379: Accessing private method_parse_system_level.The test accesses
checker._parse_system_level(), which is a private method (prefixed with underscore). While acceptable in test code, consider whetherCPICheckershould expose a public method for getting parsed system level data, or if this parsing logic should be part ofget_cpi_field().
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfglibvirt/tests/src/guest_kernel_debugging/cpi_info.py
🚧 Files skipped from review as they are similar to previous changes (1)
- libvirt/tests/cfg/guest_kernel_debugging/cpi_info.cfg
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-24T08:01:27.899Z
Learnt from: hholoubk
Repo: autotest/tp-libvirt PR: 6579
File: libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py:95-97
Timestamp: 2025-09-24T08:01:27.899Z
Learning: In the libvirt test framework used in tp-libvirt, VM cleanup including destroying running VMs is handled by the teardown method (test_obj.teardown_iommu_test()) called in the finally block, so explicit VM destroy calls on timeout are not necessary according to the maintainers.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2025-11-03T13:30:16.458Z
Learnt from: smitterl
Repo: autotest/tp-libvirt PR: 6531
File: libvirt/tests/src/virtual_network/qemu/pktgen_burst_mode_test.py:64-67
Timestamp: 2025-11-03T13:30:16.458Z
Learning: In tp-libvirt tests, prefer using a list of cleanup lambda functions that are appended when actions requiring cleanup are performed, then executed in a finally block with individual error handling. Example: `cleanup_actions.append(lambda: undo_action())` followed by iterating through the list in reverse order in the finally block.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
📚 Learning: 2025-11-18T01:20:50.891Z
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6654
File: libvirt/tests/src/virtual_network/qemu/boot_with_multiqueue.py:56-58
Timestamp: 2025-11-18T01:20:50.891Z
Learning: In the tp-libvirt project, when calling session command methods and the status code is not needed, prefer using `session.cmd_output()` over `session.cmd_status_output()` to avoid unused variables.
Applied to files:
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
🪛 Ruff (0.14.10)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py
335-335: Do not catch blind exception: Exception
(BLE001)
365-365: Do not catch blind exception: Exception
(BLE001)
400-400: Do not catch blind exception: Exception
(BLE001)
401-401: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
422-422: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Python 3.11
- GitHub Check: Python 3.12
- GitHub Check: Python 3.8
- GitHub Check: Python 3.9
🔇 Additional comments (2)
libvirt/tests/src/guest_kernel_debugging/cpi_info.py (2)
3-6: LGTM!The new imports are appropriate for the test requirements -
timefor the sleep delay andprocessfrom avocado.utils for executing commands on the L1 host.
442-443: LGTM!The dispatcher integration follows the established pattern and routes correctly to the new test function.
smitterl
left a comment
smitterl
left a comment
There was a problem hiding this comment.
@fbq815 Thank you for this. I think your code needs to be made more robust and can be simplified quite a lot. Please check my comments.
Also, in case you used AI to implement this, please refer to our AI policy: https://avocado-framework.readthedocs.io/en/latest/guides/contributor/chapters/ai_policy.html
|
|
||
| def test_nested_kvm_cpi(): | ||
| """ | ||
| Test case 4: Test CPI system_level behavior in nested KVM environment |
There was a problem hiding this comment.
test cases in avocado aren't ordered but can be executed in any order, please remove 4
| 3. L2 guest: enable nested and Boot L3 guest | ||
| 4. L2 guest: Check /sys/firmware/cpi/system_level and verify hypervisor_bit is '1' | ||
| """ | ||
| logging.info("=== Test Case 4: Nested KVM CPI ===") |
There was a problem hiding this comment.
test cases in avocado aren't ordered but can be executed in any order, please remove 4
| # Login L2 guest | ||
| session = vm.wait_for_login(timeout=60) | ||
|
|
||
| def check_and_enable_nested(is_guest=False): |
There was a problem hiding this comment.
In my environment this consistently failed on the host
[stdlog] raise exceptions.TestFail(msg)
[stdlog] avocado.core.exceptions.TestFail: Failed to reload kvm module: Command 'modprobe -r kvm ; modprobe kvm nested=1' failed.
[stdlog] stdout: b''
[stdlog] stderr: b'modprobe: FATAL: Module kvm is in use.\n'
However, when I use the function provided by avocado-vt https://github.com/autotest/tp-libvirt/blob/2f35e2b624b9144e8c9892ad0a2a7ec6e889ac76/libvirt/tests/cfg/libvirt_hugepage.cfg#L12C5-L12C26 it works well.
JOB ID : d510af7983b16278aaa7ba8b2bd45b56b4edec2c
JOB LOG : /var/log/avocado/job-results/job-2026-01-13T10.09-d510af7/job.log
(1/1) type_specific.io-github-autotest-libvirt.cpi_info.nested_kvm_cpi: STARTED
(1/1) type_specific.io-github-autotest-libvirt.cpi_info.nested_kvm_cpi: PASS (57.88 s)
RESULTS : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML : /var/log/avocado/job-results/job-2026-01-13T10.09-d510af7/results.html
JOB TIME : 59.01 s
My recommendation is to use that configuration
kvm_module_parameters = "nested=1"
which makes sure nested is enabled on the host when the test starts and when you load the module inside of L2, you load it with this function.
If loading the kvm module fails, there will be an error. Therefore I wonder if there's a reason to double check if nested is enabled? If not I think you can just omit that part to make the code easier.
All of this will make your code cleaner IMO.
|
|
||
| # install qemu-kvm on L2 | ||
| logging.info("Installing qemu-kvm") | ||
| install_status = session.cmd_status('dnf install -y qemu-kvm*', |
There was a problem hiding this comment.
We usually don't execute dnf here but use utils_package.package_install("qemu-kvm", session=session). This simplifies your code and avoids duplication.
2 participants
Check hypervisor bit of cpi info when there's nested guest boot up and it will be changed as expected
Signed-off-by: bfu [email protected]
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.