From 4ff4960c4a8c6090627ee5a4232db682c62649a8 Mon Sep 17 00:00:00 2001 From: Amarpreet Singh Date: Tue, 9 Jul 2024 14:47:23 -0400 Subject: [PATCH] Address comments Co-authored-by: Tobi Ajila Co-authored-by: Jason Feng Co-authored-by: Keith W. Campbell Signed-off-by: Amarpreet Singh --- src/java.base/share/native/libjli/java.c | 136 ++++++++--------------- 1 file changed, 47 insertions(+), 89 deletions(-) diff --git a/src/java.base/share/native/libjli/java.c b/src/java.base/share/native/libjli/java.c index 4ceb21b2f86..477a76b204a 100644 --- a/src/java.base/share/native/libjli/java.c +++ b/src/java.base/share/native/libjli/java.c @@ -77,48 +77,6 @@ #define USE_STDERR JNI_TRUE /* we usually print to stderr */ #define USE_STDOUT JNI_FALSE -#if defined(J9VM_OPT_CRAC_SUPPORT) -/* The return codes indicating errors, successful execution, or specific conditions. */ -#define EXECVP_ERROR -1 -#define EXECVP_SUCCESS 0 -#define WAIT_INDEFINITELY 0 -#define MEMORY_ALLOCATION_ERROR 1 -#define OPTION_NAME_NOT_FOUND_ERROR 2 -#define OPTION_VALUE_NOT_FOUND_ERROR 3 -#define OPTION_VALUE_NOT_VALID_ERROR 4 -#define OPTION_VALUE_NOT_EXPECTED_ERROR 5 -#define COMMAND_OPTION_LENGTH_CALCULATION_ERROR 6 - -/* The error messages. */ -#define OPTION_VALUE_NOT_FOUND_ERROR_MESSAGE "The value of the command line option %s was not found." -#define OPTION_VALUE_NOT_VALID_ERROR_MESSAGE "The value of the command line option is not valid, option=%s, value=%s." -#define OPTION_VALUE_NOT_EXPECTED_ERROR_MESSAGE "The value of the command line option is not expected, option=%s, value=%s." -#define CHECKPOINT_DIRECTORY_ERROR_MESSAGE "Failed to get the CRIU checkpoint directory, error=%d." -#define LOG_LEVEL_ERROR_MESSAGE "Failed to get the CRIU log level, error=%d." -#define UNPRIVILEGED_MODE_ERROR_MESSAGE "Failed to get the CRIU unprivileged mode, error=%d." -#define LOG_FILE_ERROR_MESSAGE "Failed to get the CRIU log file, error=%d." -#define COMMAND_OPTION_LENGTH_CALCULATION_ERROR_MESSAGE "Failed to calculate the length of the command option, value=%s, format=%s." -#define COMMAND_OPTION_MEMORY_ALLOCATION_ERROR_MESSAGE "Failed to allocate memory for the command option, value=%s, format=%s." -#define RESTORE_FROM_CHECKPOINT_ERROR_MESSAGE "Failed to restore from checkpoint, error=%d." -#define RESTORE_CHILD_PROCESS_FAILED_ERROR_MESSAGE "The CRIU restore child process failed." - -/* The option names. */ -#define CRAC_RESTORE_FROM_OPTION_NAME "-XX:CRaCRestoreFrom" -#define LOG_LEVEL_OPTION_NAME "-Dopenj9.internal.criu.logLevel" -#define UNPRIVILEGED_MODE_OPTION_NAME "-Dopenj9.internal.criu.unprivilegedMode" -#define LOG_FILE_OPTION_NAME "-Dopenj9.internal.criu.logFile" - -/* The option formats. */ -#define LOG_LEVEL_OPTION_FORMAT "-v%d" -#define LOG_FILE_OPTION_FORMAT "--log-file=%s" - -/* The CRIU command options. */ -#define CRIU_COMMAND "criu" -#define CRIU_RESTORE_OPTION "restore" -#define CRIU_CHECKPOINT_DIRECTORY_OPTION "-D" -#define CRIU_SHELL_JOB_OPTION "--shell-job" -#define CRIU_UNPRIVILEGED_MODE_OPTION "--unprivileged" -#endif /* defined(J9VM_OPT_CRAC_SUPPORT) */ static jboolean printVersion = JNI_FALSE; /* print and exit */ static jboolean showVersion = JNI_FALSE; /* print but continue */ @@ -625,7 +583,7 @@ getCommandLineOptionValue(const char *optionName, int argc, char **argv, int *er } } if (!optionNameFound) { - *error = OPTION_NAME_NOT_FOUND_ERROR; + *error = -1; } return value; } @@ -641,13 +599,13 @@ static const char * getCheckpointDirectory(int argc, char **argv, int *error) { const char *checkpointDirectory = NULL; - const char *checkpointDirectoryPropertyValue = getCommandLineOptionValue(CRAC_RESTORE_FROM_OPTION_NAME, argc, argv, error); + const char *checkpointDirectoryPropertyValue = getCommandLineOptionValue("-XX:CRaCRestoreFrom", argc, argv, error); if (0 == *error) { if (NULL != checkpointDirectoryPropertyValue) { checkpointDirectory = checkpointDirectoryPropertyValue; } else { - JLI_ReportErrorMessage(OPTION_VALUE_NOT_FOUND_ERROR_MESSAGE, CRAC_RESTORE_FROM_OPTION_NAME); - *error = OPTION_VALUE_NOT_FOUND_ERROR; + JLI_ReportErrorMessage("The value of the command line option -XX:CRaCRestoreFrom was not found."); + *error = -2; } } return checkpointDirectory; @@ -661,7 +619,7 @@ getCheckpointDirectory(int argc, char **argv, int *error) static jboolean isCommandLineOptionFoundWithError(int error) { - return (OPTION_NAME_NOT_FOUND_ERROR != error) && (0 != error); + return (-1 != error) && (0 != error); } /** @@ -676,7 +634,7 @@ getLogLevel(int argc, char **argv, int *error) { int logLevelValue = 2; /* default */ const char *logLevelPropertyValue = NULL; - logLevelPropertyValue = getCommandLineOptionValue(LOG_LEVEL_OPTION_NAME, argc, argv, error); + logLevelPropertyValue = getCommandLineOptionValue("-Dopenj9.internal.criu.logLevel", argc, argv, error); if (!isCommandLineOptionFoundWithError(*error)) { const char *c = logLevelPropertyValue; if (NULL == c) { @@ -695,8 +653,8 @@ getLogLevel(int argc, char **argv, int *error) } } setLogLevelOptionValueNotValidError: - JLI_ReportErrorMessage(OPTION_VALUE_NOT_VALID_ERROR_MESSAGE, LOG_LEVEL_OPTION_NAME, logLevelPropertyValue); - *error = OPTION_VALUE_NOT_VALID_ERROR; + JLI_ReportErrorMessage("The value of the command line option is not valid, option=-Dopenj9.internal.criu.logLevel, value=%s.", logLevelPropertyValue); + *error = -1; done: return logLevelValue; } @@ -712,13 +670,13 @@ static jboolean isUnprivilegedModeOn(int argc, char **argv, int *error) { jboolean isUnprivilegedModeOn = JNI_FALSE; - const char *unprivilegedModePropertyValue = getCommandLineOptionValue(UNPRIVILEGED_MODE_OPTION_NAME, argc, argv, error); + const char *unprivilegedModePropertyValue = getCommandLineOptionValue("-Dopenj9.internal.criu.unprivilegedMode", argc, argv, error); if (0 == *error) { if (NULL == unprivilegedModePropertyValue) { isUnprivilegedModeOn = JNI_TRUE; } else { - JLI_ReportErrorMessage(OPTION_VALUE_NOT_EXPECTED_ERROR_MESSAGE, UNPRIVILEGED_MODE_OPTION_NAME, unprivilegedModePropertyValue); - *error = OPTION_VALUE_NOT_EXPECTED_ERROR; + JLI_ReportErrorMessage("The value of the command line option is not expected, option=-Dopenj9.internal.criu.unprivilegedMode, value=%s.", unprivilegedModePropertyValue); + *error = -1; } } return isUnprivilegedModeOn; @@ -735,13 +693,13 @@ static const char * getLogFile(int argc, char **argv, int *error) { const char *logFile = NULL; - const char *logFilePropertyValue = getCommandLineOptionValue(LOG_FILE_OPTION_NAME, argc, argv, error); + const char *logFilePropertyValue = getCommandLineOptionValue("-Dopenj9.internal.criu.logFile", argc, argv, error); if (0 == *error) { if (NULL != logFilePropertyValue) { logFile = logFilePropertyValue; } else { - JLI_ReportErrorMessage(OPTION_VALUE_NOT_FOUND_ERROR_MESSAGE, LOG_FILE_OPTION_NAME); - *error = OPTION_VALUE_NOT_FOUND_ERROR; + JLI_ReportErrorMessage("The value of the command line option -Dopenj9.internal.criu.logFile was not found."); + *error = -1; } } return logFile; @@ -753,65 +711,65 @@ getLogFile(int argc, char **argv, int *error) * @param[in] logLevel The log level for CRIU logging * @param[in] unprivilegedModeOn Indicates whether the unprivileged mode option is on * @param[in] logFile The log file option for CRIU - * @return EXECVP_SUCCESS if the execution of the 'criu restore' command succeeds, error code otherwise + * @return 0 if the execution of the 'criu restore' command succeeds, -1 otherwise */ static int restoreFromCheckpoint(const char *checkpointDirectory, int logLevel, jboolean unprivilegedModeOn, const char *logFile) { - int restoreStatus = EXECVP_SUCCESS; + int restoreStatus = 0; int length = -1; char *logLevelOption = NULL; char *logFileOption = NULL; int argc = 0; const char *argv[9] = { NULL }; - argv[argc++] = CRIU_COMMAND; - argv[argc++] = CRIU_RESTORE_OPTION; - argv[argc++] = CRIU_CHECKPOINT_DIRECTORY_OPTION; + argv[argc++] = "criu"; + argv[argc++] = "restore"; + argv[argc++] = "-D"; argv[argc++] = checkpointDirectory; - length = snprintf(NULL, 0, LOG_LEVEL_OPTION_FORMAT, logLevel); + length = snprintf(NULL, 0, "-v%d", logLevel); if (length < 0) { char logLevelString[2] = { 0 }; sprintf(logLevelString, "%d", logLevel); - JLI_ReportErrorMessage(COMMAND_OPTION_LENGTH_CALCULATION_ERROR_MESSAGE, logLevelString, LOG_LEVEL_OPTION_FORMAT); - restoreStatus = MEMORY_ALLOCATION_ERROR; + JLI_ReportErrorMessage("Failed to calculate the length of the command option, value=%s, format=%s.", logLevelString, "-v%d"); + restoreStatus = -1; goto done; } logLevelOption = (char *)JLI_MemAlloc(length + 1); if (NULL == logLevelOption) { char logLevelString[2] = { 0 }; sprintf(logLevelString, "%d", logLevel); - JLI_ReportErrorMessage(COMMAND_OPTION_MEMORY_ALLOCATION_ERROR_MESSAGE, logLevelString, LOG_LEVEL_OPTION_FORMAT); - restoreStatus = MEMORY_ALLOCATION_ERROR; + JLI_ReportErrorMessage("Failed to allocate memory for the command option, value=%s, format=%s.", logLevelString, "-v%d"); + restoreStatus = -1; goto done; } - if (snprintf(logLevelOption, length + 1, LOG_LEVEL_OPTION_FORMAT, logLevel) < 0) { + if (snprintf(logLevelOption, length + 1, "-v%d", logLevel) < 0) { char logLevelString[2] = { 0 }; sprintf(logLevelString, "%d", logLevel); - JLI_ReportErrorMessage(COMMAND_OPTION_MEMORY_ALLOCATION_ERROR_MESSAGE, logLevelString, LOG_LEVEL_OPTION_FORMAT); - restoreStatus = MEMORY_ALLOCATION_ERROR; + JLI_ReportErrorMessage("Failed to allocate memory for the command option, value=%s, format=%s.", logLevelString, "-v%d"); + restoreStatus = -1; goto freeLogLevelOption; } argv[argc++] = logLevelOption; - argv[argc++] = CRIU_SHELL_JOB_OPTION; + argv[argc++] = "--shell-job"; if (unprivilegedModeOn) { - argv[argc++] = CRIU_UNPRIVILEGED_MODE_OPTION; + argv[argc++] = "--unprivileged"; } if (NULL != logFile) { - length = snprintf(NULL, 0, LOG_FILE_OPTION_FORMAT, logFile); + length = snprintf(NULL, 0, "--log-file=%s", logFile); if (length < 0) { - JLI_ReportErrorMessage(COMMAND_OPTION_LENGTH_CALCULATION_ERROR_MESSAGE, logFile, LOG_FILE_OPTION_FORMAT); - restoreStatus = MEMORY_ALLOCATION_ERROR; + JLI_ReportErrorMessage("Failed to calculate the length of the command option, value=%s, format=%s.", logFile, "--log-file=%s"); + restoreStatus = -1; goto freeLogLevelOption; } logFileOption = (char *)JLI_MemAlloc(length + 1); if (NULL == logFileOption) { - JLI_ReportErrorMessage(COMMAND_OPTION_MEMORY_ALLOCATION_ERROR_MESSAGE, logFile, LOG_FILE_OPTION_FORMAT); - restoreStatus = MEMORY_ALLOCATION_ERROR; + JLI_ReportErrorMessage("Failed to allocate memory for the command option, value=%s, format=%s.", logFile, "--log-file=%s"); + restoreStatus = -1; goto freeLogLevelOption; } - if (snprintf(logFileOption, length + 1, LOG_FILE_OPTION_FORMAT, logFile) < 0) { - JLI_ReportErrorMessage(COMMAND_OPTION_MEMORY_ALLOCATION_ERROR_MESSAGE, logFile, LOG_FILE_OPTION_FORMAT); - restoreStatus = MEMORY_ALLOCATION_ERROR; + if (snprintf(logFileOption, length + 1, "--log-file=%s", logFile) < 0) { + JLI_ReportErrorMessage("Failed to allocate memory for the command option, value=%s, format=%s.", logFile, "--log-file=%s"); + restoreStatus = -1; goto freeLogFileOption; } argv[argc++] = logFileOption; @@ -819,7 +777,7 @@ restoreFromCheckpoint(const char *checkpointDirectory, int logLevel, jboolean un argv[argc] = NULL; execvp(argv[0], (char * const *)argv); /* If execvp returns, there was an error. */ - restoreStatus = EXECVP_ERROR; + restoreStatus = -1; freeLogFileOption: if (logFileOption != NULL) { JLI_MemFree((void *)logFileOption); @@ -853,11 +811,11 @@ handleCRaCRestore(int argc, char **argv) int childProcessPidStatus = 0; int childProcessPidExitStatus = 0; checkpointDirectory = getCheckpointDirectory(argc, argv, &error); - if (OPTION_NAME_NOT_FOUND_ERROR == error) { + if (-1 == error) { return; } - if (0 != error) { - JLI_ReportErrorMessage(CHECKPOINT_DIRECTORY_ERROR_MESSAGE, error); + if (-2 == error) { + JLI_ReportErrorMessage("Failed to get the CRIU checkpoint directory, error=%d.", error); parentProcessExitStatus = EXIT_FAILURE; goto doneParentProcess; } @@ -869,19 +827,19 @@ handleCRaCRestore(int argc, char **argv) if (0 == childProcessPid) { logLevel = getLogLevel(argc, argv, &error); if (isCommandLineOptionFoundWithError(error)) { - JLI_ReportErrorMessage(LOG_LEVEL_ERROR_MESSAGE, error); + JLI_ReportErrorMessage("Failed to get the CRIU log level, error=%d.", error); childProcessExitStatus = EXIT_FAILURE; goto doneChildProcess; } unprivilegedModeOn = isUnprivilegedModeOn(argc, argv, &error); if (isCommandLineOptionFoundWithError(error)) { - JLI_ReportErrorMessage(UNPRIVILEGED_MODE_ERROR_MESSAGE, error); + JLI_ReportErrorMessage("Failed to get the CRIU unprivileged mode, error=%d.", error); childProcessExitStatus = EXIT_FAILURE; goto doneChildProcess; } logFile = getLogFile(argc, argv, &error); if (isCommandLineOptionFoundWithError(error)) { - JLI_ReportErrorMessage(LOG_FILE_ERROR_MESSAGE, error); + JLI_ReportErrorMessage("Failed to get the CRIU log file, error=%d.", error); childProcessExitStatus = EXIT_FAILURE; goto doneChildProcess; } @@ -889,15 +847,15 @@ handleCRaCRestore(int argc, char **argv) doneChildProcess: exit(childProcessExitStatus); } else { - waitpid(childProcessPid, &childProcessPidStatus, WAIT_INDEFINITELY); + waitpid(childProcessPid, &childProcessPidStatus, 0); if (WIFEXITED(childProcessPidStatus)) { childProcessPidExitStatus = WEXITSTATUS(childProcessPidStatus); if (EXIT_SUCCESS != childProcessPidExitStatus) { - JLI_ReportErrorMessage(RESTORE_FROM_CHECKPOINT_ERROR_MESSAGE, childProcessPidExitStatus); + JLI_ReportErrorMessage("Failed to restore from checkpoint, error=%d.", childProcessPidExitStatus); parentProcessExitStatus = EXIT_FAILURE; } } else { - JLI_ReportErrorMessage(RESTORE_CHILD_PROCESS_FAILED_ERROR_MESSAGE); + JLI_ReportErrorMessage("The CRIU restore child process failed."); parentProcessExitStatus = EXIT_FAILURE; } }