migration: Add case about migration with heavy nework load on host #6733
Conversation
This case is to verify that migration can succeed in high-speed network env with heavy nework load. ID: VIRT-298248 Signed-off-by: lcheng <lcheng@redhat.com>
WalkthroughThe changes add a new test case for VM migration under network load conditions. A configuration file defines migration parameters, including NFS storage setup, SSH connection details, and network performance testing ports. A corresponding test script implements the test by installing netperf on source and target hosts, configuring bidirectional network traffic generation, and executing VM migration while the network is under stress. The test includes setup and cleanup routines to manage firewall settings and netperf processes. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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: 5
🧹 Nitpick comments (3)
libvirt/tests/src/migration/migration_with_stress/network_load_on_host.py (3)
38-39: Consider using firewall port management instead of stopping firewalld.Completely stopping the firewall service may have unintended security implications. Consider using
remote_add_or_remove_port()(as shown in base_steps.py) to open only the required netperf ports instead.- process.run("systemctl stop firewalld", shell=True, verbose=True) - remote.run_remote_cmd("systemctl stop firewalld", params) + netperf_port = params.get("netperf_port") + netperf_data_port = params.get("netperf_data_port") + firewall_cmd = utils_iptables.Firewall_cmd() + firewall_cmd.add_port(netperf_port, 'tcp', permanent=True) + firewall_cmd.add_port(netperf_data_port, 'tcp', permanent=True) + migration_obj.remote_add_or_remove_port(netperf_port) + migration_obj.remote_add_or_remove_port(netperf_data_port)Note: You'll need to add
from virttest import utils_iptablesto imports and update the cleanup function accordingly.
41-44: Remove unnecessary f-string prefixes.Lines 41 and 43 use f-strings without any placeholders.
- src_netserver_cmd = f"nohup netserver >/dev/null 2>&1 &" + src_netserver_cmd = "nohup netserver >/dev/null 2>&1 &" process.run(src_netserver_cmd, shell=True, verbose=True) - dst_netserver_cmd = f"nohup netserver >/dev/null 2>&1 &" + dst_netserver_cmd = "nohup netserver >/dev/null 2>&1 &" remote.run_remote_cmd(dst_netserver_cmd, params, ignore_status=False)
46-50: Consider adding checks for already-running netperf processes.Starting netperf without checking if instances are already running could lead to multiple conflicting processes. Consider adding a check or cleanup before starting new instances.
+ process.run("pkill netperf", shell=True, verbose=True, ignore_status=True) + remote.run_remote_cmd("pkill netperf", params, ignore_status=True) + src_netperf_cmd = f"nohup netperf -H {server_ip} -l 600000000 >/dev/null 2>&1 &" process.run(src_netperf_cmd, shell=True, verbose=True)
📜 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_on_host.cfg(1 hunks)libvirt/tests/src/migration/migration_with_stress/network_load_on_host.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 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_on_host.cfglibvirt/tests/src/migration/migration_with_stress/network_load_on_host.py
📚 Learning: 2025-12-12T10:00:09.383Z
Learnt from: yanglei-rh
Repo: autotest/tp-libvirt PR: 6675
File: libvirt/tests/cfg/virtual_network/qemu/nic_bonding.cfg:16-16
Timestamp: 2025-12-12T10:00:09.383Z
Learning: In tp-libvirt test config files (*.cfg under libvirt/tests/cfg), the cartesian config parser supports variable substitution using the ${variable_name} syntax. Variables defined in the cfg (e.g., filesize = 4000) are substituted into strings that reference them (e.g., count=${filesize}) before the Python test code receives the parameters. Treat this as a general behavior for these cfg files: define variables early, reference them with ${var}, and verify substitutions in tests to avoid hard-coded literals.
Applied to files:
libvirt/tests/cfg/migration/migration_with_stress/network_load_on_host.cfg
🧬 Code graph analysis (1)
libvirt/tests/src/migration/migration_with_stress/network_load_on_host.py (1)
libvirt/tests/src/migration/migrate_vm.py (1)
run_remote_cmd(759-776)
🪛 Ruff (0.14.8)
libvirt/tests/src/migration/migration_with_stress/network_load_on_host.py
38-38: Function call with shell=True parameter identified, security issue
(S604)
41-41: f-string without any placeholders
Remove extraneous f prefix
(F541)
42-42: Function call with shell=True parameter identified, security issue
(S604)
43-43: f-string without any placeholders
Remove extraneous f prefix
(F541)
47-47: Function call with shell=True parameter identified, security issue
(S604)
61-61: Function call with shell=True parameter identified, security issue
(S604)
63-63: Function call with shell=True parameter identified, security issue
(S604)
65-65: Function call with shell=True parameter identified, security issue
(S604)
72-72: Local variable netperf_port is assigned to but never used
Remove assignment to unused variable netperf_port
(F841)
73-73: Local variable netperf_data_port is assigned to but never used
Remove assignment to unused variable netperf_data_port
(F841)
⏰ 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.11
- GitHub Check: Python 3.9
- GitHub Check: Python 3.12
🔇 Additional comments (4)
libvirt/tests/cfg/migration/migration_with_stress/network_load_on_host.cfg (1)
1-30: LGTM!The configuration parameters are well-defined and appropriate for a migration stress test under network load. The NFS storage setup, SSH connection details, and migration options (live, p2p, postcopy) are correctly configured.
libvirt/tests/src/migration/migration_with_stress/network_load_on_host.py (3)
1-17: LGTM!The copyright header and imports are appropriate for this test implementation.
54-66: Cleanup function looks appropriate.The cleanup steps properly terminate netperf processes and restore the firewall state. If you adopt the firewall port management approach suggested earlier, remember to update this cleanup function to remove the added ports instead of restarting firewalld.
82-87: LGTM!The try-finally pattern ensures cleanup runs even if the test fails, which is the correct approach for resource management in tests.
| netperf_port = "12865" | ||
| netperf_data_port = "5000" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "\bnetperf_port\b|\bnetperf_data_port\b" libvirt/tests/src/migration/migration_with_stress/network_load_on_host.py -A 2 -B 2Repository: autotest/tp-libvirt
Length of output: 306
🏁 Script executed:
rg -n "netperf_port|netperf_data_port" libvirt/tests/src/migration/migration_with_stress/network_load_on_host.pyRepository: autotest/tp-libvirt
Length of output: 170
🏁 Script executed:
rg -n "netperf\b" libvirt/tests/src/migration/migration_with_stress/network_load_on_host.py -B 2 -A 2Repository: autotest/tp-libvirt
Length of output: 1374
Remove unused port parameters or pass them to netperf/netserver commands.
The netperf_port and netperf_data_port parameters are retrieved at lines 72-73 but never used anywhere in the test. The netperf invocations at lines 46 and 49 (netperf -H {ip} -l 600000000) don't specify these ports with the -p and -P flags. Either configure the commands to use these ports or remove the unused parameters from the config.
libvirt/tests/src/migration/migration_with_stress/network_load_on_host.py
Show resolved
Hide resolved
libvirt/tests/src/migration/migration_with_stress/network_load_on_host.py
Show resolved
Hide resolved
| netperf_port = params.get("netperf_port") | ||
| netperf_data_port = params.get("netperf_data_port") |
There was a problem hiding this comment.
Unused port configuration parameters.
The netperf_port and netperf_data_port parameters are retrieved but never used. If these ports should be configured for netperf (using the -p flag), please add them to the netperf commands in setup_test(). Otherwise, these lines and the corresponding config parameters can be removed.
Related to the unused parameters in the config file at lines 31-32.
🧰 Tools
🪛 Ruff (0.14.8)
72-72: Local variable netperf_port is assigned to but never used
Remove assignment to unused variable netperf_port
(F841)
73-73: Local variable netperf_data_port is assigned to but never used
Remove assignment to unused variable netperf_data_port
(F841)
🤖 Prompt for AI Agents
In libvirt/tests/src/migration/migration_with_stress/network_load_on_host.py
around lines 72-73, the params netperf_port and netperf_data_port are retrieved
but never used; either include them in the netperf invocation inside
setup_test() (add the control port and data port via the netperf -p
control_port,data_port flag where the netperf client/server commands are
constructed and ensure defaults or validation for None) or remove these two
params and their corresponding entries in the test config (and any related
docstring) so they are not fetched; update setup_test() to use the params when
present and add a small validation/conditional to avoid injecting empty flags.
| remote_runner = remote.RemoteRunner(host=server_ip, | ||
| username=server_user, | ||
| password=server_pwd) | ||
| remote_session = remote_runner.session |
There was a problem hiding this comment.
Unused remote_session variable.
The remote_session variable is created but never used in the test. If it's not needed, consider removing it. If it should be passed to utils_package.package_install() on line 35, ensure it's in scope.
remote_runner = remote.RemoteRunner(host=server_ip,
username=server_user,
password=server_pwd)
- remote_session = remote_runner.session📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| remote_session = remote_runner.session |
🤖 Prompt for AI Agents
In libvirt/tests/src/migration/migration_with_stress/network_load_on_host.py
around line 80, the local variable remote_session is assigned but never used;
either remove the remote_session assignment to eliminate the unused variable, or
if the intention was to supply the remote session to
utils_package.package_install() (called on line 35), update that call to pass
remote_session (and ensure remote_session is defined before line 35 or move the
package_install call after its assignment) so the variable is actually used.
2 participants
This case is to verify that migration can succeed in high-speed network env with heavy nework load.
ID: VIRT-298248
Test result:
(1/1) type_specific.io-github-autotest-libvirt.migration.migration_with_stress.network_load_on_host: STARTED
migration.migration_with_stress.network_load_on_host
(1/1) type_specific.io-github-autotest-libvirt.migration.migration_with_stress.network_load_on_host: PASS (211.65 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-15T02.10-31187f8/results.html
JOB TIME : 214.61 s
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.