Add case to support heavy network traffic test after set adapterrss #6716
Conversation
WalkthroughAdds a new Libvirt test configuration Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (2)
libvirt/tests/cfg/virtual_network/qemu/netkvm_rss_test.cfg (1)
1-24: RSS queue/CPU settings may not match the intended 4‑vCPU topologyHere
smp = 4,queues_nic1 = ${smp}, butvcpu_cores = 1andset-NetAdapterRssuses${vcpu_cores}for both-NumberOfReceiveQueuesand-MaxProcessors. That effectively limits RSS to a single core/queue even though the VM/NIC are configured for 4.If the goal is to exercise multiple RSS queues, consider tying these parameters to
smp/queues_nic1instead of a hard‑coded 1, e.g.:- vcpu_cores = 1 + vcpu_cores = ${smp} @@ - set_adapterrss_cmd = 'powershell -command "Set-NetAdapterRss -Name \"%s\" -NumberOfReceiveQueues ${vcpu_cores} -MaxProcessors ${vcpu_cores} -Profile Conservative"' + set_adapterrss_cmd = 'powershell -command "Set-NetAdapterRss -Name \"%s\" -NumberOfReceiveQueues ${queues_nic1} -MaxProcessors ${smp} -Profile Conservative"'Please double‑check that this aligns with how you expect RSS to be distributed across vCPUs.
libvirt/tests/src/virtual_network/qemu/netkvm_rss_test.py (1)
58-59: Rename unused loop index to_to satisfy Ruff B007The loop index
iisn’t used inside the body; static analysis (B007) prefers an underscore for intentional unused vars. Simple tweak:- for i in range(10): + for _ in range(10):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/virtual_network/qemu/netkvm_rss_test.cfg(1 hunks)libvirt/tests/src/virtual_network/qemu/netkvm_rss_test.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6652
File: libvirt/tests/cfg/virtual_network/passt/passt_function.cfg:106-106
Timestamp: 2025-11-24T10:44:47.801Z
Learning: In libvirt/tests/cfg/virtual_network/passt/passt_function.cfg, passt test cases have only one interface, so running `dhcpcd` without specifying an interface is safe and doesn't cause network disruption.
📚 Learning: 2025-11-24T10:44:47.801Z
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6652
File: libvirt/tests/cfg/virtual_network/passt/passt_function.cfg:106-106
Timestamp: 2025-11-24T10:44:47.801Z
Learning: In libvirt/tests/cfg/virtual_network/passt/passt_function.cfg, passt test cases have only one interface, so running `dhcpcd` without specifying an interface is safe and doesn't cause network disruption.
Applied to files:
libvirt/tests/src/virtual_network/qemu/netkvm_rss_test.pylibvirt/tests/cfg/virtual_network/qemu/netkvm_rss_test.cfg
🧬 Code graph analysis (1)
libvirt/tests/src/virtual_network/qemu/netkvm_rss_test.py (1)
provider/guest_os_booting/guest_os_booting_base.py (1)
get_vm(18-56)
🪛 Ruff (0.14.8)
libvirt/tests/src/virtual_network/qemu/netkvm_rss_test.py
58-58: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
⏰ 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 (1)
libvirt/tests/src/virtual_network/qemu/netkvm_rss_test.py (1)
29-57: End‑to‑end VM/RSS/Speedtest flow looks consistentVM selection, serial login, Speedtest deployment, Windows NIC lookup (
get_windows_nic_attribute), and RSS configuration via the formattedset_adapterrss_cmdall line up with the cfg parameters; I don’t see functional issues in this part of the flow.
|
Hi @nanli1 @qiankehan Could you please help me review this patch? This is a widows only scenario, thanks a lot. |
qiankehan
left a comment
qiankehan
left a comment
There was a problem hiding this comment.
Only one tine update needed. The other LGTM
Automate RHEL-292670 heavy network traffic test after set adapterrss Signed-off-by: Lei Yang <[email protected]>
yanglei-rh
force-pushed
the
LIBVIRTAT-22206
branch
from
558f344 to
359b814
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
libvirt/tests/src/virtual_network/qemu/netkvm_rss_test.py (3)
19-27: Consider enhancing the command execution helper for better diagnostics.The helper function works correctly but could provide better diagnostics:
- It only returns output, not status, limiting caller flexibility
- Error messages don't include the command that failed
- Successful outputs aren't logged (only failures when omit=False)
Consider this enhancement:
- def execute_command(command, timeout=60, omit=False): + def execute_command(command, timeout=60, omit_failure=False): """ - Execute command and return the output + Execute command and return status and output + + :param command: Command to execute + :param timeout: Command timeout in seconds + :param omit_failure: If True, don't fail the test on non-zero exit + :return: Tuple of (status, output) """ test.log.info("Sending command: %s", command) status, output = session.cmd_status_output(command, timeout) - if status != 0 and omit is False: - test.error("execute command fail: %s" % output) - return output + if status != 0: + test.log.warning("Command failed (status=%d): %s", status, output) + if not omit_failure: + test.error("Command '%s' failed: %s" % (command, output)) + else: + test.log.debug("Command succeeded: %s", output) + return status, outputNote: This would require updating callers to handle the tuple return value.
46-50: Consider verifying that RSS settings were applied successfully.While the command execution will fail the test if the PowerShell command returns non-zero, it would be more robust to verify the RSS configuration was actually applied by reading back the settings with
Get-NetAdapterRss.Add verification after line 50:
# Verify RSS settings were applied verify_cmd = 'powershell -command "Get-NetAdapterRss -Name \\"%s\\" | Select-Object NumberOfReceiveQueues"' % ifname output = execute_command(verify_cmd) test.log.info("Current RSS settings: %s", output)
52-60: Consider adding cleanup code and final status reporting.The test doesn't explicitly clean up the session or report final test status. While the test framework may handle session cleanup automatically, explicitly closing resources is a best practice.
Add cleanup at the end:
# Final test status test.log.info("Network traffic test after RSS configuration completed") # Cleanup if session: session.close()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/virtual_network/qemu/netkvm_rss_test.cfg(1 hunks)libvirt/tests/src/virtual_network/qemu/netkvm_rss_test.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6652
File: libvirt/tests/cfg/virtual_network/passt/passt_function.cfg:106-106
Timestamp: 2025-11-24T10:44:47.801Z
Learning: In libvirt/tests/cfg/virtual_network/passt/passt_function.cfg, passt test cases have only one interface, so running `dhcpcd` without specifying an interface is safe and doesn't cause network disruption.
📚 Learning: 2025-11-24T10:44:47.801Z
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6652
File: libvirt/tests/cfg/virtual_network/passt/passt_function.cfg:106-106
Timestamp: 2025-11-24T10:44:47.801Z
Learning: In libvirt/tests/cfg/virtual_network/passt/passt_function.cfg, passt test cases have only one interface, so running `dhcpcd` without specifying an interface is safe and doesn't cause network disruption.
Applied to files:
libvirt/tests/src/virtual_network/qemu/netkvm_rss_test.pylibvirt/tests/cfg/virtual_network/qemu/netkvm_rss_test.cfg
⏰ 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 (1)
libvirt/tests/src/virtual_network/qemu/netkvm_rss_test.py (1)
38-43: LGTM - directory creation with proper error handling.The logic correctly handles the case where the directory already exists on Windows, which is appropriate.
3 participants
Automate RHEL-292670 heavy network traffic test after set adapterrss
ID:LIBVIRTAT-22206
Signed-off-by: Lei Yang [email protected]
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.