Address comments

Co-authored-by: Tobi Ajila <[email protected]>
Co-authored-by: Jason Feng <[email protected]>
Co-authored-by: Keith W. Campbell <[email protected]>
Signed-off-by: Amarpreet Singh <[email protected]>

Author: 4 people

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,73 +711,73 @@ 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;
     }
     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,35 +827,35 @@ 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;
         }
         childProcessExitStatus = restoreFromCheckpoint(checkpointDirectory, logLevel, unprivilegedModeOn, logFile);
 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;
         }
     }