Skip to content

Vendor in go-criu and use it for CRIU's RPC definition#1963

Merged
mrunalp merged 2 commits intoopencontainers:masterfrom
adrianreber:go-criu
Feb 23, 2019
Merged

Vendor in go-criu and use it for CRIU's RPC definition#1963
mrunalp merged 2 commits intoopencontainers:masterfrom
adrianreber:go-criu

Conversation

@adrianreber
Copy link
Contributor

This PR vendors in CRIU's Go bindings. Right now this only replaces the manually updated CRIU RPC definition file. This PR does not change any functionality. It just deletes one file and replaces it with the same file from the go-criu repository.

Where possible we are planing to replace CRIU related runc code with functions provided by go-criu.

@rst0git
Copy link
Contributor

rst0git commented Jan 26, 2019

@adrianreber A commit that adds an interface for the --tcp-close option to RPC was merged in the criu-dev branch. Should we wait for the next CRIU release before adding support for this option in runc?

@adrianreber
Copy link
Contributor Author

@rst0git Concerning --tcp-close: I would make it dependent on if it is possible to detect if the installed CRIU supports it. Add a feature check via RPC to CRIU for --tcp-close, then update go-criu, the update runc. This way runc can always ask if CRIU supports --tcp-close via RPC, if CRIU supports runc can use it, independent if the feature (or interface) is only available in CRIU's development branch. Otherwise you would need to add a new version check to make sure runc can use the RPC option for --tcp-close. I think CRIU versions which do not know about --tcp-close via RPC would probably just ignore the option and the user thinks --tcp-close was active and runc passes it down to CRIU, but CRIU would just ignore it. Not really a perfect user experience... But as probably not many people would be using it anyway, so probably not really important.

@adrianreber
Copy link
Contributor Author

Any more questions concerning this PR? From my point of view it is ready to be merged.

Now that CRIU has released Go bindings, this commit vendors those in.

At first it only replaces the copy of RPC interface but the goal is to
use CRIU functions from the Go bindings instead of replicating the
functionality in runc.

Signed-off-by: Adrian Reber <areber@redhat.com>
This makes use of the vendored in Go bindings and removes the copy of
the CRIU RPC interface definition. runc now relies on go-criu for RPC
definition and hopefully more CRIU functions can be used in the future
from the CRIU Go bindings.

Signed-off-by: Adrian Reber <areber@redhat.com>
@adrianreber
Copy link
Contributor Author

Rebased, switch to tag v3.11 for criu in vendor.conf and force pushed.

@crosbymichael
Copy link
Member

crosbymichael commented Feb 14, 2019

LGTM

Approved with PullApprove

@adrianreber
Copy link
Contributor Author

Any further comments from other runc maintainers?

@mrunalp mrunalp merged commit 5b5130a into opencontainers:master Feb 23, 2019
@mrunalp
Copy link
Contributor

mrunalp commented Feb 23, 2019

LGTM

Approved with PullApprove

thaJeztah added a commit to thaJeztah/containerd that referenced this pull request Mar 7, 2019
This includes an improved fix for CVE-2019-5736 to reduce the
increased memory-consumption introduced by the original patch,
RHEL 7.6 getting into a loop due to a kernel bug in those kernels,
and improve compatibility with older kernels.

changes included:

- opencontainers/runc#1973 Vendor opencontainers/runtime-spec 29686dbc
- opencontainers/runc#1978 Remove detection for scope properties, which have always been broken
- opencontainers/runc#1963 Vendor in go-criu and use it for CRIU's RPC definition
- opencontainers/runc#1995 exec: expose --preserve-fds
- opencontainers/runc#2000 fix preserve-fds flag may cause runc hang
- opencontainers/runc#1968 Create bind mount mountpoints during restore
- opencontainers/runc#1984 nsenter: cloned_binary: "memfd" cleanups

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/containerd that referenced this pull request Mar 7, 2019
This includes an improved fix for CVE-2019-5736 to reduce the
increased memory-consumption introduced by the original patch,
RHEL 7.6 getting into a loop due to a kernel bug in those kernels,
and improve compatibility with older kernels.

changes included:

- opencontainers/runc#1973 Vendor opencontainers/runtime-spec 29686dbc
- opencontainers/runc#1978 Remove detection for scope properties, which have always been broken
- opencontainers/runc#1963 Vendor in go-criu and use it for CRIU's RPC definition
- opencontainers/runc#1995 exec: expose --preserve-fds
- opencontainers/runc#2000 fix preserve-fds flag may cause runc hang
- opencontainers/runc#1968 Create bind mount mountpoints during restore
- opencontainers/runc#1984 nsenter: cloned_binary: "memfd" cleanups

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit b8d40b3)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/containerd that referenced this pull request Mar 7, 2019
This includes an improved fix for CVE-2019-5736 to reduce the
increased memory-consumption introduced by the original patch,
RHEL 7.6 getting into a loop due to a kernel bug in those kernels,
and improve compatibility with older kernels.

changes included:

- opencontainers/runc#1973 Vendor opencontainers/runtime-spec 29686dbc
- opencontainers/runc#1978 Remove detection for scope properties, which have always been broken
- opencontainers/runc#1963 Vendor in go-criu and use it for CRIU's RPC definition
- opencontainers/runc#1995 exec: expose --preserve-fds
- opencontainers/runc#2000 fix preserve-fds flag may cause runc hang
- opencontainers/runc#1968 Create bind mount mountpoints during restore
- opencontainers/runc#1984 nsenter: cloned_binary: "memfd" cleanups

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit b8d40b3)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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.

4 participants