Skip to content
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

libct/cgroups: switch to fscommon.{Read,Write}File #2605

Merged
merged 8 commits into from
Oct 5, 2020

Conversation

kolyshkin
Copy link
Contributor

This switches all code under libcontainer/cgroups that reads or writes cgroup files to use appropriate functions from fscommon. This used to be part of #2598.

Besides, it slightly refactors fs/cpuset.go code.

AkihiroSuda
AkihiroSuda previously approved these changes Sep 29, 2020
@kolyshkin
Copy link
Contributor Author

@thaJeztah PTAL

thaJeztah
thaJeztah previously approved these changes Oct 1, 2020
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@mrunalp
Copy link
Contributor

mrunalp commented Oct 1, 2020

Looks good but needs rebase now.

@kolyshkin
Copy link
Contributor Author

rebased

thaJeztah
thaJeztah previously approved these changes Oct 1, 2020
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

mrunalp
mrunalp previously approved these changes Oct 1, 2020
mrunalp
mrunalp previously approved these changes Oct 3, 2020
@kolyshkin
Copy link
Contributor Author

Rebased, addressed latest review comments.

@AkihiroSuda
Copy link
Member

needs rebase

This removes package dependency on cgroup, as following commits
make cgroup use fscommon, which would result in dependency cycle.

The code to find out memory cgroup root is not really needed,
as 99% of test envrionments will have it at /sys/fs/cgroup/memory.
If not, that means we're either on cgroupv2 or on some very custom
system, so just skip the test.

The code that checks if we're on cgroupv2 is replaced by the check
of the particular v1 control file.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
fscommon.WriteFile is added specifically to work with cgroup files,
and the error it returns does not need to be wrapped.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
fscommon's ReadFile and WriteFile are tailored to cgroupfs,
so let's use them here.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
While at it,

 - change some functions to not be methods of CpusetCgroup as
   they don't use any members;
 - simplify isEmpty.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor Author

@AkihiroSuda @mrunalp PTAL :)

@AkihiroSuda AkihiroSuda merged commit 0a9af39 into opencontainers:master Oct 5, 2020
kolyshkin pushed a commit to kolyshkin/runc that referenced this pull request Oct 6, 2020
libct/cgroups: switch to fscommon.{Read,Write}File

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants