Skip to content

Conversation

@jokemanfire
Copy link

@jokemanfire jokemanfire commented Aug 1, 2025

I don't think errors should be hidden here, it will cause the error to occur in the subsequent process, and I think this design is incorrect.
you can choose to ignore this error ,but should not hide it in inner.

  let cg = Cgroup::new(hierarchies::auto(), path).unwrap();
  let mem_controller: &MemController = cg.controller_of().unwrap();
  mem_controller.set_limit(10 * 1024 * 1024).unwrap(); // 10M

this will case the panic occur in set_limit,this is unreasonable.

@jokemanfire jokemanfire changed the title fix(Controller): Errors are hidden by defaul fix(Controller): Errors are hidden by default Aug 1, 2025
Copy link
Member

@Tim-Zhang Tim-Zhang left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @jokemanfire

@jokemanfire jokemanfire force-pushed the dev branch 2 times, most recently from 34ea544 to 9f398c7 Compare November 20, 2025 11:08
}

fn start_default_cgroup(pid: CgroupPid, unit: &str) -> SystemdClient {
fn start_default_cgroup(pid: CgroupPid, unit: &'_ str) -> SystemdClient<'_> {
Copy link
Member

Choose a reason for hiding this comment

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

Why add a <'_> here?

Copy link
Author

Choose a reason for hiding this comment

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

when I usemake check rustc 1.90 clippy put out this warning.

Copy link
Author

Choose a reason for hiding this comment

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

error: could not compile cgroups-rs (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
error: hiding a lifetime that's elided elsewhere is confusing
--> src/systemd/dbus/client.rs:251:51
|
251 | fn start_default_cgroup(pid: CgroupPid, unit: &str) -> SystemdClient {
| ^^^^ ------------- the same lifetime is hidden here
| |
| the lifetime is elided here
|
= help: the same lifetime is referred to in inconsistent ways, making the signature confusing
= note: -D mismatched-lifetime-syntaxes implied by -D warnings
= help: to override -D warnings add #[allow(mismatched_lifetime_syntaxes)]
help: use '_ for type paths
|
251 | fn start_default_cgroup(pid: CgroupPid, unit: &str) -> SystemdClient<'_> {

Fixed an error hidden during the creation of cgroup,
which resulted in subsequent error propagation

Signed-off-by: jokemanfire <[email protected]>
Copy link
Member

@justxuewei justxuewei left a comment

Choose a reason for hiding this comment

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

Lgtm, thanks!

@justxuewei justxuewei merged commit 6a11b32 into kata-containers:main Nov 20, 2025
6 checks passed
@jokemanfire jokemanfire deleted the dev branch November 20, 2025 11:20
Tim-Zhang added a commit to Tim-Zhang/cgroups-rs that referenced this pull request Nov 21, 2025
Bump major(minor actually) version for incompatible changes in kata-containers#154.

Changelog:
- kata-containers#152
- kata-containers#154
- kata-containers#159

Signed-off-by: Tim Zhang <[email protected]>
@Tim-Zhang Tim-Zhang mentioned this pull request Nov 21, 2025
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