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

Restore a thread to a correct v1 cgroup #2347

Closed

Conversation

StepanPieshkin
Copy link
Contributor

Currently we have checkpoint/restore support only of cgroup v2 threaded controllers. Threads originating in cgroup v1 environments will be restored to the main thread's cgroup. This change extends the support for a cgroups v1.

Copy link
Member

@minhbq-99 minhbq-99 left a comment

Choose a reason for hiding this comment

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

I wasn't aware that cgroup-v1 supports thread granularity when implementing the threaded controller support. Thanks for your PR, the PR looks good to me, I just leave some minor comments.

criu/cgroup.c Show resolved Hide resolved
test/zdtm/static/cgroup_threads.c Outdated Show resolved Hide resolved
Copy link
Contributor

@osctobe osctobe left a comment

Choose a reason for hiding this comment

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

I don't like the copy-pasting of the test helper code, but I guess cleaning that up is not in scope for this change. The added logic looks ok.

criu/cgroup.c Outdated Show resolved Hide resolved
test/zdtm/static/cgroup_threads.hook Show resolved Hide resolved
test/zdtm/static/cgroup_threads.hook Show resolved Hide resolved
test/zdtm/static/cgroup_threads.hook Outdated Show resolved Hide resolved
test/zdtm/static/cgroup_threads.hook Show resolved Hide resolved
test/zdtm/static/cgroup_threads.hook Show resolved Hide resolved
test/zdtm/static/cgroup_threads.hook Outdated Show resolved Hide resolved
criu/cgroup.c Show resolved Hide resolved
Currently we have checkpoint/restore support only of cgroup v2 threaded
controllers. Threads originating in cgroup v1 environments will be
restored to the main thread's cgroup. This change extends the support
for a cgroups v1.

Signed-off-by: Stepan Pieshkin <[email protected]>
Co-developed-by: Stepan Pieshkin <[email protected]>
Signed-off-by: Stepan Pieshkin <[email protected]>
Signed-off-by: Michal Clapinski <[email protected]>
#include <pthread.h>
#include "zdtmtst.h"

const char *test_doc = "Check that cgroup layout of threads is preserved";
Copy link
Member

@Snorch Snorch Feb 16, 2024

Choose a reason for hiding this comment

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

Note: clang-format is complaining about alignment here. s/\t/ / here and below.


int p1[2], pr[2];

void *child(void *args)
Copy link
Member

Choose a reason for hiding this comment

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

Misleading name for a thread, don't you think?

* then it reports the restore status (int).
*/

pipe(p1);
Copy link
Member

Choose a reason for hiding this comment

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

pls handler errors here and in other places.


if (mkdir(dirname, 0700) < 0) {
pr_perror("Can't make dir");
goto out;
Copy link
Member

Choose a reason for hiding this comment

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

it has to exit with a non-zero code in this case.

@avagin
Copy link
Member

avagin commented Feb 18, 2024

Both commits have been merged. Thanks!
1120308
d553fad

@avagin avagin closed this Feb 18, 2024
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.

6 participants