Improve connectivity_check_bridge_interface to create guest xml via the framework#6796
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRefactors the bridge-interface test config from multiple per-bridge variants to a single bridge-centric structure, consolidating multiqueue and nwfilter attributes and aligning s390-virtio settings. Adds explicit VM/libvirt lifecycle controls and image/login settings (create_vm_libvirt, use_no_reboot, kill_vm_libvirt with ovmf options, master_images_clone, remove_image_image1, login_timeout). Test code now derives bridge_name from netdst, invokes setup_ovs_bridge_attrs for OVS detection/cancellation, uses login_timeout for serial logins, and removes explicit Linux/OVS bridge creation and teardown logic. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can use your project's `ruff` configuration to improve the quality of Python code reviews.Add a Ruff configuration file to your project to customize how CodeRabbit runs |
|
Test pass. |
yanglei-rh
force-pushed
the
LIBVIRTAT-22301
branch
2 times, most recently
from
41303b1 to
cc3768a
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libvirt/tests/src/virtual_network/connectivity/connectivity_check_bridge_interface.py (1)
169-170:⚠️ Potential issue | 🟠 MajorCleanup may fail if nwfilter was never defined.
The nwfilter is only defined when
libvirt_version < 8.0.0(line 69), but the cleanup unconditionally tries to undefine it whennwfilter_attrsis present. On libvirt >= 8.0.0, this will attempt to undefine a filter that was never created.Proposed fix to match the define condition
finally: [backup_xml.sync() for backup_xml in bkxmls] - if nwfilter_attrs: + if nwfilter_attrs and not libvirt_version.version_compare(8, 0, 0): virsh.nwfilter_undefine(nwfilter_attrs['filter_name'])
🤖 Fix all issues with AI agents
In
`@libvirt/tests/src/virtual_network/connectivity/connectivity_check_bridge_interface.py`:
- Line 46: Remove the unused local variable assignment "rand_id =
utils_misc.generate_random_string(3)" from the
connectivity_check_bridge_interface module; locate the statement where rand_id
is defined and delete that line (no other code changes are needed since rand_id
is not referenced elsewhere).
7563dad to
1599f98
Compare
yanglei-rh
force-pushed
the
LIBVIRTAT-22301
branch
from
1599f98 to
6f57d96
Compare
|
Hi @YongxueHong Please help review this patch again, thanks a lot. |
…he framework 1.Add some parameters to allow the framework provide guest xml 2.Remove the ovs and linux_br variants, let them created by the test loop's environment setup part 3.Add some codes to support ovs bridge type Signed-off-by: Lei Yang <leiyang@redhat.com>
yanglei-rh
force-pushed
the
LIBVIRTAT-22301
branch
from
6f57d96 to
7b1172c
Compare
|
@yanglei-rh You have several PRs for adopting KAR runner, I wonder if they still work on previous test setup or not. Or are these scripts still running on avocado runner? do your changes work well on multi arch network jobs using avocado runner? |
Hi @Yingshun KAR itself does not determine the test; it serves merely as an auxiliary tool. The underlying command remains the CC @YongxueHong If my explanation of "kar" is not sufficiently professional, please feel free to add to it. |
|
I agree that the KAR execute avocado command runs the test cases. However, that KAR includes setup configurations and actions. For example, your commit message states: "2.Remove the ovs and linux_br variants, let them created by the test loop's environment setup part." Since bridge-related variant actions have been removed, how will upstream users be aware of this change? How can the test with this PR be used to execute the original test scenarios for upstream users? |
For the upstream user, they can based on the "create_vm_libvirt=yes" parameters usage to know they should provide a |
Ok, I see. but I don't think everyone can quickly understand how to use "create_vm_libvirt=yes" parameter. Can you please provide 'avocado run' command like above for the following PRs? |
Ok, I will paste the avocado runnner command. |
3 participants
1.Add some parameters to allow the framework provide guest xml
2.Remove the ovs and linux_br variants, let them created by the test loop's environment setup part
3.Add some codes to support ovs bridge type
ID::LIBVIRTAT-22301
Signed-off-by: Lei Yang leiyang@redhat.com
Summary by CodeRabbit
New Features
Improvements