-
Notifications
You must be signed in to change notification settings - Fork 178
Add case to support netperf stress tests #6622
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
1. Automate VIRT-95955 netperf stress tests 2. Create a new directory to store netperf packages 3. The netperf_test function's doc string from claude code Signed-off-by: Lei Yang <[email protected]>
WalkthroughIntroduces a new netperf stress test framework for virtual networks consisting of a configuration file defining test parameters, OS-specific overrides, protocol variants, and a comprehensive Python test script that orchestrates multi-endpoint netperf stress testing with dynamic VM preparation, server initialization, background test execution, and resource cleanup. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Script
participant VM as VM/Endpoint
participant Server as NetperfServer
participant Client as NetperfClient
participant Monitor as Monitor Loop
Test->>Test: Parse config & resolve endpoints<br/>(OS type, IP, credentials)
Test->>Test: Determine host IP (VLAN support)
Note over Test: Preparation Phase
Test->>VM: Extract per-endpoint metadata<br/>(paths, MD5 sums, shells)
Test->>Server: Initialize NetperfServer<br/>instances
Test->>Client: Initialize NetperfClient<br/>instances
Note over Test: Execution Phase
Test->>Server: Start all servers
Test->>Test: Build test config<br/>(duration, protocols, etc.)
loop For each protocol
Test->>Test: Assign servers/clients<br/>(round-robin)
Test->>Client: Launch netperf in<br/>background (bg_start)
Test->>Monitor: Track start time<br/>set env["netperf_run"]=True
end
Note over Monitor: Monitoring Phase
loop Until completion
Monitor->>Monitor: Sleep & check<br/>if netperf running
alt Premature termination detected
Monitor->>Test: FAIL (hung/crashed)
else Still running
Monitor->>Monitor: Continue monitoring
else Completed successfully
Monitor->>Test: Mark success
end
end
Note over Test: Cleanup Phase
Test->>Server: Stop/cleanup all servers
Test->>Client: Stop/cleanup all clients
Test->>Test: Clear env["netperf_run"]
Test->>VM: Close login sessions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Rationale: The changes introduce substantial new functionality with multiple complex components: multi-endpoint orchestration logic, OS-specific conditional handling (Windows/Linux paths, MD5 sums, shell clients), VLAN configuration resolution, dynamic round-robin server/client assignment, background process monitoring with hang/crash detection, error context propagation, and comprehensive resource cleanup. The execution flow spans multiple abstraction levels (config parsing, endpoint resolution, server/client instantiation, protocol iteration, background monitoring) requiring careful validation of state transitions, error paths, and resource management. Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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: 7
🧹 Nitpick comments (6)
libvirt/tests/cfg/virtual_network/qemu/netperf_stress_test.cfg (2)
31-40: Clarify client/server roles in host2guest variant.The header comment says “server is main vm, client is host/another vm,” but host2guest sets server=localhost and client=${vms}, reversing roles. This may confuse test readers.
Add a brief comment under host2guest explaining the inversion or adjust the header comment for accuracy.
28-29: Legacy RHEL.4 override—keep or drop?If RHEL.4 is not actively tested, remove the override to reduce maintenance. Otherwise, add a short note why it’s still needed.
libvirt/tests/src/virtual_network/qemu/netperf_stress_test.py (4)
270-278: Guard against empty server list before modulo.If misconfigured, len(server_infos) can be 0, causing ZeroDivisionError.
- s_len = len(server_infos) + s_len = len(server_infos) + if s_len == 0: + test.error("No netperf servers provided in 'netperf_server'")
285-291: Consider longer wait for netperf start.10s total may be short under load/slow guests; raise timeout or make it configurable (e.g., netperf_start_timeout).
195-199: /var/tmp usage flagged; prefer a per-test subdir with restricted perms.Static analysis warns about S108. Use a dedicated directory under data_dir.get_tmp_dir() or create /var/tmp/netperf-$UID with 0700.
Based on static analysis hints
252-254: Optionally verify netserver readiness before starting clients.After n_server.start(), poll a simple health check to avoid client start flaps.
Happy to add a small wait_for(n_server.is_running, ...) block if available in utils_netperf.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
deps/netperf/netperf-2.4.5.tar.bz2is excluded by!**/*.bz2deps/netperf/netperf-2.6.0-1.tar.bz2is excluded by!**/*.bz2deps/netperf/netperf-2.6.0.tar.bz2is excluded by!**/*.bz2deps/netperf/netperf-2.7.1-1.tar.bz2is excluded by!**/*.bz2deps/netperf/netperf-2.7.1.tar.bz2is excluded by!**/*.bz2deps/netperf/netperf.exeis excluded by!**/*.exedeps/netperf/netperf_32bit.exeis excluded by!**/*.exedeps/netperf/netserver-2.6.0.exeis excluded by!**/*.exedeps/netperf/netserver_32bit.exeis excluded by!**/*.exe
📒 Files selected for processing (2)
libvirt/tests/cfg/virtual_network/qemu/netperf_stress_test.cfg(1 hunks)libvirt/tests/src/virtual_network/qemu/netperf_stress_test.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
libvirt/tests/src/virtual_network/qemu/netperf_stress_test.py (1)
libvirt/tests/src/serial/serial_functional.py (1)
wait_for_login(235-258)
🪛 Ruff (0.14.0)
libvirt/tests/src/virtual_network/qemu/netperf_stress_test.py
195-195: Probable insecure usage of temporary file or directory: "/var/tmp"
(S108)
196-196: Probable insecure usage of temporary file or directory: "/var/tmp"
(S108)
🔇 Additional comments (2)
libvirt/tests/cfg/virtual_network/qemu/netperf_stress_test.cfg (2)
41-51: Protocol matrix looks good.TCP_MAERTS spelling is correct for the reverse STREAM test; gating TCP_SENDFILE with “no Windows” is appropriate.
20-27: The review comment is incorrect.The virttest configuration parser automatically strips quotes when parsing .cfg files. According to virttest's cartesian_config parser documentation, "the parser treats the quotes as string delimiters and does not keep the surrounding quotes as part of the parameter value."
This means:
netperf_server_link_win = "netserver-2.6.0.exe"in the .cfg file- Is parsed as:
netserver-2.6.0.exe(quotes removed)params.get("netperf_server_link_win")returns the unquoted valueos.path.join()receives and joins the valid filename without literal quotes- Result:
/path/to/netperf/netserver-2.6.0.exe✓ (valid path)No code changes are needed. The suggested diff would be unnecessary.
Likely an incorrect or invalid review comment.
|
@nanli1 Please help review this patch, thanks a lot. |
ID: LIBVIRTAT-22103
Signed-off-by: Lei Yang [email protected]
Summary by CodeRabbit