-
Notifications
You must be signed in to change notification settings - Fork 168
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
add case for invalid dimm device config #5263
add case for invalid dimm device config #5263
Conversation
modular_redesign |
8c5fd47
to
eeeaa4c
Compare
83b6f4d
to
cf9ec7f
Compare
cf9ec7f
to
d7549c0
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.
Others LGTM
else: | ||
page_size = process.run(pagesize_cmd, ignore_status=True, | ||
shell=True).stdout_text.strip() | ||
return int(page_size) |
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.
Need to add docstring for return value.
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.
Thanks liping ,Updated
""" | ||
mem_obj = memory.Memory() | ||
mem_obj.setup_attrs(**eval(mem_dict)) | ||
return mem_obj |
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.
Same as above.
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.
Thanks liping ,Updated
def define_guest(test, params, page_size): | ||
""" | ||
Define guest with specific | ||
""" |
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.
Miss docstring.
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.
Thanks liping ,Updated
d7549c0
to
4857fc9
Compare
4857fc9
to
e3de46f
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.
Others LGTM
original_xml.setup_attrs(**vm_attrs) | ||
test.log.debug("Define vm by '%s' \n", original_xml) | ||
original_xml.sync() | ||
virsh.start(vm_name, debug=True) |
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.
ignore_status=False
is needed.
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.
Thanks dzheng, Updated
e3de46f
to
5a382c7
Compare
define_error = "Invalid value for attribute 'type' in element 'address': '${addr_type}'" | ||
aarch64: | ||
define_error = "unknown address type '${addr_type}'" | ||
addr_dict = "'address':{'attrs': {'type': '${addr_type}', 'base': '${addr_base}', 'slot': '${slot}'}}" |
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.
Maybe move those 3 lines to just follow up line 19
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.
Thanks chunfu, But the addr_dict
has some variables in the lines after L19 , such as ${addr_type},So I add addr_dict
here .
2.memory setting: with numa | ||
""" | ||
|
||
def setup_test(): |
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.
since previously most of method were moved out of scope of run(test, params, env).
Can setup_test(), run_test() and teardown_test() be moved out ? and just put above def run(test, params, env):
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.
Hi, chunfu , Thanks your advice. Have we confirmed we should do like that? I think both are OK.
VIRT-299046: Verify error messages prompt with invalid memory device configs Signed-off-by: nanli <[email protected]>
5a382c7
to
6fa5057
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.
LGTM
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.
lgtm
Signed-off-by: nanli [email protected]
RHEL9& RHEL 8 + x86
PASSED on aarch64+rhel9