Add case to support nic bonding test

[yanglei-rh]

Automate VIRT-95958 nic bonding test

ID:LIBVIRTAT-22160
Signed-off-by: Lei Yang [email protected]

Summary by CodeRabbit

• Tests
  • Added NIC bonding failover test for virtual networks. Validates data transfer integrity during interface failover scenarios across multiple bonded NICs with configurable parameters on RHEL 7/8/9 variants.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This pull request adds a new NIC bonding test for libvirt. It introduces a configuration file that defines test parameters for bonding with four NICs, including bonding module settings, timeouts, and directories, with Linux-specific DHCP command overrides. Additionally, a new Python test script implements NIC bonding failover testing by configuring bonding in a guest VM, performing data transfers between host and guest while randomly toggling network interfaces up and down, and verifying transfer integrity through MD5 checksums. The script includes proper cleanup of bonding and SSH configurations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Python test script complexity: The core test logic involves asynchronous file transfer operations with concurrent NIC failover simulation, threading coordination, error handling for process termination, and MD5 verification. Each of these components requires careful review to ensure correctness.
• Bonding configuration and parameter handling: Verify that bonding module loading, interface configuration commands, and parameter passing are correct and follow established patterns.
• Thread synchronization and cleanup: The use of InterruptedThread for concurrent operations and the finally block cleanup logic need thorough inspection to prevent resource leaks or race conditions.
• Error handling and edge cases: Ensure proper exception handling for ShellProcessTerminatedError and validation of transfer integrity verification logic.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding a new NIC bonding test case with corresponding configuration and implementation files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (5)

libvirt/tests/cfg/virtual_network/qemu/nic_bonding.cfg (1)

18-20: Align DHCP client choice with guest distro and Python cleanup

dhcp_cmd defaults to dhcpcd -n bond0, but RHEL variants override to dhclient -r bond0 && dhclient bond0, while the Python test always cleans up via kill -9 \pgrep dhclient`. On non‑RHEL guests without dhcpcdthis will likely fail, and even on RHEL you may kill unrelateddhclient` instances.

Consider either narrowing this test to RHEL only and using dhclient consistently, or adding per‑distro overrides (including for non‑RHEL Linux) and updating the cleanup in nic_bonding.py to target the actually used client (or to scope the kill to the bond0 instance only).

libvirt/tests/src/virtual_network/qemu/nic_bonding.py (4)

31-34: Optionally validate discovered NICs against the expected count

You rely on utils_net.get_linux_ifname(session_serial) to return the NICs to enslave, but the config explicitly expects four NICs (nics = "nic1 nic2 nic3 nic4"). To catch misconfigured guests early, you could compare len(ifnames) to the expected count from params["nics"] and fail fast if they diverge.

---

55-58: Make guest_path construction robust to tmp_dir formatting

guest_path = os.path.join(tmp_dir + "dst-%s" % utils_misc.generate_random_string(8)) relies on tmp_dir ending with a trailing slash. With the current cfg (/var/tmp/) this is fine, but it becomes fragile if another cfg reuses this test with tmp_dir = "/var/tmp".

You can make it robust and mirror the host side by using two arguments to os.path.join:

-    guest_path = os.path.join(tmp_dir + "dst-%s" % utils_misc.generate_random_string(8))
+    guest_path = os.path.join(
+        tmp_dir, "dst-%s" % utils_misc.generate_random_string(8)
+    )

---

60-61: shell=True for dd is low‑risk here but flagged by S604

process.run(dd_cmd % host_path, shell=True) is using dd_cmd from controlled params and host_path from test.tmpdir, so from a security perspective this is effectively trusted input in a test harness. However, Ruff’s S604 warning will keep firing.

If you want to silence S604, consider invoking dd without a shell, building the argv list directly from filesize and host_path, or explicitly documenting that this usage is intentional and adding an ignore comment (e.g. # noqa: S604).

---

117-120: Cleanup of DHCP client is broad and assumes dhclient

session_serial.sendline("kill -9 \pgrep dhclient`")will kill everydhclientin the guest, not just the one you started forbond0, and it does nothing if dhcpcd` (from the default cfg) is used instead. Because the VM is killed after the test this is mostly cosmetic, but it’s a bit heavy‑handed.

If you want more precise cleanup, consider either:

• targeting only the bond0 client (e.g. pkill -9 -f "dhclient.*bond0"), or
• aligning with the dhcp_cmd in the cfg and cleaning up the matching client type.

This also ties into the DHCP alignment comment in nic_bonding.cfg.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a302a6f and 905b4e4.

📒 Files selected for processing (2)

• libvirt/tests/cfg/virtual_network/qemu/nic_bonding.cfg (1 hunks)
• libvirt/tests/src/virtual_network/qemu/nic_bonding.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

libvirt/tests/src/virtual_network/qemu/nic_bonding.py (1)

libvirt/tests/src/virsh_cmd/domain/virsh_domfsfreeze_domfsthaw.py (1)

• generate_random_string (104-113)

🪛 Ruff (0.14.5)

libvirt/tests/src/virtual_network/qemu/nic_bonding.py

60-60: Function call with shell=True parameter identified, security issue

(S604)

---

77-77: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

---

79-79: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

---

105-105: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

⏰ 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.12
• GitHub Check: Python 3.9
• GitHub Check: Python 3.8
• GitHub Check: Python 3.11

🔇 Additional comments (1)

libvirt/tests/src/virtual_network/qemu/nic_bonding.py (1)

69-80: Use of random.randint for sleep intervals is acceptable; optionally silence S311

The failover loops use random.randint only to vary sleep durations while toggling links, which is not a cryptographic use case. The Ruff S311 warnings on these lines can safely be ignored.

If you want a clean lint run, add a localized # noqa: S311 on the random.randint calls instead of switching to a cryptographic RNG, which would add overhead without improving test quality.

Also applies to: 94-107