Add case for updating interface mtu during migration #6611
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughAdds a negative test variant that exercises updating a network interface MTU during migration and extends migration XML update logic to handle modify="mtu" by setting the interface mtu size to updated_mtu; test expects an error when MTU mismatch is unsupported. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test Runner
participant CFG as migration_xml_update.cfg
participant PY as migration_xml_update.py
participant XML as Domain XML
participant M as Migration Engine
T->>CFG: Load variant "update_interface_mtu" (negative)
T->>PY: run(modify="mtu", original_mtu, updated_mtu, status_error=yes)
PY->>XML: create_migration_xml(modify="mtu")
Note right of PY #EFEFEF: Find interface device -> mtu element\nSet `mtu/@size` = updated_mtu
PY->>M: Attempt migration with modified XML
M-->>T: Return error matching MTU mismatch pattern
Note over T,M #FFF2CC: Test expects failure (status_error=yes)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
Comment |
![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 (1)
libvirt/tests/src/migration/migration_xml_update.py (1)
108-108: Verify trailing newline is required.The added trailing newline appears to be a formatting preference. Confirm this aligns with the project's coding guidelines or was intentional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/cfg/migration/migration_xml_update.cfg(1 hunks)libvirt/tests/src/migration/migration_xml_update.py(2 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). (4)
- GitHub Check: Python 3.12
- GitHub Check: Python 3.9
- GitHub Check: Python 3.11
- GitHub Check: Python 3.8
🔇 Additional comments (1)
libvirt/tests/cfg/migration/migration_xml_update.cfg (1)
48-55: LGTM!The test variant configuration is well-structured for a negative test case. The MTU values (5000 and 6000) are appropriate, and the error pattern correctly uses regex to match the expected failure message.
Comment on lines
+58
to
+61
| elif modify == "mtu": | ||
| # Update MTU size for interface element | ||
| mtu_element = dev.find("mtu") | ||
| mtu_element.attrib['size'] = params.get("updated_mtu") |
There was a problem hiding this comment.
Add null check for mtu_element.
The code assumes the mtu element exists in the device XML, but dev.find("mtu") returns None if not found. Accessing .attrib on None will raise an AttributeError.
Apply this diff to add a null check:
elif modify == "mtu":
# Update MTU size for interface element
mtu_element = dev.find("mtu")
+ if mtu_element is None:
+ test.error("MTU element not found in device XML")
mtu_element.attrib['size'] = params.get("updated_mtu")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| elif modify == "mtu": | |
| # Update MTU size for interface element | |
| mtu_element = dev.find("mtu") | |
| mtu_element.attrib['size'] = params.get("updated_mtu") | |
| elif modify == "mtu": | |
| # Update MTU size for interface element | |
| mtu_element = dev.find("mtu") | |
| if mtu_element is None: | |
| test.error("MTU element not found in device XML") | |
| mtu_element.attrib['size'] = params.get("updated_mtu") |
🤖 Prompt for AI Agents
In libvirt/tests/src/migration/migration_xml_update.py around lines 58 to 61,
the code assumes dev.find("mtu") returns an element and directly accesses
mtu_element.attrib which will raise if mtu is missing; add a null check: if
mtu_element is None, create a new 'mtu' element under dev (e.g., using
ElementTree.SubElement or equivalent) and set its 'size' attribute to
params.get("updated_mtu"), otherwise update the existing
mtu_element.attrib['size'] with params.get("updated_mtu"); ensure the value is
converted to a string and handle missing updated_mtu by skipping or raising as
appropriate.
nanli1
force-pushed
the
add_case_for_updating_interface_mtu_during_migration
branch
from
a9f3247 to
002b190
Compare
This commit adds test cases for updating network interface MTU settings in guest XML configuration during live migration scenarios. The motivation is to ensure network configuration integrity during complex migration procedures, preventing connectivity issues that may arise from inconsistent MTU settings. The approach involves modifying MTU values at different migration stages and validating network functionality post-migration. Reference: xxxx-298951 Signed-off-by: nanli <[email protected]>
nanli1
force-pushed
the
add_case_for_updating_interface_mtu_during_migration
branch
from
002b190 to
8d285f5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
xxxx-298951:[network]change mtu settings of guest xml during migration
Signed-off-by: nanli [email protected]
Summary by CodeRabbit
New Features
Tests