Merge pull request #3607 from rhc54/topic/server

Ensure that data from a job that was stored in ompi-server is purged once that job completes. Cleanup a few typos. Silence a Coverity warning

Author: Ralph Castain

orte/mca/odls/base/odls_base_default_fns.c

@@ -485,8 +485,7 @@ int orte_odls_base_default_construct_child_list(opal_buffer_t *buffer,
         }
     }
 
-    if (!ORTE_PROC_IS_HNP &&
-        !orte_get_attribute(&jdata->attributes, ORTE_JOB_FULLY_DESCRIBED, NULL, OPAL_BOOL)) {
+    if (!orte_get_attribute(&jdata->attributes, ORTE_JOB_FULLY_DESCRIBED, NULL, OPAL_BOOL)) {
         /* compute and save bindings of local children */
         if (ORTE_SUCCESS != (rc = orte_rmaps_base_compute_bindings(jdata))) {
             ORTE_ERROR_LOG(rc);
@@ -535,11 +534,8 @@ int orte_odls_base_default_construct_child_list(opal_buffer_t *buffer,
 
 static int setup_path(orte_app_context_t *app, char **wdir)
 {
-    int rc;
+    int rc=ORTE_SUCCESS;
     char dir[MAXPATHLEN];
-    char **argvptr;
-    char *pathenv = NULL, *mpiexec_pathenv = NULL;
-    char *full_search;
 
     if (!orte_get_attribute(&app->attributes, ORTE_APP_SSNDIR_CWD, NULL, OPAL_BOOL)) {
         /* Try to change to the app's cwd and check that the app
@@ -573,40 +569,6 @@ static int setup_path(orte_app_context_t *app, char **wdir)
         *wdir = NULL;
     }
 
-    /* Search for the OMPI_exec_path and PATH settings in the environment. */
-    for (argvptr = app->env; *argvptr != NULL; argvptr++) {
-        if (0 == strncmp("OMPI_exec_path=", *argvptr, 15)) {
-            mpiexec_pathenv = *argvptr + 15;
-        }
-        if (0 == strncmp("PATH=", *argvptr, 5)) {
-            pathenv = *argvptr + 5;
-        }
-    }
-
-    /* If OMPI_exec_path is set (meaning --path was used), then create a
-       temporary environment to be used in the search for the executable.
-       The PATH setting in this temporary environment is a combination of
-       the OMPI_exec_path and PATH values.  If OMPI_exec_path is not set,
-       then just use existing environment with PATH in it.  */
-    if (NULL != mpiexec_pathenv) {
-        argvptr = NULL;
-        if (pathenv != NULL) {
-            asprintf(&full_search, "%s:%s", mpiexec_pathenv, pathenv);
-        } else {
-            asprintf(&full_search, "%s", mpiexec_pathenv);
-        }
-        opal_setenv("PATH", full_search, true, &argvptr);
-        free(full_search);
-    } else {
-        argvptr = app->env;
-    }
-
-    rc = orte_util_check_context_app(app, argvptr);
-    /* do not ERROR_LOG - it will be reported elsewhere */
-    if (NULL != mpiexec_pathenv) {
-        opal_argv_free(argvptr);
-    }
-
  CLEANUP:
     return rc;
 }
@@ -663,6 +625,9 @@ void orte_odls_base_spawn_proc(int fd, short sd, void *cbdata)
     int rc, i;
     bool found;
     orte_proc_state_t state;
+    char **argvptr;
+    char *pathenv = NULL, *mpiexec_pathenv = NULL;
+    char *full_search;
 
     /* thread-protect common values */
     cd->env = opal_argv_copy(app->env);
@@ -694,6 +659,54 @@ void orte_odls_base_spawn_proc(int fd, short sd, void *cbdata)
         child->rml_uri = NULL;
     }
 
+    /* setup the rest of the environment with the proc-specific items - these
+     * will be overwritten for each child
+     */
+    if (ORTE_SUCCESS != (rc = orte_schizo.setup_child(jobdat, child, app, &cd->env))) {
+        ORTE_ERROR_LOG(rc);
+        state = ORTE_PROC_STATE_FAILED_TO_LAUNCH;
+        goto errorout;
+    }
+
+    /* Search for the OMPI_exec_path and PATH settings in the environment. */
+    for (argvptr = app->env; *argvptr != NULL; argvptr++) {
+        if (0 == strncmp("OMPI_exec_path=", *argvptr, 15)) {
+            mpiexec_pathenv = *argvptr + 15;
+        }
+        if (0 == strncmp("PATH=", *argvptr, 5)) {
+            pathenv = *argvptr + 5;
+        }
+    }
+
+    /* If OMPI_exec_path is set (meaning --path was used), then create a
+       temporary environment to be used in the search for the executable.
+       The PATH setting in this temporary environment is a combination of
+       the OMPI_exec_path and PATH values.  If OMPI_exec_path is not set,
+       then just use existing environment with PATH in it.  */
+    if (NULL != mpiexec_pathenv) {
+        argvptr = NULL;
+        if (pathenv != NULL) {
+            asprintf(&full_search, "%s:%s", mpiexec_pathenv, pathenv);
+        } else {
+            asprintf(&full_search, "%s", mpiexec_pathenv);
+        }
+        opal_setenv("PATH", full_search, true, &argvptr);
+        free(full_search);
+    } else {
+        argvptr = app->env;
+    }
+
+    rc = orte_util_check_context_app(app, argvptr);
+    /* do not ERROR_LOG - it will be reported elsewhere */
+    if (NULL != mpiexec_pathenv) {
+        opal_argv_free(argvptr);
+    }
+    if (ORTE_SUCCESS != rc) {
+        opal_output(0, "%s:%d", __FILE__, __LINE__);
+        state = ORTE_PROC_STATE_FAILED_TO_LAUNCH;
+        goto errorout;
+    }
+
     /* did the user request we display output in xterms? */
     if (NULL != orte_xterm && !ORTE_FLAG_TEST(jobdat, ORTE_JOB_FLAG_DEBUGGER_DAEMON)) {
         opal_list_item_t *nmitem;
@@ -754,15 +767,6 @@ void orte_odls_base_spawn_proc(int fd, short sd, void *cbdata)
         cd->argv = opal_argv_copy(app->argv);
     }
 
-    /* setup the rest of the environment with the proc-specific items - these
-     * will be overwritten for each child
-     */
-    if (ORTE_SUCCESS != (rc = orte_schizo.setup_child(jobdat, child, app, &cd->env))) {
-        ORTE_ERROR_LOG(rc);
-        state = ORTE_PROC_STATE_FAILED_TO_LAUNCH;
-        goto errorout;
-    }
-
     /* if we are indexing the argv by rank, do so now */
     if (cd->index_argv && !ORTE_FLAG_TEST(jobdat, ORTE_JOB_FLAG_DEBUGGER_DAEMON)) {
         char *param;

orte/mca/rmaps/round_robin/rmaps_rr_mappers.c

@@ -359,7 +359,7 @@ int orte_rmaps_rr_bynode(orte_job_t *jdata,
                     return ORTE_ERR_OUT_OF_RESOURCE;
                 }
                 nprocs_mapped++;
-            orte_set_attribute(&proc->attributes, ORTE_PROC_HWLOC_LOCALE, ORTE_ATTR_LOCAL, obj, OPAL_PTR);
+                orte_set_attribute(&proc->attributes, ORTE_PROC_HWLOC_LOCALE, ORTE_ATTR_LOCAL, obj, OPAL_PTR);
             }
             /* not all nodes are equal, so only set oversubscribed for
              * this node if it is in that state

orte/mca/schizo/ompi/schizo_ompi.c

@@ -1207,6 +1207,11 @@ static int setup_child(orte_job_t *jdata,
         opal_setenv("PWD", param, true, env);
         /* update the initial wdir value too */
         opal_setenv("OMPI_MCA_initial_wdir", param, true, env);
+    } else if (NULL != app->cwd) {
+        /* change to it */
+        if (0 != chdir(app->cwd)) {
+            return ORTE_ERROR;
+        }
     }
     return ORTE_SUCCESS;
 }

orte/mca/state/base/state_base_fns.c

@@ -24,6 +24,8 @@
 #include "opal/mca/event/event.h"
 #include "opal/mca/pmix/pmix.h"
 
+#include "orte/orted/pmix/pmix_server_internal.h"
+#include "orte/runtime/orte_data_server.h"
 #include "orte/runtime/orte_globals.h"
 #include "orte/runtime/orte_wait.h"
 #include "orte/mca/errmgr/errmgr.h"
@@ -466,6 +468,50 @@ void orte_state_base_report_progress(int fd, short argc, void *cbdata)
     OBJ_RELEASE(caddy);
 }
 
+void orte_state_base_notify_data_server(orte_process_name_t *target)
+{
+    opal_buffer_t *buf;
+    int rc, room = -1;
+    uint8_t cmd = ORTE_PMIX_PURGE_PROC_CMD;
+
+    /* if nobody local to us published anything, then we can ignore this */
+    if (ORTE_JOBID_INVALID == orte_pmix_server_globals.server.jobid) {
+        return;
+    }
+
+    buf = OBJ_NEW(opal_buffer_t);
+
+    /* pack the room number */
+    if (OPAL_SUCCESS != (rc = opal_dss.pack(buf, &room, 1, OPAL_INT))) {
+        ORTE_ERROR_LOG(rc);
+        OBJ_RELEASE(buf);
+        return;
+    }
+
+    /* load the command */
+    if (OPAL_SUCCESS != (rc = opal_dss.pack(buf, &cmd, 1, OPAL_UINT8))) {
+        ORTE_ERROR_LOG(rc);
+        OBJ_RELEASE(buf);
+        return;
+    }
+
+    /* provide the target */
+    if (ORTE_SUCCESS != (rc = opal_dss.pack(buf, target, 1, ORTE_NAME))) {
+        ORTE_ERROR_LOG(rc);
+        OBJ_RELEASE(buf);
+        return;
+    }
+
+    /* send the request to the server */
+    rc = orte_rml.send_buffer_nb(orte_mgmt_conduit,
+                                 &orte_pmix_server_globals.server, buf,
+                                 ORTE_RML_TAG_DATA_SERVER,
+                                 orte_rml_send_callback, NULL);
+    if (ORTE_SUCCESS != rc) {
+        OBJ_RELEASE(buf);
+    }
+}
+
 static void _send_notification(int status,
                                orte_proc_state_t state,
                                orte_process_name_t *proc,
@@ -725,6 +771,13 @@ void orte_state_base_track_procs(int fd, short argc, void *cbdata)
             if (orte_state_base_run_fdcheck) {
                 orte_state_base_check_fds(jdata);
             }
+            /* if ompi-server is around, then notify it to purge
+             * any session-related info */
+            if (NULL != orte_data_server_uri) {
+                target.jobid = jdata->jobid;
+                target.vpid = ORTE_VPID_WILDCARD;
+                orte_state_base_notify_data_server(&target);
+            }
             ORTE_ACTIVATE_JOB_STATE(jdata, ORTE_JOB_STATE_TERMINATED);
             /* if they requested notification upon completion, provide it */
             if (orte_get_attribute(&jdata->attributes, ORTE_JOB_NOTIFY_COMPLETION, NULL, OPAL_BOOL)) {
@@ -1035,6 +1088,7 @@ void orte_state_base_check_fds(orte_job_t *jdata)
     char path[1024], info[256], **list=NULL, *status, *result, *r2;
     ssize_t rc;
     struct flock fl;
+    bool flk;
     int cnt = 0;
 
     /* get the number of available file descriptors
@@ -1066,7 +1120,11 @@ void orte_state_base_check_fds(orte_job_t *jdata)
         fl.l_whence = 0;
         fl.l_start = 0;
         fl.l_len = 0;
-        fcntl(i, F_GETLK, &fl);
+        if (-1 == fcntl(i, F_GETLK, &fl)) {
+            flk = false;
+        } else {
+            flk = true;
+        }
         /* construct the list of capabilities */
         if (fdflags & FD_CLOEXEC) {
             opal_argv_append_nosize(&list, "cloexec");
@@ -1077,14 +1135,18 @@ void orte_state_base_check_fds(orte_job_t *jdata)
         if (flflags & O_NONBLOCK) {
             opal_argv_append_nosize(&list, "nonblock");
         }
-        if (flflags & O_RDONLY) {
+        /* from the man page:
+         *  Unlike the other values that can be specified in flags,
+         * the access mode values O_RDONLY, O_WRONLY, and O_RDWR,
+         * do not specify individual bits.  Rather, they define
+         * the low order two bits of flags, and defined respectively
+         * as 0, 1, and 2. */
+        if (O_RDONLY == (flflags & 3)) {
             opal_argv_append_nosize(&list, "rdonly");
-        }
-        if (flflags & O_RDWR) {
-            opal_argv_append_nosize(&list, "rdwr");
-        }
-        if (flflags & O_WRONLY) {
+        } else if (O_WRONLY == (flflags & 3)) {
             opal_argv_append_nosize(&list, "wronly");
+        } else {
+            opal_argv_append_nosize(&list, "rdwr");
         }
         if (flflags & O_DSYNC) {
             opal_argv_append_nosize(&list, "dsync");
@@ -1095,7 +1157,7 @@ void orte_state_base_check_fds(orte_job_t *jdata)
         if (flflags & O_SYNC) {
             opal_argv_append_nosize(&list, "sync");
         }
-        if (F_UNLCK != fl.l_type) {
+        if (flk && F_UNLCK != fl.l_type) {
             if (F_WRLCK == fl.l_type) {
                 opal_argv_append_nosize(&list, "wrlock");
             } else {

orte/mca/state/base/state_private.h

@@ -78,6 +78,7 @@ ORTE_DECLSPEC void orte_state_base_report_progress(int fd, short argc, void *cbd
 ORTE_DECLSPEC void orte_state_base_track_procs(int fd, short argc, void *cbdata);
 ORTE_DECLSPEC void orte_state_base_check_all_complete(int fd, short args, void *cbdata);
 ORTE_DECLSPEC void orte_state_base_check_fds(orte_job_t *jdata);
+ORTE_DECLSPEC void orte_state_base_notify_data_server(orte_process_name_t *target);
 
 END_C_DECLS
 #endif

orte/mca/state/dvm/state_dvm.c

@@ -80,6 +80,8 @@ static orte_job_state_t launch_states[] = {
     ORTE_JOB_STATE_DAEMONS_LAUNCHED,
     ORTE_JOB_STATE_DAEMONS_REPORTED,
     ORTE_JOB_STATE_VM_READY,
+    ORTE_JOB_STATE_MAP,
+    ORTE_JOB_STATE_MAP_COMPLETE,
     ORTE_JOB_STATE_SYSTEM_PREP,
     ORTE_JOB_STATE_LAUNCH_APPS,
     ORTE_JOB_STATE_LOCAL_LAUNCH_COMPLETE,
@@ -98,6 +100,8 @@ static orte_state_cbfunc_t launch_callbacks[] = {
     orte_plm_base_daemons_launched,
     orte_plm_base_daemons_reported,
     vm_ready,
+    orte_rmaps_base_map_job,
+    orte_plm_base_mapping_complete,
     orte_plm_base_complete_setup,
     orte_plm_base_launch_apps,
     orte_state_base_local_launch_complete,
@@ -211,7 +215,7 @@ static void files_ready(int status, void *cbdata)
         ORTE_FORCED_TERMINATE(status);
         return;
     } else {
-        ORTE_ACTIVATE_JOB_STATE(jdata, ORTE_JOB_STATE_SYSTEM_PREP);
+        ORTE_ACTIVATE_JOB_STATE(jdata, ORTE_JOB_STATE_MAP);
     }
 }
 

orte/mca/state/orted/state_orted.c

@@ -27,6 +27,8 @@
 #include "orte/mca/rml/rml.h"
 #include "orte/mca/routed/routed.h"
 #include "orte/util/session_dir.h"
+#include "orte/orted/pmix/pmix_server_internal.h"
+#include "orte/runtime/orte_data_server.h"
 #include "orte/runtime/orte_quit.h"
 
 #include "orte/mca/state/state.h"
@@ -260,6 +262,7 @@ static void track_procs(int fd, short argc, void *cbdata)
     orte_std_cntr_t index;
     orte_job_map_t *map;
     orte_node_t *node;
+    orte_process_name_t target;
 
     OPAL_OUTPUT_VERBOSE((5, orte_state_base_framework.framework_output,
                          "%s state:orted:track_procs called for proc %s state %s",
@@ -489,6 +492,14 @@ static void track_procs(int fd, short argc, void *cbdata)
                 orte_state_base_check_fds(jdata);
             }
 
+            /* if ompi-server is around, then notify it to purge
+             * any session-related info */
+            if (NULL != orte_data_server_uri) {
+                target.jobid = jdata->jobid;
+                target.vpid = ORTE_VPID_WILDCARD;
+                orte_state_base_notify_data_server(&target);
+            }
+
             /* cleanup the job info */
             opal_hash_table_set_value_uint32(orte_job_data, jdata->jobid, NULL);
             OBJ_RELEASE(jdata);

orte/orted/orted_submit.c

@@ -370,7 +370,6 @@ int orte_submit_init(int argc, char *argv[],
     } else {
         orte_process_info.proc_type = ORTE_PROC_TOOL;
     }
-
     if (ORTE_PROC_IS_TOOL) {
         if (0 == strncasecmp(orte_cmd_options.hnp, "file", strlen("file"))) {
             char input[1024], *filename;
@@ -1629,22 +1628,17 @@ static int create_app(int argc, char* argv[],
     app->num_procs = (orte_std_cntr_t)orte_cmd_options.num_procs;
     total_num_apps++;
 
-    /* Capture any preload flags */
-    if (orte_cmd_options.preload_binaries) {
-        orte_set_attribute(&app->attributes, ORTE_APP_PRELOAD_BIN, ORTE_ATTR_GLOBAL, NULL, OPAL_BOOL);
-    }
-    /* if we were told to cwd to the session dir and the app was given in
-     * relative syntax, then we need to preload the binary to
+    /* see if we need to preload the binary to
      * find the app - don't do this for java apps, however, as we
      * can't easily find the class on the cmd line. Java apps have to
      * preload their binary via the preload_files option
      */
-    if (!opal_path_is_absolute(app->argv[0]) &&
-        NULL == strstr(app->argv[0], "java")) {
+    if (NULL == strstr(app->argv[0], "java")) {
         if (orte_cmd_options.preload_binaries) {
             orte_set_attribute(&app->attributes, ORTE_APP_SSNDIR_CWD, ORTE_ATTR_GLOBAL, NULL, OPAL_BOOL);
-        } else if (orte_get_attribute(&app->attributes, ORTE_APP_SSNDIR_CWD, NULL, OPAL_BOOL)) {
             orte_set_attribute(&app->attributes, ORTE_APP_PRELOAD_BIN, ORTE_ATTR_GLOBAL, NULL, OPAL_BOOL);
+            /* no harm in setting this attribute twice as the function will simply ignore it */
+            orte_set_attribute(&app->attributes, ORTE_APP_SSNDIR_CWD, ORTE_ATTR_GLOBAL, NULL, OPAL_BOOL);
         }
     }
     if (NULL != orte_cmd_options.preload_files) {