Improve mtu to create guest xml via the framework#6807
Improve mtu to create guest xml via the framework#6807Yingshun merged 1 commit intoautotest:masterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds Libvirt VM lifecycle flags to libvirt/tests/cfg/virtual_network/mtu.cfg under the virtual_network.mtu section: Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ 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
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. Comment Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
|
Test results: |
|
@yanglei-rh , Could you help to review this patch? |
yiqianwei
force-pushed
the
LIBVIRTAT-22318
branch
from
c07432c to
d137f7f
Compare
|
Hi @nickzhq, Add "only Linux" to limit guest type, Could you help review it again?thanks |
|
@yanglei-rh Can you please help review this pr? Thanks! |
| if not libvirt_version.version_compare(7, 0, 0): | ||
| test.cancel('This test is not supported until libvirt-7.0.0') | ||
|
|
||
| if net_type in ('network', 'bridge'): |
There was a problem hiding this comment.
Hi @yiqianwei According to the case cfg, this case support to test with ovs bridge, why you setup cancel this scenario?
There was a problem hiding this comment.
@yanglei-rh ,When the host uses an OVS bridge, skip the relevant non-OVS bridge test cases. When the host uses a Linux bridge, skip the non-Linux bridge test cases.
yiqianwei
force-pushed
the
LIBVIRTAT-22318
branch
from
d137f7f to
7c616a3
Compare
|
Test results: for ovs_br: |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libvirt/tests/src/virtual_network/mtu.py`:
- Around line 58-66: The code overwrites the earlier bridge_name and doesn't
handle exceptions from utils_net.find_bridge_manager; change the local variable
used for the lookup (e.g., use bridge_to_check or bridge_dest =
params.get('netdst','').split(',')[0]) instead of reassigning bridge_name, and
wrap the call to utils_net.find_bridge_manager(bridge_to_check) in a try/except
catching process.CmdError; on exception call test.cancel with a clear message
including the error, and keep the existing isinstance(br_backend,
utils_net.Bridge) check to cancel when appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c93aec5-d68f-49fa-b2b6-192de5467a2c
📒 Files selected for processing (2)
libvirt/tests/cfg/virtual_network/mtu.cfglibvirt/tests/src/virtual_network/mtu.py
🚧 Files skipped from review as they are similar to previous changes (1)
- libvirt/tests/cfg/virtual_network/mtu.cfg
1.Add some parameters to allow the framework provide guest xml 2.Call cancel_if_ovs_bridge to cancel test when host using linux bridge 3.Add "only Linux" to limit guest type ID: LIBVIRTAT-22318 Signed-off-by: Yiqian Wei <yiwei@redhat.com>
yiqianwei
force-pushed
the
LIBVIRTAT-22318
branch
from
7c616a3 to
b3ac726
Compare
4 participants
1.Add some parameters to allow the framework provide guest xml
2.Call cancel_if_ovs_bridge to cancel test when host using linux bridge
3.Add "only Linux" to limit guest type
ID: LIBVIRTAT-22318
Summary by CodeRabbit