-
Notifications
You must be signed in to change notification settings - Fork 181
various_mem_ballongs: fix device detection on s390x #6669
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
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]>
|
WalkthroughThis change introduces a parameterizable guest command execution mechanism in the memory balloon test framework. A new Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this 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 newguest_cmdparameter.The function signature correctly adds the
guest_cmdparameter 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
lscsson 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
📒 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_cmdparameter is properly propagated from the test configuration to thecheck_device_on_guestfunction call.
271-271: LGTM! Parameter retrieval is correct.The
guest_cmdparameter 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.
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
✏️ Tip: You can customize this high-level summary in your review settings.