migration: Add case about network load in vm #6697
Conversation
WalkthroughAdds a new libvirt migration test configuration (migration_with_stress/network_load.cfg) and a new test script (libvirt/tests/src/migration/migration_with_stress/network_load.py). The script creates local and remote Linux bridges, reconfigures VM network interfaces, establishes a migration connection, starts a VM and a long-running netperf client inside the guest to generate sustained network load, performs live migration, and then tears down bridges, netperf processes, and migration connections in cleanup. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: 1
🧹 Nitpick comments (3)
libvirt/tests/src/migration/migration_with_stress/network_load.py (3)
80-80: Consider usingast.literal_evalinstead ofeval.While
paramscomes from controlled cfg files,eval()can execute arbitrary code.ast.literal_evalis safer for parsing dictionary literals.+from ast import literal_eval + from avocado.utils import process- iface_dict = eval(params.get("iface_dict", "{}")) + iface_dict = literal_eval(params.get("iface_dict", "{}"))
54-59: Ensurevm_sessionis closed on exception.If an exception occurs during package installation or netperf command execution, the session won't be closed properly.
vm.start() vm_session = vm.wait_for_serial_login() - if not utils_package.package_install('netperf', session=vm_session): - test.error('Failed to install netperf on VM.') - vm_ip = vm.get_address(timeout=60) - vm_session.cmd(f"nohup netperf -H {client_ip} -l 10000000 -L {vm_ip}>/dev/null 2>&1 &") - vm_session.close() + try: + if not utils_package.package_install('netperf', session=vm_session): + test.error('Failed to install netperf on VM.') + vm_ip = vm.get_address(timeout=60) + vm_session.cmd(f"nohup netperf -H {client_ip} -l 10000000 -L {vm_ip}>/dev/null 2>&1 &") + finally: + vm_session.close()
69-71: Add defensive check forremote_host_ifacein cleanup.If
setup_testfails before settingremote_host_iface(line 41), cleanup will passNonetodelete_linux_bridge_tmux, which may fail or behave unexpectedly.- utils_net.delete_linux_bridge_tmux( - bridge_name, iface_name=params.get("remote_host_iface"), - session=remote_session) + remote_host_iface = params.get("remote_host_iface") + if remote_host_iface: + utils_net.delete_linux_bridge_tmux( + bridge_name, iface_name=remote_host_iface, + session=remote_session)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/migration/migration_with_stress/network_load.cfg(1 hunks)libvirt/tests/src/migration/migration_with_stress/network_load.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/cfg/migration/migration_with_stress/network_load.cfglibvirt/tests/src/migration/migration_with_stress/network_load.py
🧬 Code graph analysis (1)
libvirt/tests/src/migration/migration_with_stress/network_load.py (1)
provider/migration/base_steps.py (4)
setup_connection(343-364)cleanup_connection(366-378)MigrationBase(27-477)verify_default(309-324)
🪛 Ruff (0.14.7)
libvirt/tests/src/migration/migration_with_stress/network_load.py
52-52: Function call with shell=True parameter identified, security issue
(S604)
73-73: Function call with shell=True parameter identified, security issue
(S604)
80-80: Use of possibly insecure function; consider using ast.literal_eval
(S307)
⏰ 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 (2)
libvirt/tests/cfg/migration/migration_with_stress/network_load.cfg (1)
1-34: Configuration looks complete for the network load migration test.The configuration properly sets up NFS storage, migration URIs, credentials, and bridge/interface settings needed for the test scenario. Parameters are well-organized and follow the established patterns for libvirt migration tests.
libvirt/tests/src/migration/migration_with_stress/network_load.py (1)
92-97: Main test flow follows the MigrationBase pattern correctly.The try/finally structure ensures cleanup runs even on failure, and the flow (setup → run_migration → verify_default → cleanup) properly integrates with the migration framework.
libvirt/tests/src/migration/migration_with_stress/network_load.py
Outdated
Show resolved
Hide resolved
This case is to verify that vm migration can succeed when there is heavy network load in vm. ID: VIRT-298236 Signed-off-by: lcheng <lcheng@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
libvirt/tests/src/migration/migration_with_stress/network_load.py (3)
95-101: Avoidevalwhen parsingiface_dictfrom paramsUsing
evalon config data is unnecessary and flagged as unsafe;ast.literal_evalis safer and sufficient for dict-like literals.Consider:
+import ast @@ - iface_dict = eval(params.get("iface_dict", "{}")) + iface_dict = ast.literal_eval(params.get("iface_dict", "{}"))Optionally, wrap in
try/except (ValueError, SyntaxError)and calltest.error(...)with a clear message if the value is malformed.
40-44: Make remote bridge cleanup tolerant of partial setup failuresIf
setup_test()fails beforeparams.update({"remote_host_iface": ...}),cleanup_test()will passNoneasiface_nametodelete_linux_bridge_tmux, which may behave unpredictably.Example:
- utils_net.delete_linux_bridge_tmux( - bridge_name, iface_name=params.get("remote_host_iface"), - session=remote_session) + remote_host_iface = params.get("remote_host_iface") + if remote_host_iface: + utils_net.delete_linux_bridge_tmux( + bridge_name, iface_name=remote_host_iface, + session=remote_session)This keeps cleanup robust even if setup aborts early.
Also applies to: 81-85
58-60: Shell commands and process cleanup are acceptable but could be tightenedThe
process.run(..., shell=True)calls andpkill netserverare reasonable for test code, but they’re what security linters complain about.If you want to harden / quiet linters:
- Ensure
netperf_port,netperf_data_port, andclient_ipcome only from trusted cfg and contain no shell metacharacters.- Narrow the kill to the exact binary name:
- process.run("pkill netserver", shell=True, verbose=True, ignore_status=True) + process.run("pkill -x netserver", shell=True, verbose=True, ignore_status=True)You can also add a linter ignore marker on these specific lines if the environment is controlled and this pattern is standard in the suite.
Also applies to: 72-72, 87-87
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/migration/migration_with_stress/network_load.cfg(1 hunks)libvirt/tests/src/migration/migration_with_stress/network_load.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libvirt/tests/cfg/migration/migration_with_stress/network_load.cfg
🧰 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/migration/migration_with_stress/network_load.py
🪛 Ruff (0.14.7)
libvirt/tests/src/migration/migration_with_stress/network_load.py
60-60: Function call with shell=True parameter identified, security issue
(S604)
87-87: Function call with shell=True parameter identified, security issue
(S604)
98-98: Use of possibly insecure function; consider using ast.literal_eval
(S307)
⏰ 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.8
- GitHub Check: Python 3.11
🔇 Additional comments (1)
libvirt/tests/src/migration/migration_with_stress/network_load.py (1)
91-116: Overall test flow and migration orchestration look goodThe main
run()flow (param extraction,MigrationBaseusage, remote session setup, andtry/finallyensuring cleanup) is coherent and matches existing tp-libvirt migration patterns.
|
|
||
| if not utils_package.package_install('netperf'): | ||
| test.error('Failed to install netperf on host.') | ||
| process.run(f"nohup netserver -p {netperf_port} -L {client_ip} -- -D -P {netperf_data_port} >/dev/null 2>&1 &", shell=True, verbose=True) |
There was a problem hiding this comment.
Can we reuse the existing functions for netperf test? for eg. exec_netperf_test() in network provider or utils_netperf in avocado-vt.
3 participants
This case is to verify that vm migration can succeed when there is heavy network load in vm.
ID: VIRT-298236
Test result:
(1/1) type_specific.io-github-autotest-libvirt.migration.migration_with_stress.network_load: STARTED
(1/1) type_specific.io-github-autotest-libvirt.migration.migration_with_stress.network_load: PASS (225.53 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-12-01T22.14-6874cf1/results.html
JOB TIME : 227.19 s
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.