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

qemu_vm: includes support for mem policy and host-nodes #3792

Merged

Conversation

mcasquer
Copy link
Contributor

@mcasquer mcasquer commented Nov 7, 2023

qemu_vm: includes support for mem policy and host-nodes

Includes the support of the policy and host-nodes options
for the machine memory backend.

ID: 1398
Signed-off-by: mcasquer [email protected]

@mcasquer
Copy link
Contributor Author

mcasquer commented Nov 7, 2023

After applying the avocado-vt patch

[root@dell-per750-22 avocado-vt]# git am 3792.patch
Applying: qemu_vm: includes support for mem policy and host-nodes

Now the machine memory includes the policy and host-nodes option

[stdlog] MALLOC_PERTURB_=1 numactl \
[stdlog]     -m 1   /usr/libexec/qemu-kvm \
[stdlog]     -machine q35,pflash0=drive_ovmf_code,pflash1=drive_ovmf_vars,memory-backend=mem-machine_mem \
...
[stdlog]     -object '{"size": 4294967296, "policy": "bind", "host-nodes": [1], "id": "mem-machine_mem", "qom-type": "memory-backend-ram"}'  \

@mcasquer mcasquer marked this pull request as ready for review November 7, 2023 09:56
@mcasquer
Copy link
Contributor Author

mcasquer commented Nov 7, 2023

@yanan-fu could you review this PR? Thanks !

@yanan-fu
Copy link
Contributor

yanan-fu commented Nov 9, 2023

This patch aim to add new options for the machine mem object, so it can bind the machine mem to the specified host numa node.
@YongxueHong @luckyh Could you please help review ?

if params.get("vm_mem_policy"):
backend_options["policy_mem"] = params.get("vm_mem_policy")
if params.get("vm_mem_host-nodes"):
backend_options["host-nodes"] = params.get("vm_mem_host-nodes")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
backend_options["host-nodes"] = params.get("vm_mem_host-nodes")
backend_options["host-nodes"] = params.get("vm_mem_host_nodes")

To align with the current design of the format parameters, I suggest using the underline to split the different keywords.
Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@YongxueHong I understand line 1102 needs to be vm_mem_host_nodes as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you review again this PR? Thanks!

Includes the support of the policy and host-nodes options
for the machine memory backend.

Signed-off-by: mcasquer <[email protected]>
@mcasquer mcasquer force-pushed the 1398_machine_mem_support branch from 8421dd7 to 5c428c2 Compare November 13, 2023 08:29
Copy link
Contributor

@YongxueHong YongxueHong left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@yanan-fu yanan-fu left a comment

Choose a reason for hiding this comment

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

Ack

@mcasquer
Copy link
Contributor Author

It seems this PR could be merged 😀

@YongxueHong YongxueHong merged commit b2324bc into avocado-framework:master Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants