cgroup: add support for unified cgroup v2 #459
Conversation
|
@mrunalp PTAL |
|
Reviewing.. |
src/libcrun/cgroup.c
Outdated
| return crun_make_error (err, 0, "key `%s` must be a file name without any slash", resources->unified->keys[i]); | ||
|
|
||
| len = strlen (resources->unified->values[i]); | ||
| ret = write_file_at (cgroup_dirfd, resources->unified->keys[i], resources->unified->values[i], len, err); |
There was a problem hiding this comment.
How do we handle validation for the values?
There was a problem hiding this comment.
We could punt to the kernel entirely but we should ensure we are returning a clear error that we failed writing to unified cgroup value and advise user to check the kernel documentation for the accepted values for a setting.
There was a problem hiding this comment.
agree, I've added some logic to return more useful error messages when the controller is not present
|
Could we add any tests for this? |
src/libcrun/cgroup.c
Outdated
| { | ||
| size_t len; | ||
|
|
||
| if (strchr (resources->unified->keys[i], '/')) |
There was a problem hiding this comment.
We should map the keys to the known controllers as well and fail if it isn't known to the runtime.
There was a problem hiding this comment.
why would we need to do that? I think we can just write to the specified file (and that was one of the reasons for using the map in the OCI spec PR), since the key is the file name itself. In any case the OCI runtime is not doing any handling of this information
Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
sure. Added tests |
|
Can we have cgroup2 CI as in runc? https://github.com/opencontainers/runc/blob/2265daa55b594bea973e14bf1a22a7fe513ad334/.travis.yml#L24-L38 |
it allows to manually tweak the configuration for cgroup v2. we will expose some of the options in future as single options (e.g. the new memory knobs), but for now add the more generic --unified mechanism for maximum control on the cgroup configuration. OCI specs change: opencontainers/runtime-spec#1040 Requires: containers/crun#459 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
|
Support for Podman: containers/podman#7372 |
add support for: opencontainers/runtime-spec#1040 Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>
it allows to manually tweak the configuration for cgroup v2. we will expose some of the options in future as single options (e.g. the new memory knobs), but for now add the more generic --unified mechanism for maximum control on the cgroup configuration. OCI specs change: opencontainers/runtime-spec#1040 Requires: containers/crun#459 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
it allows to manually tweak the configuration for cgroup v2. we will expose some of the options in future as single options (e.g. the new memory knobs), but for now add the more generic --unified mechanism for maximum control on the cgroup configuration. OCI specs change: opencontainers/runtime-spec#1040 Requires: containers/crun#459 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
|
LGTM |
thanks for the hint. Yes I'll need to add CI for cgroup v2 (and also more tests :-) ) |
it allows to manually tweak the configuration for cgroup v2. we will expose some of the options in future as single options (e.g. the new memory knobs), but for now add the more generic --cgroup-unified-configuration mechanism for maximum control on the cgroup configuration. OCI specs change: opencontainers/runtime-spec#1040 Requires: containers/crun#459 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
|
if there are no blockers, let's move forward, we can always improve it later if needed |
it allows to manually tweak the configuration for cgroup v2. we will expose some of the options in future as single options (e.g. the new memory knobs), but for now add the more generic --cgroup-conf mechanism for maximum control on the cgroup configuration. OCI specs change: opencontainers/runtime-spec#1040 Requires: containers/crun#459 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
|
Yeah sounds good |
it allows to manually tweak the configuration for cgroup v2. we will expose some of the options in future as single options (e.g. the new memory knobs), but for now add the more generic --cgroup-conf mechanism for maximum control on the cgroup configuration. OCI specs change: opencontainers/runtime-spec#1040 Requires: containers/crun#459 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
it allows to manually tweak the configuration for cgroup v2. we will expose some of the options in future as single options (e.g. the new memory knobs), but for now add the more generic --cgroup-conf mechanism for maximum control on the cgroup configuration. OCI specs change: opencontainers/runtime-spec#1040 Requires: containers/crun#459 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
| int | ||
| read_all_file (const char *path, char **out, size_t *len, libcrun_error_t *err) | ||
| { | ||
| cleanup_close int fd = -1; |
it allows to manually tweak the configuration for cgroup v2. we will expose some of the options in future as single options (e.g. the new memory knobs), but for now add the more generic --cgroup-conf mechanism for maximum control on the cgroup configuration. OCI specs change: opencontainers/runtime-spec#1040 Requires: containers/crun#459 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
add support for: opencontainers/runtime-spec#1040
Signed-off-by: Giuseppe Scrivano giuseppe@scrivano.org