Merge pull request #6321 from hppritcha/topic/fix_6236_for_v4.x

Topic/fix 6236 for v4.x

orte/mca/rmaps/base/base.h

@@ -123,7 +123,8 @@ ORTE_DECLSPEC int orte_rmaps_base_filter_nodes(orte_app_context_t *app,
                                                opal_list_t *nodes,
                                                bool remove);
 
-ORTE_DECLSPEC int orte_rmaps_base_set_mapping_policy(orte_mapping_policy_t *policy,
+ORTE_DECLSPEC int orte_rmaps_base_set_mapping_policy(orte_job_t *jdata,
+                                                     orte_mapping_policy_t *policy,
                                                      char **device, char *spec);
 ORTE_DECLSPEC int orte_rmaps_base_set_ranking_policy(orte_ranking_policy_t *policy,
                                                      orte_mapping_policy_t mapping,

orte/mca/rmaps/base/rmaps_base_frame.c

@@ -12,7 +12,7 @@
  * Copyright (c) 2006-2015 Cisco Systems, Inc.  All rights reserved.
  * Copyright (c) 2011-2013 Los Alamos National Security, LLC.
  *                         All rights reserved.
- * Copyright (c) 2014-2018 Intel, Inc. All rights reserved.
+ * Copyright (c) 2014-2019 Intel, Inc.  All rights reserved.
  * Copyright (c) 2014-2015 Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
  * $COPYRIGHT$
@@ -296,7 +296,7 @@ static int orte_rmaps_base_open(mca_base_open_flag_t flags)
                        "rmaps_base_cpus_per_proc", "rmaps_base_mapping_policy=<obj>:PE=N, default <obj>=NUMA");
     }
 
-    if (ORTE_SUCCESS != (rc = orte_rmaps_base_set_mapping_policy(&orte_rmaps_base.mapping,
+    if (ORTE_SUCCESS != (rc = orte_rmaps_base_set_mapping_policy(NULL, &orte_rmaps_base.mapping,
                                                                  &orte_rmaps_base.device,
                                                                  rmaps_base_mapping_policy))) {
         return rc;
@@ -593,7 +593,8 @@ static int check_modifiers(char *ck, orte_mapping_policy_t *tmp)
     return ORTE_ERR_TAKE_NEXT_OPTION;
 }
 
-int orte_rmaps_base_set_mapping_policy(orte_mapping_policy_t *policy,
+int orte_rmaps_base_set_mapping_policy(orte_job_t *jdata,
+                                       orte_mapping_policy_t *policy,
                                        char **device, char *inspec)
 {
     char *ck;
@@ -618,7 +619,9 @@ int orte_rmaps_base_set_mapping_policy(orte_mapping_policy_t *policy,
 
     if (NULL == inspec) {
         ORTE_SET_MAPPING_POLICY(tmp, ORTE_MAPPING_BYSOCKET);
-    } else {
+        goto setpolicy;
+    }
+
     spec = strdup(inspec);  // protect the input string
     /* see if a colon was included - if so, then we have a policy + modifier */
     ck = strchr(spec, ':');
@@ -626,7 +629,7 @@ int orte_rmaps_base_set_mapping_policy(orte_mapping_policy_t *policy,
         /* if the colon is the first character of the string, then we
          * just have modifiers on the default mapping policy */
         if (ck == spec) {
-                ck++;
+            ck++;  // step over the colon
             opal_output_verbose(5, orte_rmaps_base_framework.framework_output,
                                 "%s rmaps:base only modifiers %s provided - assuming bysocket mapping",
                                 ORTE_NAME_PRINT(ORTE_PROC_MY_NAME), ck);
@@ -639,20 +642,20 @@ int orte_rmaps_base_set_mapping_policy(orte_mapping_policy_t *policy,
             free(spec);
             goto setpolicy;
         }
-            /* split the string */
+        *ck = '\0';  // terminate spec where the colon was
-            *ck = '\0';
+        ck++;    // step past the colon
-            ck++;
         opal_output_verbose(5, orte_rmaps_base_framework.framework_output,
                             "%s rmaps:base policy %s modifiers %s provided",
                             ORTE_NAME_PRINT(ORTE_PROC_MY_NAME), spec, ck);
-            /* if the policy is "dist", then we set the policy to that value
+
-             * and save the second argument as the device
-             */
         if (0 == strncasecmp(spec, "ppr", strlen(spec))) {
-                /* we have to allow additional modifiers here - e.g., specifying
+            /* at this point, ck points to a string that contains at least
-                 * #pe's/proc or oversubscribe - so check for modifiers
+             * two fields (specifying the #procs/obj and the object we are
+             * to map by). we have to allow additional modifiers here - e.g.,
+             * specifying #pe's/proc or oversubscribe - so check for modifiers. if
+             * they are present, ck will look like "N:obj:mod1,mod2,mod3"
              */
-                if (NULL == (ptr = strrchr(ck, ':'))) {
+            if (NULL == (ptr = strchr(ck, ':'))) {
                 /* this is an error - there had to be at least one
                  * colon to delimit the number from the object type
                  */
@@ -661,15 +664,14 @@ int orte_rmaps_base_set_mapping_policy(orte_mapping_policy_t *policy,
                 return ORTE_ERR_SILENT;
             }
             ptr++; // move past the colon
-                /* at this point, ck is pointing to the number of procs/object
+            /* at this point, ptr is pointing to the beginning of the string that describes
-                 * and ptr is pointing to the beginning of the string that describes
+             * the object plus any modifiers (i.e., "obj:mod1,mod2". We first check to see if there
-                 * the object plus any modifiers. We first check to see if there
+             * is another colon indicating that there are modifiers to the request */
-                 * is a comma indicating that there are modifiers to the request */
+            if (NULL != (cptr = strchr(ptr, ':'))) {
-                if (NULL != (cptr = strchr(ptr, ','))) {
                 /* there are modifiers, so we terminate the object string
-                     * at the location of the first comma */
+                 * at the location of the colon */
                 *cptr = '\0';
-                    /* step over that comma */
+                /* step over that colon */
                 cptr++;
                 /* now check for modifiers  - may be none, so
                  * don't emit an error message if the modifier
@@ -681,7 +683,11 @@ int orte_rmaps_base_set_mapping_policy(orte_mapping_policy_t *policy,
                 }
             }
             /* now save the pattern */
+            if (NULL == jdata || NULL == jdata->map) {
                 orte_rmaps_base.ppr = strdup(ck);
+            } else {
+                jdata->map->ppr = strdup(ck);
+            }
             ORTE_SET_MAPPING_POLICY(tmp, ORTE_MAPPING_PPR);
             ORTE_SET_MAPPING_DIRECTIVE(tmp, ORTE_MAPPING_GIVEN);
             free(spec);
@@ -744,10 +750,13 @@ int orte_rmaps_base_set_mapping_policy(orte_mapping_policy_t *policy,
     }
     free(spec);
     ORTE_SET_MAPPING_DIRECTIVE(tmp, ORTE_MAPPING_GIVEN);
-    }
 
  setpolicy:
+    if (NULL == jdata || NULL == jdata->map) {
         *policy = tmp;
+    } else {
+        jdata->map->mapping = tmp;
+    }
 
     return ORTE_SUCCESS;
 }

orte/orted/orted_submit.c

@@ -876,7 +876,7 @@ int orte_submit_job(char *argv[], int *index,
     jdata->map = OBJ_NEW(orte_job_map_t);
 
     if (NULL != orte_cmd_options.mapping_policy) {
-        if (ORTE_SUCCESS != (rc = orte_rmaps_base_set_mapping_policy(&jdata->map->mapping, NULL, orte_cmd_options.mapping_policy))) {
+        if (ORTE_SUCCESS != (rc = orte_rmaps_base_set_mapping_policy(jdata, &jdata->map->mapping, NULL, orte_cmd_options.mapping_policy))) {
             ORTE_ERROR_LOG(rc);
             return rc;
         }

orte/orted/pmix/pmix_server_dyn.c

@@ -308,7 +308,7 @@ int pmix_server_spawn_fn(opal_process_name_t *requestor,
                                orte_rmaps_base_print_mapping(orte_rmaps_base.mapping));
                 return ORTE_ERR_BAD_PARAM;
             }
-            rc = orte_rmaps_base_set_mapping_policy(&jdata->map->mapping,
+            rc = orte_rmaps_base_set_mapping_policy(jdata, &jdata->map->mapping,
                                                     NULL, info->data.string);
             if (ORTE_SUCCESS != rc) {
                 return rc;

orte/orted/pmix/pmix_server_gen.c

@@ -489,7 +489,7 @@ static void _query(int sd, short args, void *cbdata)
     orte_job_t *jdata;
     orte_proc_t *proct;
     orte_app_context_t *app;
-    int rc, i, k, num_replies;
+    int rc = ORTE_SUCCESS, i, k, num_replies;
     opal_list_t *results, targets, *array;
     size_t n;
     uint32_t key;
@@ -716,7 +716,7 @@ static void _query(int sd, short args, void *cbdata)
                     }
                 }
                 if (ORTE_JOBID_INVALID == jobid) {
-                    rc = ORTE_ERR_BAD_PARAM;
+                    rc = ORTE_ERR_NOT_FOUND;
                     goto done;
                 }
                 /* construct a list of values with opal_proc_info_t
@@ -810,12 +810,12 @@ static void _query(int sd, short args, void *cbdata)
     }
 
   done:
+    if (ORTE_SUCCESS == rc) {
         if (0 == opal_list_get_size(results)) {
             rc = ORTE_ERR_NOT_FOUND;
         } else if (opal_list_get_size(results) < opal_list_get_size(cd->info)) {
             rc = ORTE_ERR_PARTIAL_SUCCESS;
-    } else {
+        }
-        rc = ORTE_SUCCESS;
     }
     cd->infocbfunc(rc, results, cd->cbdata, qrel, results);
 }