8378446: [CRaC] Replace libcrexec with libcriuengine#297
8378446: [CRaC] Replace libcrexec with libcriuengine#297rvansa wants to merge 4 commits intoopenjdk:cracfrom
Conversation
|
👋 Welcome back rvansa! A progress list of the required criteria for merging this PR into |
|
@rvansa This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
|
@rvansa this pull request can not be integrated into git checkout crexec_to_criuengine
git fetch https://git.openjdk.org/crac.git crac
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge crac"
git push |
openjdk
bot
added
merge-conflict
openjdk
bot
added
ready
| return; | ||
| if (is_vm_statically_linked()) { | ||
| log_warning(crac)("Cannot load CRaC engine API entrypoint '%s' from %s", resolved_engine_func, path); | ||
| // Maybe the deprecated short name was used, in that case find_engine did not amend the 'engine' |
There was a problem hiding this comment.
Maybe I miss a case where this may happen but shouldn't it be handled here: https://github.com/rvansa/crac/blob/807167d37ca7cf6e2b079d18a45a768e24478083/src/hotspot/share/runtime/crac_engine.cpp#L147?
There was a problem hiding this comment.
For static VM we take a shortcut as we cannot check the filesystem path: https://github.com/rvansa/crac/blob/807167d37ca7cf6e2b079d18a45a768e24478083/src/hotspot/share/runtime/crac_engine.cpp#L135
Yes, it is redundant, so I hope to remove this in 28 when we don't allow short engine names anymore.
| } | ||
| } | ||
|
|
||
| static int restorewait(void) { |
There was a problem hiding this comment.
Do I understand correctly that this is needed because CRIU's executable (which this helper replaces as the restoree's parent) does not handle signals and return values in the same manner?
There was a problem hiding this comment.
Correct, the process triggering restore might think that it can use the signals normally. This code can be traced back all the way to f25eb7b#diff-b768a59a1eadf0ad4a63bd5011db6952a5ecc7cbaa88e236f65798e38d42f117R12-R235
|
Failure on MacOS is in |
2 participants
After JDK-8376959 the only use of libcrexec is running CRIU; in fact the code already is quite CRIU-dependent. The point of this task is to stop pretending that libcrexec is generic, and move code from criuengine binary (now removed) into libcriuengine implementing the C/R API.
This removes smuggling of some parameters through environment variables and execution of the criuengine. We still require anexecuteable (now called
criuhelper) to become parent of the restored process, but this has significantly simplified implementation.Replacement of communication between the restoring and restored process through signals and temporary files is out of scope of this change.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/crac.git pull/297/head:pull/297$ git checkout pull/297Update a local copy of the PR:
$ git checkout pull/297$ git pull https://git.openjdk.org/crac.git pull/297/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 297View PR using the GUI difftool:
$ git pr show -t 297Using diff file
Download this PR as a diff file:
https://git.openjdk.org/crac/pull/297.diff
Using Webrev
Link to Webrev Comment