Skip to content

Conversation

@smitterl
Copy link
Contributor

@smitterl smitterl commented Nov 21, 2025

On s390x the 'lscss' command should be used to identify the memballoon through its device type; ccw bus is the default pci currently not available in RHEL.

Summary by CodeRabbit

  • Tests
    • Extended memory balloon testing to support s390-virtio architecture variant
    • Enhanced device verification testing with parameterized guest command capabilities for improved test flexibility and configuration options

✏️ Tip: You can customize this high-level summary in your review settings.

On s390x the 'lscss' command should be used to identify
the memballoon through its device type; ccw bus is the default pci
currently not available in RHEL.

Signed-off-by: Sebastian Mitterle <[email protected]>
@smitterl
Copy link
Contributor Author

JOB ID     : 99a72f0e5a5b6b026292936ff451d28838219117
JOB LOG    : /var/log/avocado/job-results/job-2025-11-21T06.00-99a72f0/job.log
 (1/1) type_specific.io-github-autotest-libvirt.memory.balloon.various.virtio: STARTED
 (1/1) type_specific.io-github-autotest-libvirt.memory.balloon.various.virtio: PASS (37.11 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /var/log/avocado/job-results/job-2025-11-21T06.00-99a72f0/results.html
JOB TIME   : 38.52 s

@coderabbitai
Copy link

coderabbitai bot commented Nov 21, 2025

Walkthrough

This change introduces a parameterizable guest command execution mechanism in the memory balloon test framework. A new guest_cmd parameter is added to the check_device_on_guest function with a default value of "lspci -vvv", replacing the previously hardcoded command. The configuration file is extended to support the s390-virtio architecture variant by specifying guest_cmd = 'lscss'. The parameter is propagated through the function call chain, including teardown operations, enabling architecture-specific command customization without modifying core test logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • The changes follow a consistent parameterization pattern across both configuration and code files
  • Function parameter addition is straightforward with sensible defaults
  • Configuration addition is minimal (single variant entry)
  • Primary concern: Verify default value "lspci -vvv" aligns with existing test behavior and that "lscss" is the correct command for s390-virtio architecture

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding device detection support for s390x by using the appropriate command for that architecture.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_var_mem_ball

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
libvirt/tests/src/memory/memory_balloon/various_mem_balloons.py (2)

34-42: Update docstring to document the new guest_cmd parameter.

The function signature correctly adds the guest_cmd parameter with a sensible default, but the docstring doesn't document it. For completeness, add documentation for this parameter.

Example addition to the docstring:

     """
     Check if memory balloon device exists on guest.
 
     :param guest_session (optional): Guest session object.
     :param close_session (bool, optional): Close the guest session after running command. Defaults to False.
+    :param guest_cmd (str, optional): Command to run on guest to detect devices. Defaults to "lspci -vvv".
 
     :returns guest_session: Updated guest session object.
     """

57-57: Consider making the error message generic to reflect the actual command used.

The error message hardcodes "guest lspci command" but the command is now parameterized and could be lscss on s390x. Consider updating the message to reference the actual command or use generic wording.

Apply this diff to use the actual command in the error message:

-            test.fail(f"Expected string {memballoon_device_str} was not returned from guest lspci command. Returned: {stdout}")
+            test.fail(f"Expected string {memballoon_device_str} was not returned from guest command '{guest_cmd}'. Returned: {stdout}")
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 875c970 and 6ea7de5.

📒 Files selected for processing (2)
  • libvirt/tests/cfg/memory/memory_balloon/various_mem_balloons.cfg (1 hunks)
  • libvirt/tests/src/memory/memory_balloon/various_mem_balloons.py (3 hunks)
⏰ 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.8
  • GitHub Check: Python 3.12
  • GitHub Check: Python 3.11
  • GitHub Check: Python 3.9
🔇 Additional comments (3)
libvirt/tests/cfg/memory/memory_balloon/various_mem_balloons.cfg (1)

16-17: LGTM! Configuration correctly sets s390x-specific command.

The addition of the s390-virtio variant with guest_cmd = 'lscss' properly configures the test to use the appropriate device detection command for s390x architecture. This works in conjunction with the nested s390-virtio variant at lines 22-24 and aligns with the code changes in the test script.

libvirt/tests/src/memory/memory_balloon/various_mem_balloons.py (2)

227-227: LGTM! Parameter correctly passed to function.

The guest_cmd parameter is properly propagated from the test configuration to the check_device_on_guest function call.


271-271: LGTM! Parameter retrieval is correct.

The guest_cmd parameter is correctly retrieved from test parameters with a sensible default that matches the function signature default. This will properly pick up the 'lscss' value configured for s390-virtio systems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants