-
Notifications
You must be signed in to change notification settings - Fork 242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix the FileNotFoundError of get_model_features() #2052
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
""" | ||
Shared code for tests that need to get the libvirt version | ||
""" | ||
|
||
import re | ||
import logging | ||
|
||
from avocado.utils import process | ||
|
||
LIBVIRT_LIB_VERSION = 0 | ||
|
||
|
||
def version_compare(major, minor, update): | ||
""" | ||
Determine/use the current libvirt library version on the system | ||
and compare input major, minor, and update values against it. | ||
If the running version is greater than or equal to the input | ||
params version, then return True; otherwise, return False | ||
|
||
This is designed to handle upstream version comparisons for | ||
test adjustments and/or comparisons as a result of upstream | ||
fixes or changes that could impact test results. | ||
|
||
:param major: Major version to compare against | ||
:param minor: Minor version to compare against | ||
:param update: Update value to compare against | ||
:return: True if running version is greater than or | ||
equal to the input libvirt version | ||
""" | ||
global LIBVIRT_LIB_VERSION | ||
|
||
if LIBVIRT_LIB_VERSION == 0: | ||
try: | ||
regex = r'[Uu]sing\s*[Ll]ibrary:\s*[Ll]ibvirt\s*' | ||
regex += r'(\d+)\.(\d+)\.(\d+)' | ||
lines = process.run("virsh version").stdout_text.splitlines() | ||
for line in lines: | ||
mobj = re.search(regex, line) | ||
if bool(mobj): | ||
LIBVIRT_LIB_VERSION = int(mobj.group(1)) * 1000000 + \ | ||
int(mobj.group(2)) * 1000 + \ | ||
int(mobj.group(3)) | ||
break | ||
except (ValueError, TypeError, AttributeError): | ||
logging.warning("Error determining libvirt version") | ||
return False | ||
|
||
compare_version = major * 1000000 + minor * 1000 + update | ||
|
||
if LIBVIRT_LIB_VERSION >= compare_version: | ||
return True | ||
return False |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,6 +72,7 @@ | |
from virttest import utils_selinux | ||
from virttest import utils_disk | ||
from virttest import logging_manager | ||
from virttest import libvirt_version | ||
from virttest.staging import utils_koji | ||
from virttest.staging import service | ||
from virttest.xml_utils import XMLTreeFile | ||
|
@@ -4035,7 +4036,8 @@ def setup_remote(self): | |
|
||
def get_model_features(model_name): | ||
""" | ||
/usr/share/libvirt/cpu_map.xml defines all CPU models. | ||
libvirt-4.5.0 :/usr/share/libvirt/cpu_map.xml defines all CPU models. | ||
libvirt-5.0.0 :/usr/share/libvirt/cpu_map/ defines all CPU models. | ||
One CPU model is a set of features. | ||
This function is to get features of one specific model. | ||
|
||
|
@@ -4045,26 +4047,37 @@ def get_model_features(model_name): | |
""" | ||
features = [] | ||
conf = "/usr/share/libvirt/cpu_map.xml" | ||
conf_dir = "/usr/share/libvirt/cpu_map/" | ||
|
||
try: | ||
output = open(conf, 'r').read() | ||
root = ET.fromstring(output) | ||
# Find model | ||
for model_n in root.findall('arch/model'): | ||
if model_n.get('name') == model_name: | ||
model_node = model_n | ||
for feature in model_n.findall('feature'): | ||
features.append(feature.get('name')) | ||
break | ||
# Handle nested model | ||
nested_model = model_node.find('model') | ||
if nested_model is not None: | ||
nested_model_name = nested_model.get('name') | ||
for model_n in root.findall('arch/model'): | ||
if model_n.get('name') == nested_model_name: | ||
for feature in model_n.findall('feature'): | ||
features.append(feature.get('name')) | ||
break | ||
if not libvirt_version.version_compare(5, 0, 0): | ||
with open(conf, 'r') as output: | ||
root = ET.fromstring(output.read()) | ||
while True: | ||
# Find model in file /usr/share/libvirt/cpu_map.xml | ||
for model_n in root.findall('arch/model'): | ||
if model_n.get('name') == model_name: | ||
model_node = model_n | ||
for feature in model_n.findall('feature'): | ||
features.append(feature.get('name')) | ||
break | ||
# Handle nested model | ||
if model_node.find('model') is not None: | ||
model_name = model_node.find('model').get('name') | ||
continue | ||
else: | ||
break | ||
|
||
else: | ||
# Find model in dir /usr/share/libvirt/cpu_map | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please fix indentation |
||
filelist = os.listdir(conf_dir) | ||
for file_name in filelist: | ||
if model_name in file_name: | ||
with open(os.path.join(conf_dir, file_name), "r") as output: | ||
model = ET.fromstring(output.read()) | ||
for feature in model.findall("model/feature"): | ||
features.append(feature.get('name')) | ||
break | ||
except ET.ParseError as error: | ||
logging.warn("Configuration file %s has wrong xml format" % conf) | ||
raise | ||
|
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@balamuruhans
Thank you very much for helping review the code and pointing out my code errors !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The utils_misc.py file path is "~/avocado-vt/virttest/utils_misc.py"
The libvirt_version.py file path is "tp-libvirt/provider/ libvirt_version.py "
I can not import libvirt_version.version_compare() directly in this utils_misc.py.
Should I write a function to get libvirt version, or what can I do so that I can import the libvirt_version.version_compare() ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggested to move the code from
tp-libvirt/provider/libvirt_version.py
toavocado-vt/virttest/
and use it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dzhengfy @sathnaga request for your acknowlegement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yanghangliu @dzhengfy @balamuruhans looks like moving
libvirt_version.py
needs a change in many tests, for now we can copylibvirt_version.py
to virttest and use in this testcase and slowly change import in other testcases through different PR and once all movement happened, we can safely removetp-libvirt/provider/libvirt_version.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, once moving to virttest, we will have to send separate PR to
tp-libirt
to adopt it by changing all the testcases using it from virttest and remove the file inprovider