Skip to content

F #228: Add cputune_shares setting for FE VMs#233

Open
sk4zuzu wants to merge 1 commit into
masterfrom
f-228
Open

F #228: Add cputune_shares setting for FE VMs#233
sk4zuzu wants to merge 1 commit into
masterfrom
f-228

Conversation

@sk4zuzu

@sk4zuzu sk4zuzu commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: Michal Opala <sk4zuzu@gmail.com>
@sk4zuzu
sk4zuzu requested review from dann1 and tinova June 19, 2026 10:53
@tinova tinova linked an issue Jun 24, 2026 that may be closed by this pull request
@dann1

dann1 commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Hi @sk4zuzu 👋

Can cputune_shares_max be hardcoded ? It is something that the user can't change really. Since we don't want to support legacy things, we can use the cgv2 cap.

@dann1

dann1 commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Also, not related to these changes, but related to the role in question. Looks like ETH0_SEARCH_DOMAIN is mandatory now. Not setting it yields an error while generating the context.sh contents

TASK [opennebula.deploy.infra : Create context.sh files] *************************************************************************************************************************
Tuesday 07 July 2026  17:00:51 +0200 (0:00:00.315)       0:00:08.304 **********
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ansible.errors.AnsibleUndefinedVariable: 'dict object' has no attribute 'ETH0_SEARCH_DOMAIN'. 'dict object' has no attribute 'ETH0_SEARCH_DOMAIN'
failed: [i1] (item=0) =>
    ansible_loop_var: item
    changed: false
    item: 0
    msg: 'AnsibleUndefinedVariable: ''dict object'' has no attribute ''ETH0_SEARCH_DOMAIN''.
        ''dict object'' has no attribute ''ETH0_SEARCH_DOMAIN'''

Can this behavior be changed so it becomes optional, or has a default value, or an assertion, to avoid an exception. I'd say it should be optional, as it was before #173. The documentation of the parameter looks like copy pasted from DNS. Should be updated accordingly.

@dann1

dann1 commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Maybe a review of these previous changes is needed, the dpdk socket path, could be made transparent now, for example. And the queues of that nic is set to static 2, could be tied to vcpu count instead.

@sk4zuzu

sk4zuzu commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

@dann1 Yes, definitely, previous changes where more about how to combine OVS with FE VMs, so some things may be not optimal. Actually I'm working on bigger refactor of the whole role (from the templates to structures) and ability to run those FE VMs on isolated cpu and hugepages as a requirement. I think let's postpone this PR a bit. 🤗

@dann1 dann1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If the nitpick about hardcoding the shares cap is deemed unnecessary, then LGTM

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.

CPU time priority for frontend VM

2 participants