Add CPU model auto-detection for Power systems for host-model and extend migration variants! - #6907
Open
Anushree-Mathur wants to merge 1 commit into
Open
Conversation
WalkthroughThe migration test configuration adds five new migration variants and their Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libvirt/tests/src/migration/migration_misc/migration_with_cpu_mode.py (1)
19-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer structured capability parsing over inline regex.
Parsing the raw capabilities string with a regex (and importing
reinside the function) is brittle. The framework'scapability_xml.CapabilityXMLexposes the host CPU model directly, which is more robust and avoids the broadexcept Exception(Ruff BLE001 at Line 30). Consider:♻️ Suggested approach
from virttest.libvirt_xml import capability_xml def get_host_cpu_model(): """Get host CPU model in lowercase.""" try: model = capability_xml.CapabilityXML().model if model: model = model.lower() test.log.info(f"Detected CPU model: {model}") return model except Exception as e: test.log.warning(f"Failed to detect CPU model: {e}") return NoneIf you keep the regex, at minimum move
import reto the module top.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libvirt/tests/src/migration/migration_misc/migration_with_cpu_mode.py` around lines 19 - 33, The get_host_cpu_model helper currently parses virsh.capabilities() with an inline regex and imports re inside the function, which is brittle and broadens error handling. Update get_host_cpu_model to use capability_xml.CapabilityXML().model directly, lowercasing and logging the detected value as before, and remove the regex-based parsing path; if you must keep the regex fallback, move the re import to module scope and avoid the broad except Exception by narrowing the handled failure where possible.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@libvirt/tests/src/migration/migration_misc/migration_with_cpu_mode.py`:
- Around line 19-33: The get_host_cpu_model helper currently parses
virsh.capabilities() with an inline regex and imports re inside the function,
which is brittle and broadens error handling. Update get_host_cpu_model to use
capability_xml.CapabilityXML().model directly, lowercasing and logging the
detected value as before, and remove the regex-based parsing path; if you must
keep the regex fallback, move the re import to module scope and avoid the broad
except Exception by narrowing the handled failure where possible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c0d07883-9afd-46b6-86c2-92dfdfbe0c17
📒 Files selected for processing (2)
libvirt/tests/cfg/migration/migration_misc/migration_with_cpu_mode.cfglibvirt/tests/src/migration/migration_misc/migration_with_cpu_mode.py
…end migration variants!
- Auto-detect host CPU model on ppc64/ppc64le architectures
- Convert CPU model to lowercase for libvirt compatibility (POWER11 → power11)
- Add CPU model to VM XML with fallback='forbid' for host-model mode
- Extend migration variants to include:
* p2p_tunneled: P2P with tunneled transport
* postcopy: Postcopy migration
* postcopy_p2p: Postcopy with P2P
* auto_converge: Auto-converge for slow migrations
* auto_converge_p2p: Auto-converge with P2P
- Change transport from TCP to SSH for easier setup
- Fix variable reference from ${migrate_dest_host} to ${remote_ip}
This provides comprehensive migration testing with CPU mode configurations
across 7 migration types, 2 XML options, and 2 CPU modes (28 test cases total).
Signed-off-by: Anushree-Mathur <anushree.mathur@linux.ibm.com>
Anushree-Mathur
force-pushed
the
migration_host_model
branch
from
June 26, 2026 13:08
e56d438 to
02b58a3
Compare
Contributor
Author
custom mode testcases I have removed from here because it is already being tested in other migration scenarios. Removed it only for ppc architecture. |
hholoubk
reviewed
Aug 20, 2026
| :param params: Dictionary with the test parameters | ||
| :param env: Dictionary with test environment. | ||
| """ | ||
| def get_host_cpu_model(): |
Collaborator
There was a problem hiding this comment.
this method is not necessary.
I understood there are two ways:
-
using virsh capability
from virttest.libvirt_xml import capability_xml # at beginnig of file
model = capability_xml.CapabilityXML().model -
using virsh domcapability
from virttest.libvirt_xml import domcapability_xml # at beginnig of file
model = domcapability_xml.DomCapabilityXML().get_hostmodel_name()
(just got it from AI .. I never needed to use this so far ... and no time to try :( )
also be aware both do not lowercase
I would go with the first one .. the second looks for me a little bit overhead)
| vm_attrs = eval(params.get('vm_attrs', '{}')) | ||
| arch = platform.machine() | ||
| if arch in ['ppc64', 'ppc64le'] and 'cpu' in vm_attrs: | ||
| host_model = get_host_cpu_model() |
Collaborator
There was a problem hiding this comment.
you can use avocado-vt methods here as mentioned earlier
| if cpu_mode == "host_model": | ||
| vm_attrs = eval(params.get('vm_attrs', '{}')) | ||
| arch = platform.machine() | ||
| if arch in ['ppc64', 'ppc64le'] and 'cpu' in vm_attrs: |
Collaborator
There was a problem hiding this comment.
thinking about different way how to configure this ... so it will be usable/extendable later
e.g. instead of depending on hardcoded archs. there can be parameter
"add_cpu_fallback" = "forbid" (or yes)
then you need
cpu_model_fallback = params.get('cpu_model_fallback')
if cpu_model_fallback:
.
.
vm_attrs['cpu']['fallback'] = cpu_model_fallback
then just in cfg file:
variants cpu_mode:
- host_model:
no s390x
ppc64le,ppc64:
cpu_model_fallback = "forbid"
numa_cell = "'numa_... etc
Collaborator
|
Hi @Anushree-Mathur . If I count properly for |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
This provides comprehensive migration testing with CPU mode configurations across 7 migration types, 2 XML options, and 1 CPU modes (14 test cases total).
Signed-off-by: Anushree-Mathur anushree.mathur@linux.ibm.com
Summary by CodeRabbit
New Features
Bug Fixes