virtual_network/migrate_with_bridge: enable remote host iface config #6717
Conversation
WalkthroughThis change introduces explicit remote interface handling to a virtual network migration test. A new configuration key Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
We previously enabled configuration for host face selection for bridge setup to account for environments where it's not the default interface (the first one). However, the test case also requires this for the remote host as it's a migration test case. Add this option and the same setting logic as for 'host_iface'. For this, the setting logic has to be brought further up in the execution where the variable is defined. Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
smitterl
force-pushed
the
vn_add_remote_iface_config
branch
from
c0b31a0 to
25e5d62
Compare
|
The ovs bridge must be created with a specific host interface that provides DHCP which in our case isn't the default. If the value is not configured the behavior is unchanged. Signed-off-by: Sebastian Mitterle <smitterl@redhat.com>
|
![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/virtual_network/migrate/migrate_with_bridge_type_interface.py (3)
175-179: Consider using the variable directly for consistency.While
params.get("remote_host_iface")works correctly, using theremote_host_ifacevariable directly (as done in the OVS teardown at line 189) would be more consistent and clearer.Apply this diff for consistency:
utils_net.delete_linux_bridge_tmux( bridge_name, - iface_name=params.get("remote_host_iface"), + iface_name=remote_host_iface, session=remote_session, )
220-220: Consider removing unused variable.The variable
src_uriis assigned but never used in the code. If it's not needed for future use, consider removing it.- src_uri = params.get("virsh_migrate_connect_uri") dest_uri = params.get("virsh_migrate_desturi")
233-233: Consider removing unused variable.The variable
new_xmlis assigned but never used in the code. If it's not needed for future use, consider removing it.vm = env.get_vm(vm_name) - new_xml = vm_xml.VMXML.new_from_inactive_dumpxml(vm_name)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/virtual_network/migrate/migrate_with_bridge_type_interface.cfg(1 hunks)libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py(6 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 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/virtual_network/migrate/migrate_with_bridge_type_interface.cfglibvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py
📚 Learning: 2025-11-18T08:47:14.465Z
Learnt from: smitterl
Repo: autotest/tp-libvirt PR: 6072
File: libvirt/tests/src/passthrough/pci/libvirt_pci_passthrough_hotplug.py:239-241
Timestamp: 2025-11-18T08:47:14.465Z
Learning: In tp-libvirt tests, `utils_test.ping` is a valid function that exists in the avocado-vt framework (virttest.utils_test module). It is an alias to `utils_net.ping` and supports parameters including dest, count, interface, timeout, and session.
Applied to files:
libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py
📚 Learning: 2025-09-24T08:01:27.899Z
Learnt from: hholoubk
Repo: autotest/tp-libvirt PR: 6579
File: libvirt/tests/src/sriov/vIOMMU/iommu_device_lifecycle.py:95-97
Timestamp: 2025-09-24T08:01:27.899Z
Learning: In the libvirt test framework used in tp-libvirt, VM cleanup including destroying running VMs is handled by the teardown method (test_obj.teardown_iommu_test()) called in the finally block, so explicit VM destroy calls on timeout are not necessary according to the maintainers.
Applied to files:
libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py
🧬 Code graph analysis (1)
libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py (2)
provider/virtual_network/network_base.py (1)
ping_check(98-149)provider/guest_os_booting/guest_os_booting_base.py (1)
get_vm(18-56)
🪛 Ruff (0.14.8)
libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py
220-220: Local variable src_uri is assigned to but never used
Remove assignment to unused variable src_uri
(F841)
222-222: Use of possibly insecure function; consider using ast.literal_eval
(S307)
223-223: Use of possibly insecure function; consider using ast.literal_eval
(S307)
233-233: Local variable new_xml is assigned to but never used
Remove assignment to unused variable new_xml
(F841)
🔇 Additional comments (4)
libvirt/tests/cfg/virtual_network/migrate/migrate_with_bridge_type_interface.cfg (1)
30-30: LGTM! Configuration addition aligns with PR objectives.The new
remote_host_ifaceconfiguration key provides the expected capability for specifying the remote host interface during migration tests, mirroring the existinghost_ifaceparameter.libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py (3)
182-195: LGTM! OVS teardown correctly mirrors the setup logic.The cleanup properly removes both local and remote OVS bridges with explicit interface names, and deletes networks on both hosts using the
remote_argsparameter.
113-126: OVS bridge and network setup correctly extended for remote host.The changes properly configure both local and remote OVS bridges with explicit interface names, and the network is created on both hosts using the
remote_argsparameter.
107-110: Verifycreate_linux_bridge_tmuxsupports theiface_nameandsessionparameters.Lines 108-110 pass
iface_name=remote_host_ifaceandsession=remote_sessiontocreate_linux_bridge_tmux(). Confirm these parameters are accepted by the function signature in theutils_netmodule to ensure the remote bridge creation will work as intended.
| remote_session = aexpect.remote.remote_login( | ||
| "ssh", server_ip, "22", server_user, server_pwd, r"[$#%]" | ||
| ) | ||
| remote_host_iface = params.get("remote_host_iface") | ||
| if not remote_host_iface: | ||
| remote_host_iface = utils_net.get_default_gateway( | ||
| iface_name=True, session=remote_session, force_dhcp=False, json=True | ||
| ) | ||
| params.update({"remote_host_iface": remote_host_iface}) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for get_default_gateway function definition
ast-grep --pattern 'def get_default_gateway($$$)'Repository: autotest/tp-libvirt
Length of output: 182
🏁 Script executed:
# Also search with rg for broader context
rg "def get_default_gateway" -A 15Repository: autotest/tp-libvirt
Length of output: 182
🌐 Web query:
utils_net.get_default_gateway session parameter autotest libvirt
💡 Result:
There is no utils_net.get_default_gateway that takes a session. Use virttest.utils_net.get_host_default_gateway() (host default gateway — no session param). For guest-side lookups use virttest.utils_net.get_guest_ip_addr(session, mac_addr, ...), which does accept a session. [1][2]
Sources:
- virttest.utils_net API (get_host_default_gateway, get_guest_ip_addr). [1]
- virttest documentation index showing utils_net functions. [2]
Fix incorrect API call: get_default_gateway does not support the session parameter.
The code at lines 235-242 calls utils_net.get_default_gateway() with a session parameter for remote execution, but this function does not accept a session parameter. For remote gateway resolution, use utils_net.get_guest_ip_addr(session, mac_addr, ...) instead, which is designed to work with session objects. Alternatively, if you need the host default gateway on the remote machine, you may need to execute the appropriate command directly over the remote session.
🤖 Prompt for AI Agents
In
libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py
around lines 235-243, the call to utils_net.get_default_gateway(...,
session=remote_session, ...) is invalid because get_default_gateway does not
accept a session; replace this with a remote-capable call: either call
utils_net.get_guest_ip_addr(remote_session, mac_addr, ...) (provide the
appropriate mac_addr from test params or obtained from the VM) to resolve the
remote address, or run the host command over remote_session to extract the
default gateway (e.g., parse `ip route` / `route -n`) and assign that result to
remote_host_iface; finally update params with the obtained value and remove the
invalid session argument from get_default_gateway usage.
2 participants
Depends on avocado-framework/avocado-vt#4280
We previously enabled configuration for host face selection for bridge setup to account for environments where it's not the default interface (the first one). However, the test case also requires this for the remote host as it's a migration test case.
Add this option and the same setting logic as for 'host_iface'.
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.