-
Notifications
You must be signed in to change notification settings - Fork 377
Revert "cgroup: do not create a sub-cgroup by default" #1870
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
Conversation
This reverts commit 262d6ac. This is a breaking change, we need a version bump. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Reviewer's GuideThis PR reverts the prior change that disabled default systemd sub-cgroup creation by restoring the "container" fallback in code and aligning the manpage and markdown documentation with the revived default behavior, necessitating a version bump. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this 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:
- Don’t forget to bump the project version (e.g. in configure.ac or NEWS) since this revert is a breaking change.
- The code now always returns "container"—you should detect cgroup v1 vs v2 and return an empty string for v1 to match the docs.
- The example path
/sys/fs/cgroup//system.slice/...
has a double slash and looks confusing—consider fixing it to show the intended layout.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Don’t forget to bump the project version (e.g. in configure.ac or NEWS) since this revert is a breaking change.
- The code now always returns "container"—you should detect cgroup v1 vs v2 and return an empty string for v1 to match the docs.
- The example path `/sys/fs/cgroup//system.slice/...` has a double slash and looks confusing—consider fixing it to show the intended layout.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
TMT tests failed. @containers/packit-build please check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This reverts commit 262d6ac.
This is a breaking change, we need a version bump.
Summary by Sourcery
Revert the change that disabled automatic creation of a systemd sub-cgroup by default, restoring the previous behavior where a "container" subgroup is generated if no custom subgroup is provided.
Enhancements: