Enable checkpoint/restore support (finally) #480
Conversation
CRIU can either freeze/pause all processes in the container by going through a loop and using ptrace (default) or use the cgroup freezer to pause all processes in a container. This changes crun to tell CRIU the information about the used cgroup freezer. This needs the not yet released CRIU 3.15 which should be released in the next weeks. Signed-off-by: Adrian Reber <areber@redhat.com>
With this change the Podman checkpoint/restore test suites passes on crun based systems. With this CRIU can tell us the console socket of the restored process which can then be passed to the upper layer (Podman). Signed-off-by: Adrian Reber <areber@redhat.com>
Now that all functionality for the Podman checkpoint/restore test suite has been implemented in crun, the 'checkpoint' and 'restore' commands are no longer hidden (with the '_' prefix), but enabled to be used. Checkpoint/restore support will not show up in crun unless CRIU 3.15 is available (which has not yet been released). Signed-off-by: Adrian Reber <areber@redhat.com>
|
Not sure if the test |
I've restarted them and they are green now. Thanks for working on it! The patch LGTM, if you are fine with it we can merge it. |
|
Removed draft status. This means that #71 could be closed. We could wait until CRIU 3.15 has been released and until I have made sure the corresponding CI tests are running to make sure we have test coverage. |
|
Where are we on this one now? |
|
@adrianreber @giuseppe Do we want to get this in? |
|
still waiting for CRIU to release 3.15. Once this is in, I think we can release crun 1.0 |
| static int | ||
| criu_notify (char *action, criu_notify_arg_t na) | ||
| { | ||
| if (strncmp (action, "orphan-pts-master", 17) == 0) |
There was a problem hiding this comment.
@adrianreber do you think that inverting the logic of this check would make the function look better? For example:
if (strncmp (action, "orphan-pts-master", 17) != 0)
return 0;
// function flow proceeds...
This way, the entire logic doesn't need to be nested. And it's similar to what is done within the if when checking console_socket, console_socket_fd, and ret.
There was a problem hiding this comment.
Usually I try to structure the code like you suggested. As the notify function potentially can handle different callback types it does not sound useful once there are multiple callbacks. Right now, however, I can changed the code flow. You are right. I will do that as I want to include additional tests for this PR anyway.
|
CRIU 3.15 has been prepared, the Makefile has been updated with the 3.15 version and I was told that the release will be tomorrow. I will make sure that the packages are available in Fedora and PPA as soon as possible to be able to update the CI to use 3.15. |
|
now that ciu 3.15 is in testing, let's merge. Thanks again for working on it! |
|
I am just working on a PR to enable checkpoint/restore testing in Travis because that test needs to run as root and is currently skipped. |
This pull requests adds the missing pieces of crun checkpoint/restore and enables it.
Missing pieces were, from my point of view, to make Podman's checkpoint/restore tests successful. With this Podman's checkpoint restore finishes successfully:
The changes in this PR to enable this are:
checkpointandrestorecommand which were hidden and prefixed with_(as long as CRIU >= 3.15 is not detected during build-time, the subcommands will stay deactivated.)
Once CRIU 3.15 is released I plan to update crun's CI to make sure the checkpoint/restore tests are being run. Once crun with CRIU 3.15 has been built and once it hits Podman's CI the checkpoint/restore tests on crun based systems will be running automatically.
Just a draft to see what crun's CI thinks of this change.