utils_net: enable host interface setting for ovs bridge

[smitterl]

Tested via autotest/tp-libvirt#6717

ovs bridge handling discovers the network interface to use. However, in our environments we need to select a specific interface that has DHCP. Make this parameter configurable but default to previous behavior to not change anything for existing tests.

Summary by CodeRabbit

Release Notes

• New Features
  • OpenVSwitch bridge operations now support optional interface specification, with automatic interface discovery when not explicitly provided.

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

[coderabbitai]

Walkthrough

This pull request adds an optional iface_name parameter to two OpenVswitch bridge management functions in virttest/utils_net.py: create_ovs_bridge and delete_ovs_bridge. When the parameter is not provided, the code auto-discovers the interface by retrieving network interfaces in UP state. The parameter is then passed through to the underlying tmux commands for bridge operations, preserving existing behavior when explicitly specified.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Single file modified with localized changes
• Straightforward conditional logic for parameter discovery vs. explicit provision
• Backward-compatible addition (optional parameter with default None)
• May require verification of:
  • Interface auto-discovery logic correctness (UP state filtering)
  • Proper parameter threading through command construction
  • Testing coverage for both explicit and auto-discovery code paths

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and clearly describes the main change: making the host interface setting configurable for OVS bridge operations.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

virttest/utils_net.py (1)

4631-4632: Consider validating user-provided interface name.

When iface_name is explicitly provided by the user, the function doesn't validate that the interface exists or is in an appropriate state. This could lead to confusing errors later in the tmux command execution.

Consider adding validation like:

if not iface_name:
    up_ifaces = get_net_if(runner=runner, state="UP", ip_options=ip_options)
    if not up_ifaces:
        raise exceptions.TestError(
            "No network interfaces in UP state found for OVS bridge creation"
        )
    iface_name = up_ifaces[0]
else:
    # Validate user-provided interface exists and is UP
    all_ifaces = get_net_if(runner=runner, ip_options=ip_options)
    if iface_name not in all_ifaces:
        raise exceptions.TestError(
            f"Specified interface '{iface_name}' does not exist"
        )
    up_ifaces = get_net_if(runner=runner, state="UP", ip_options=ip_options)
    if iface_name not in up_ifaces:
        LOG.warning(
            f"Specified interface '{iface_name}' is not in UP state"
        )

This same pattern would apply to delete_ovs_bridge at lines 4677-4678.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be01736 and f6ebb37.

📒 Files selected for processing (1)

• virttest/utils_net.py (6 hunks)

⏰ 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). (1)

• GitHub Check: Static checks

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Add bounds checking to prevent IndexError.

If get_net_if() returns an empty list (e.g., no interfaces in UP state), accessing [0] will raise an IndexError and crash at runtime.

Apply this diff to add proper error handling:

-    if not iface_name:
-        iface_name = get_net_if(runner=runner, state="UP", ip_options=ip_options)[0]
+    if not iface_name:
+        up_ifaces = get_net_if(runner=runner, state="UP", ip_options=ip_options)
+        if not up_ifaces:
+            raise exceptions.TestError(
+                "No network interfaces in UP state found for OVS bridge creation"
+            )
+        iface_name = up_ifaces[0]

🤖 Prompt for AI Agents

In virttest/utils_net.py around lines 4631-4632, the code unconditionally
indexes [0] from get_net_if() which can be an empty list; add bounds checking by
capturing the returned list into a variable, test if it's non-empty, and only
then assign iface_name = result[0]; if the list is empty, raise a clear
exception (e.g., ValueError) or return a sensible default/None with a
descriptive message so the caller does not get an unhandled IndexError.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Add bounds checking to prevent IndexError.

If get_net_if() returns an empty list (e.g., no interfaces in UP state), accessing [0] will raise an IndexError and crash at runtime.

Apply this diff to add proper error handling:

-    if not iface_name:
-        iface_name = get_net_if(runner=runner, state="UP", ip_options=ip_options)[0]
+    if not iface_name:
+        up_ifaces = get_net_if(runner=runner, state="UP", ip_options=ip_options)
+        if not up_ifaces:
+            raise exceptions.TestError(
+                "No network interfaces in UP state found for OVS bridge deletion"
+            )
+        iface_name = up_ifaces[0]

🤖 Prompt for AI Agents

In virttest/utils_net.py around lines 4677-4678, the call get_net_if(...)[0] can
raise IndexError if it returns an empty list; change to first assign the result
to a variable (e.g., candidates = get_net_if(...)), check if candidates is
non-empty, and if empty raise a clear exception or handle the case (return, log
and exit, or raise ValueError with context including runner and ip_options);
otherwise set iface_name = candidates[0]. Ensure the new behavior is covered by
an appropriate error message and the function's callers can handle the raised
error.

[smitterl]

Tested via autotest/tp-libvirt#6717