Skip to content

Conversation

giuseppe
Copy link
Member

@giuseppe giuseppe commented Aug 27, 2025

implement the change specified here:

opencontainers/runtime-spec#1289

Summary by Sourcery

Implement handling of the default CLOS ID per OCI runtime-spec: treat name '/' as the default clos, adjust file path resolution accordingly, and skip creation, validation, and destruction operations for it

Enhancements:

  • Add is_default_clos and get_resctrl_path helpers to detect and build resctrl paths for the default CLOS ID
  • Replace direct path concatenation with get_resctrl_path to handle default clos and normal clos uniformly
  • Skip creation, validation, update, and destruction operations when the CLOS ID is the default '/'

Copy link

sourcery-ai bot commented Aug 27, 2025

Reviewer's Guide

This PR introduces special handling for the default Intel RDT CLOS ID (“/”) by adding helper functions to detect and build paths accordingly, and updates all rdt-related operations to skip unnecessary file-system interactions for the default CLOS.

Class diagram for updated Intel RDT CLOS handling

classDiagram
    class IntelRDT {
        +compare_rdt_configurations(a, b)
        +validate_rdt_configuration(name, l3_cache_schema, mem_bw_schema, err)
        +resctl_create(name, explicit_clos_id, created, l3_cache_schema, mem_bw_schema, err)
        +resctl_move_task_to(name, pid, err)
        +resctl_update(name, l3_cache_schema, mem_bw_schema, schemata, err)
        +resctl_destroy(name, err)
    }
    class get_resctrl_path {
        +get_resctrl_path(path, file, name, err)
    }
    class is_default_clos {
        +is_default_clos(name)
    }
    IntelRDT --> get_resctrl_path : uses
    IntelRDT --> is_default_clos : uses
    get_resctrl_path --> is_default_clos : calls
Loading

File-Level Changes

Change Details Files
Added helper functions for default CLOS detection and path building
  • Introduce is_default_clos(name) to check for “/”
  • Implement get_resctrl_path() to route paths through default or named directories
src/libcrun/intelrdt.c
Replaced direct append_paths calls with get_resctrl_path
  • Use get_resctrl_path for SCHEMATA_FILE, TASKS_FILE, and base paths
  • Remove redundant append_paths invocations
src/libcrun/intelrdt.c
Added default-CLOS-specific logic to skip ops
  • In resctl_create: treat default CLOS as existing, skip validation and Dir creation
  • In resctl_destroy: return immediately for default CLOS
src/libcrun/intelrdt.c

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • Extract the default CLOS ID string "/" into a named constant (e.g., DEFAULT_CLOS_ID) instead of hardcoding it in is_default_clos.
  • Add a null or empty check for the name parameter in is_default_clos (or get_resctrl_path) to prevent potential null pointer dereferences.
  • Include a brief code comment or link to the OCI runtime-spec PR explaining why the default CLOS ID is represented as "/".
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the default CLOS ID string "/" into a named constant (e.g., DEFAULT_CLOS_ID) instead of hardcoding it in is_default_clos.
- Add a null or empty check for the `name` parameter in is_default_clos (or get_resctrl_path) to prevent potential null pointer dereferences.
- Include a brief code comment or link to the OCI runtime-spec PR explaining why the default CLOS ID is represented as "/".

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@giuseppe giuseppe force-pushed the handle-default-closeid branch from 5214e61 to 89a5afc Compare September 4, 2025 08:30
Copy link

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

implement the change specified here:

opencontainers/runtime-spec#1289

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe giuseppe force-pushed the handle-default-closeid branch from 89a5afc to 275305f Compare September 4, 2025 15:37
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