Skip to content

Comments

linux: honor execCPUAffinity from main container config when missing in exec process#1961

Closed
anonymous0719 wants to merge 1 commit intocontainers:mainfrom
anonymous0719:crun-pr1
Closed

linux: honor execCPUAffinity from main container config when missing in exec process#1961
anonymous0719 wants to merge 1 commit intocontainers:mainfrom
anonymous0719:crun-pr1

Conversation

@anonymous0719
Copy link

Hello, maintains. We’ve been developing a tool that scans downstream forks to find valuable commits that haven’t been merged back upstream, and this change was one of the best candidates we found.

This PR brings in a fix to ensure execCPUAffinity is honored from the container’s main configuration when it is not explicitly specified in the exec process configuration.

It also adds a regression test to validate the fallback behavior.

Closes: https://issues.redhat.com/browse/OCPBUGS-65579

We hope this contribution is helpful for the project.

If you have any feedback on whether this is useful (or suggestions on how our tool could better surface upstream-worthy commits), we’d really appreciate your input.

@gemini-code-assist
Copy link

Summary of Changes

Hello @anonymous0719, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses an issue where execCPUAffinity settings were not consistently applied when an exec process lacked its own explicit configuration. The changes introduce a fallback mechanism to honor the execCPUAffinity defined at the main container level, ensuring proper CPU resource allocation for exec processes. A new test case validates this behavior, enhancing the robustness of the crun runtime.

Highlights

  • CPU Affinity Fallback: Implemented logic to ensure that if execCPUAffinity is not explicitly defined for an exec process, the system will fall back to using the execCPUAffinity configured for the main container.
  • Refactored CPU Affinity Check: The has_exec_cpu_affinity helper function was refactored to directly accept the exec_cpu_affinity structure, simplifying its usage and improving clarity.
  • Regression Test Added: A new Python-based regression test (test_exec_cpu_affinity_config_file) has been introduced to validate the correct fallback behavior of execCPUAffinity from the container's main configuration.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly implements the fallback logic for execCPUAffinity, ensuring that the main container's configuration is used when it's not specified for an exec process. The changes in src/libcrun/linux.c are clear and safe. The addition of test_exec_cpu_affinity_config_file in tests/test_exec.py provides good test coverage for the new behavior. I have one suggestion to improve the test code by reducing duplication. Overall, this is a valuable contribution.

Comment on lines +493 to +498
def cpu_mask_from_proc_status(status):
for l in status.split("\n"):
parts = l.split(":")
if parts[0] == "Cpus_allowed_list":
return parts[1].strip()
return ""

Choose a reason for hiding this comment

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

medium

This helper function cpu_mask_from_proc_status is also defined in the test_exec_cpu_affinity test. To improve maintainability and avoid code duplication, consider moving this function to the module level so it can be shared by both tests.

@packit-as-a-service
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@packit-as-a-service
Copy link

TMT tests failed. @containers/packit-build please check.

@giuseppe giuseppe closed this Feb 3, 2026
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.

2 participants