[crac] RFR: Print better diagnostics for spawning CRIU [v2]

Anton Kozlov akozlov at openjdk.org
Wed Aug 9 20:01:28 UTC 2023

On Fri, 4 Aug 2023 06:44:25 GMT, Jan Kratochvil <jkratochvil at openjdk.org> wrote:

>> It was always a mystery why it does not work this time as there are no error messages printed.
>> Still may system call errors are currently not reported but those errors at least do not happen commonly.
>> In fact `os::exec_child_process_and_wait` should be more verbose but that is already a part of OpenJDK so I did not modify it.
>> It would be much easier if `criuengine.c` was in C++. I was considering to switch it but then I wrote it already in C.
>> I understand this code will become obsolete after planned integration of CRIU into JVM but it may not yet be soon enough.
> Jan Kratochvil has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
>  - print error messages in C++
>  - Merge branch 'crac' into crac-error
>  - Print better diagnostics for spawning CRIU.

OK, looks fine although a bit overcomplicated as for me.

src/java.base/unix/native/criuengine/criuengine.cpp line 91:

> 89:             }
> 90:         }
> 91:         retval += '\'';

It will be surprised to meet these symbols in the args, and IMHO this is overly complicated way to report them

src/java.base/unix/native/criuengine/criuengine.cpp line 109:

> 107:     free(cwd_s);
> 108:     return path_from(cwd, rel);
> 109: }

The problem with this function as abstraction is that it first calculates CWD, then checks was the argument absolute or relative.

src/java.base/unix/native/criuengine/criuengine.cpp line 199:

> 197:         if (WEXITSTATUS(status) != SUPPRESS_ERROR_IN_PARENT) {
> 198:             std::cerr << "Spawned CRIU \"" << join_args(args) << "\" has not properly exited:"
> 199:               " exit code " << WEXITSTATUS(status) << " - check " << path_abs(imagedir, log_local) << std::endl;

we are not required to abs(imagedir), as unprocessed that is in the same way as it was provided in the args


Marked as reviewed by akozlov (Lead).

PR Review: https://git.openjdk.org/crac/pull/97#pullrequestreview-1570417037
PR Review Comment: https://git.openjdk.org/crac/pull/97#discussion_r1289114649
PR Review Comment: https://git.openjdk.org/crac/pull/97#discussion_r1289111229
PR Review Comment: https://git.openjdk.org/crac/pull/97#discussion_r1289115491

More information about the crac-dev mailing list