libct/cg/sd/v1: do not update non-frozen cgroup after frozen failed.#3804
Conversation
jiusanzhou
force-pushed
the
bugfix/skip-update-while-frozen-faield
branch
from
68168b7 to
fad60b8
Compare
kolyshkin
left a comment
kolyshkin
left a comment
There was a problem hiding this comment.
Overall the fix makes sense to me -- we should not update a non-frozen container.
Will take a deeper look later.
jiusanzhou
force-pushed
the
bugfix/skip-update-while-frozen-faield
branch
from
6bacdf5 to
446bf56
Compare
|
@jiusanzhou you need to Update: ah, I see you've rebased already. |
|
For the commit subject, I suggest something like
For the description, you can get some text from #3803 to describe what happens and then go on to say how the commit is fixing this. |
jiusanzhou
force-pushed
the
bugfix/skip-update-while-frozen-faield
branch
2 times, most recently
from
ac81a17 to
53a8bd4
Compare
|
@kolyshkin thank you for your advice, CI checking is green now. Please take another look. |
jiusanzhou
force-pushed
the
bugfix/skip-update-while-frozen-faield
branch
from
53a8bd4 to
cbfa85b
Compare
|
@cyphar PTAL |
jiusanzhou
force-pushed
the
bugfix/skip-update-while-frozen-faield
branch
from
cbfa85b to
01caf55
Compare
jiusanzhou
changed the title
jiusanzhou
force-pushed
the
bugfix/skip-update-while-frozen-faield
branch
from
01caf55 to
ad9e9b9
Compare
|
any update？ |
kolyshkin
added
backport/1.1-todo
In code we have frozen the cgroup to avoid the processes get an occasional "permission denied" error, while the systemd's application of device rules is done disruptively. When the processes in the container can not be frozen over 2 seconds (which defined in fs/freezer.go), we still update the cgroup which resulting the container get an occasional "permission denied" error in some cases. Return error directly without updating cgroup, when freeze fails. Fixes: opencontainers#3803 Signed-off-by: Zoe <hi@zoe.im>
jiusanzhou
force-pushed
the
bugfix/skip-update-while-frozen-faield
branch
from
ad9e9b9 to
62963fe
Compare
|
@opencontainers/runc-maintainers PTAL |
|
Can we have a test? |
I'm not sure we can. To test the branch of code being added, we need cgroup freeze to fail, and I don't know how to do that reliably. |
|
@jiusanzhou can you open a PR against |
|
1.1 backport: #3921 |
kolyshkin
added
backport/1.1-done
4 participants
In code we have frozen the cgroup to avoid the processes get
an occasional "permission denied" error, while the systemd's application of device
rules is done disruptively. When the processes in the container can not
be frozen over 2 seconds (which defined in fs/freezer.go),
we still update the cgroup which resulting the container get an occasional
"permission denied" error in some cases.
This PR make sure to return error directly without updating cgroup, when freeze fails.
Fixes: #3803