Skip to content
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

libvirt_numa: Fix parsing of continuous numa nodes #3883

Merged
merged 4 commits into from
May 17, 2024

Conversation

QianJianhua1
Copy link

@QianJianhua1 QianJianhua1 commented Mar 29, 2024

In PR#3804, a similar bug is fixed by replacing parse_numa_nodeset_to_str with the new convert_all_nodes_to_string function. However, parse_numa_nodeset_to_str is still called at several places. Therefore, in this PR, directly call convert_all_nodes_to_string in parse_numa_nodeset_to_str to fix the bug with minimal changes.

Signed-off-by: Qian Jianhua [email protected]

Before:

 (1/1) type_specific.io-github-autotest-libvirt.guest_numa_node_tuning.auto_memory_nodeset_placement.without_iothread.nodeset_defined.mem_mode_interleave.vcpu_auto: FAIL: Expect cpuset.mems=0-1, but found 0-3 (56.75 s)

After:

 (1/1) type_specific.io-github-autotest-libvirt.guest_numa_node_tuning.auto_memory_nodeset_placement.without_iothread.nodeset_defined.mem_mode_interleave.vcpu_auto: PASS (48.46 s)

In PR#3804, a similar bug is fixed by replacing parse_numa_nodeset_to_str
with the new convert_all_nodes_to_string function.
However, parse_numa_nodeset_to_str is still called at several places.
Therefore, in this PR, directly call convert_all_nodes_to_string
in parse_numa_nodeset_to_str to fix the bug with minimal changes.

Signed-off-by: Qian Jianhua <[email protected]>
@QianJianhua1
Copy link
Author

@dzhengfy Could you please review this PR for me?

@Yingshun Yingshun requested a review from dzhengfy May 14, 2024 09:37
)

LOG.debug("Parse output for numa nodeset: '%s'", numa_nodeset)
numa_nodeset = convert_all_nodes_to_string(node_list)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If convert_all_nodes_to_string() could completely replace the old function, we'd better replace it completely, which is to replace in tp-libvirt instead of keeping old function as a shell. Thanks

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I have removed parse_numa_nodeset_to_str(). And add a PR for tp-libvirt autotest/tp-libvirt#5616. Please review it again.

Copy link
Contributor

@dzhengfy dzhengfy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dzhengfy dzhengfy merged commit b24e150 into avocado-framework:master May 17, 2024
50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants