From 19bbba905b90905db794a4e7356b8b8128c30376 Mon Sep 17 00:00:00 2001 From: Ralph Castain Date: Sun, 4 Aug 2024 17:06:44 -0600 Subject: [PATCH] Use the PMIx functions to check params Break the multi-loop thru loading of param files that caused us to overwrite values. Defer to the PMIx pmdl components for obtaining envars and for checking MCA param overlaps across projects. Signed-off-by: Ralph Castain --- src/mca/schizo/base/base.h | 4 +- src/mca/schizo/base/schizo_base_stubs.c | 70 +------ src/mca/schizo/ompi/schizo_ompi.c | 231 +----------------------- src/runtime/prte_init.c | 31 +++- src/tools/prte/prte.c | 47 +---- 5 files changed, 34 insertions(+), 349 deletions(-) diff --git a/src/mca/schizo/base/base.h b/src/mca/schizo/base/base.h index 57b2d85f0a..983eb862a1 100644 --- a/src/mca/schizo/base/base.h +++ b/src/mca/schizo/base/base.h @@ -2,7 +2,7 @@ * Copyright (c) 2015-2020 Intel, Inc. All rights reserved. * Copyright (c) 2020 IBM Corporation. All rights reserved. * Copyright (c) 2020 Cisco Systems, Inc. All rights reserved - * Copyright (c) 2021-2023 Nanook Consulting. All rights reserved. + * Copyright (c) 2021-2024 Nanook Consulting All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -95,8 +95,6 @@ PRTE_EXPORT bool prte_schizo_base_check_directives(char *directive, PRTE_EXPORT bool prte_schizo_base_check_qualifiers(char *directive, char **valid, char *qual); -PRTE_EXPORT bool prte_schizo_base_check_prte_param(char *param); -PRTE_EXPORT bool prte_schizo_base_check_pmix_param(char *param); PRTE_EXPORT void prte_schizo_base_expose(char *param, char *prefix); PRTE_EXPORT int prte_schizo_base_add_directive(pmix_cli_result_t *results, const char *deprecated, const char *target, diff --git a/src/mca/schizo/base/schizo_base_stubs.c b/src/mca/schizo/base/schizo_base_stubs.c index 6a3bc7225a..149762c1c8 100644 --- a/src/mca/schizo/base/schizo_base_stubs.c +++ b/src/mca/schizo/base/schizo_base_stubs.c @@ -21,6 +21,7 @@ #include "src/include/pmix_frameworks.h" #include "src/include/prte_frameworks.h" #include "src/mca/errmgr/errmgr.h" +#include "src/mca/pmdl/base/base.h" #include "src/mca/schizo/base/base.h" #include "src/runtime/prte_globals.h" #include "src/util/pmix_argv.h" @@ -242,26 +243,6 @@ char *prte_schizo_base_strip_quotes(char *p) return pout; } -bool prte_schizo_base_check_prte_param(char *param) -{ - char *p; - size_t n; - int len; - - p = strchr(param, '_'); - len = (int)(p - param); - - if (0 == strncmp(param, "prte", len)) { - return true; - } - for (n=0; NULL != prte_framework_names[n]; n++) { - if (0 == strncmp(param, prte_framework_names[n], len)) { - return true; - } - } - return false; -} - int prte_schizo_base_parse_prte(int argc, int start, char **argv, char ***target) { int i; @@ -312,7 +293,7 @@ int prte_schizo_base_parse_prte(int argc, int start, char **argv, char ***target /* this is a generic MCA designation, so see if the parameter it * refers to belongs to one of our frameworks */ - use = prte_schizo_base_check_prte_param(p1); + use = pmix_pmdl_base_check_prte_param(p1); if (use) { /* replace the generic directive with a PRRTE specific * one so we know this has been processed */ @@ -361,51 +342,6 @@ int prte_schizo_base_parse_prte(int argc, int start, char **argv, char ***target return PRTE_SUCCESS; } -static char **pmix_frameworks_tocheck = pmix_framework_names; -static bool pmix_frameworks_setup = false; - -static void setup_pmix_frameworks(void) -{ - if (pmix_frameworks_setup) { - return; - } - pmix_frameworks_setup = true; - - char *env = getenv("PMIX_MCA_PREFIXES"); - if (NULL == env) { - return; - } - - // If we found the env variable, it will be a comma-delimited list - // of values. Split it into an argv-style array. - char **tmp = PMIX_ARGV_SPLIT_COMPAT(env, ','); - if (NULL != tmp) { - pmix_frameworks_tocheck = tmp; - } -} - -bool prte_schizo_base_check_pmix_param(char *param) -{ - char *p; - size_t n; - int len; - - setup_pmix_frameworks(); - - p = strchr(param, '_'); - len = (int)(p - param); - - if (0 == strncmp(param, "pmix", len)) { - return true; - } - for (n=0; NULL != pmix_frameworks_tocheck[n]; n++) { - if (0 == strncmp(param, pmix_frameworks_tocheck[n], len)) { - return true; - } - } - return false; -} - int prte_schizo_base_parse_pmix(int argc, int start, char **argv, char ***target) { int i; @@ -487,7 +423,7 @@ int prte_schizo_base_parse_pmix(int argc, int start, char **argv, char ***target /* this is a generic MCA designation, so see if the parameter it * refers to belongs to one of our frameworks */ - use = prte_schizo_base_check_pmix_param(p1); + use = pmix_pmdl_base_check_pmix_param(p1); if (use) { /* replace the generic directive with a PMIx specific * one so we know this has been processed */ diff --git a/src/mca/schizo/ompi/schizo_ompi.c b/src/mca/schizo/ompi/schizo_ompi.c index 90eb691fa4..f8804a5ec6 100644 --- a/src/mca/schizo/ompi/schizo_ompi.c +++ b/src/mca/schizo/ompi/schizo_ompi.c @@ -57,6 +57,7 @@ #include "src/mca/base/pmix_mca_base_vari.h" #include "src/mca/errmgr/errmgr.h" #include "src/mca/ess/base/base.h" +#include "src/mca/pmdl/base/base.h" #include "src/mca/rmaps/base/base.h" #include "src/mca/state/base/base.h" #include "src/runtime/prte_globals.h" @@ -1821,232 +1822,6 @@ static int parse_env(char **srcenv, char ***dstenv, return PRTE_SUCCESS; } -static bool check_prte_overlap(char *var, char *value) -{ - char *tmp; - - if (0 == strncmp(var, "dl_", 3)) { - pmix_asprintf(&tmp, "PRTE_MCA_prtedl_%s", &var[3]); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, value, false); - free(tmp); - return true; - } else if (0 == strncmp(var, "oob_", 4)) { - pmix_asprintf(&tmp, "PRTE_MCA_%s", var); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, value, false); - free(tmp); - return true; - } else if (0 == strncmp(var, "hwloc_", 6)) { - pmix_asprintf(&tmp, "PRTE_MCA_%s", var); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, value, false); - free(tmp); - return true; - } else if (0 == strncmp(var, "if_", 3)) { - // need to convert if to prteif - pmix_asprintf(&tmp, "PRTE_MCA_prteif_%s", &var[3]); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, value, false); - free(tmp); - return true; - } else if (0 == strncmp(var, "reachable_", strlen("reachable_"))) { - // need to convert reachable to prtereachable - pmix_asprintf(&tmp, "PRTE_MCA_prtereachable_%s", &var[strlen("reachable_")]); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, value, false); - free(tmp); - return true; - } else if (0 == strncmp(var, "plm_rsh_", strlen("plm_rsh_"))) { - // need to convert rsh to ssh - pmix_asprintf(&tmp, "PRTE_MCA_plm_ssh_%s", &var[strlen("plm_rsh_")]); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, value, false); - free(tmp); - return true; - } else if (0 == strncmp(var, "orte_", strlen("orte_"))) { - // need to convert "orte" to "prte" - pmix_asprintf(&tmp, "PRTE_MCA_prte_%s", &var[strlen("orte_")]); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, value, false); - free(tmp); - return true; - } - return false; -} - - -static bool check_pmix_overlap(char *var, char *value) -{ - char *tmp; - - if (0 == strncmp(var, "dl_", 3)) { - pmix_asprintf(&tmp, "PMIX_MCA_pdl_%s", &var[3]); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, value, false); - free(tmp); - return true; - } else if (0 == strncmp(var, "oob_", 4)) { - pmix_asprintf(&tmp, "PMIX_MCA_ptl_%s", &var[4]); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, value, false); - free(tmp); - return true; - } else if (0 == strncmp(var, "hwloc_", 6)) { - pmix_asprintf(&tmp, "PMIX_MCA_%s", var); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, value, false); - free(tmp); - return true; - } else if (0 == strncmp(var, "if_", 3)) { - // need to convert if to pif - pmix_asprintf(&tmp, "PMIX_MCA_pif_%s", &var[3]); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, value, false); - free(tmp); - return true; - } - return false; -} - -// NOTE: This code is fundamentally the same (module PMIX <-> OPAL) -// as the translate_params() routine in the OMPI repo's -// opal/mca/pmix/base/pmix_base_fns.c file. If there are -// changes here, there are likely to be changes there. -static int translate_params(void) -{ - char *evar, *tmp, *e2; - char *file; - const char *home; - pmix_list_t params; - pmix_mca_base_var_file_value_t *fv; - uid_t uid; - int n, len; - - /* since we are the proxy, we need to check the OMPI default - * MCA params to see if there is something relating to PRRTE - * in them - this would be "old" references to things from - * ORTE, as well as a few OPAL references that also impact us - * - * NOTE: we do this in the following precedence order. Note - * that we do not overwrite at any step - this is so that we - * don't overwrite something previously set by the user. So - * the order to execution is the opposite of the intended - * precedence order. - * - * 1. check the environmental paramaters for OMPI_MCA values - * that need to be translated - * - * 2. the user's home directory file as it should - * overwrite the system default file, but not the - * envars - * - * 3. the system default parameter file - */ - len = strlen("OMPI_MCA_"); - for (n=0; NULL != environ[n]; n++) { - if (0 == strncmp(environ[n], "OMPI_MCA_", len)) { - e2 = strdup(environ[n]); - evar = strrchr(e2, '='); - *evar = '\0'; - ++evar; - if (check_prte_overlap(&e2[len], evar)) { - // check for pmix overlap - check_pmix_overlap(&e2[len], evar); - } else if (prte_schizo_base_check_prte_param(&e2[len])) { - pmix_asprintf(&tmp, "PRTE_MCA_%s", &e2[len]); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, evar, false); - free(tmp); - // check for pmix overlap - check_pmix_overlap(&e2[len], evar); - } else if (prte_schizo_base_check_pmix_param(&e2[len])) { - pmix_asprintf(&tmp, "PMIX_MCA_%s", &e2[len]); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, evar, false); - free(tmp); - } - free(e2); - } - } - - /* see if the user has a default MCA param file */ - uid = geteuid(); - - /* try to get their home directory */ - home = pmix_home_directory(uid); - if (NULL != home) { - file = pmix_os_path(false, home, ".openmpi", "mca-params.conf", NULL); - PMIX_CONSTRUCT(¶ms, pmix_list_t); - pmix_mca_base_parse_paramfile(file, ¶ms); - free(file); - PMIX_LIST_FOREACH (fv, ¶ms, pmix_mca_base_var_file_value_t) { - // see if this param relates to PRRTE - if (check_prte_overlap(fv->mbvfv_var, fv->mbvfv_value)) { - check_pmix_overlap(fv->mbvfv_var, fv->mbvfv_value); - } else if (prte_schizo_base_check_prte_param(fv->mbvfv_var)) { - pmix_asprintf(&tmp, "PRTE_MCA_%s", fv->mbvfv_var); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, fv->mbvfv_value, false); - free(tmp); - // if this relates to the DL, OOB, HWLOC, IF, or - // REACHABLE frameworks, then we also need to set - // the equivalent PMIx value - check_pmix_overlap(fv->mbvfv_var, fv->mbvfv_value); - } else if (prte_schizo_base_check_pmix_param(fv->mbvfv_var)) { - pmix_asprintf(&tmp, "PMIX_MCA_%s", fv->mbvfv_var); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, fv->mbvfv_value, false); - free(tmp); - } - } - PMIX_LIST_DESTRUCT(¶ms); - } - - /* check if the user has set OMPIHOME in their environment */ - if (NULL != (evar = getenv("OMPIHOME"))) { - /* look for the default MCA param file */ - file = pmix_os_path(false, evar, "etc", "openmpi-mca-params.conf", NULL); - PMIX_CONSTRUCT(¶ms, pmix_list_t); - pmix_mca_base_parse_paramfile(file, ¶ms); - free(file); - PMIX_LIST_FOREACH (fv, ¶ms, pmix_mca_base_var_file_value_t) { - // see if this param relates to PRRTE - if (check_prte_overlap(fv->mbvfv_var, fv->mbvfv_value)) { - check_pmix_overlap(fv->mbvfv_var, fv->mbvfv_value); - } else if (prte_schizo_base_check_prte_param(fv->mbvfv_var)) { - pmix_asprintf(&tmp, "PRTE_MCA_%s", fv->mbvfv_var); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, fv->mbvfv_value, false); - free(tmp); - // if this relates to the DL, OOB, HWLOC, IF, or - // REACHABLE frameworks, then we also need to set - // the equivalent PMIx value - check_pmix_overlap(fv->mbvfv_var, fv->mbvfv_value); - } - } - PMIX_LIST_DESTRUCT(¶ms); - } - - return 100; -} - static int detect_proxy(char *personalities) { char *evar; @@ -2062,7 +1837,7 @@ static int detect_proxy(char *personalities) /* this is a list of personalities we need to check - * if it contains "ompi", then we are available */ if (NULL != strstr(personalities, "ompi")) { - return translate_params(); + return 100; } return 0; } @@ -2070,7 +1845,7 @@ static int detect_proxy(char *personalities) /* if we were told the proxy, then use it */ if (NULL != (evar = getenv("PRTE_MCA_schizo_proxy"))) { if (0 == strcmp(evar, "ompi")) { - return translate_params(); + return 100; } else { return 0; } diff --git a/src/runtime/prte_init.c b/src/runtime/prte_init.c index e417f4b98c..db7022cfb1 100644 --- a/src/runtime/prte_init.c +++ b/src/runtime/prte_init.c @@ -60,6 +60,7 @@ #include "src/hwloc/hwloc-internal.h" #include "src/prted/pmix/pmix_server.h" #include "src/threads/pmix_threads.h" +#include "src/include/prte_frameworks.h" #include "src/mca/base/pmix_base.h" #include "src/mca/base/pmix_mca_base_var.h" @@ -147,8 +148,9 @@ static void print_error(unsigned major, int prte_init_minimum(void) { - int ret; + int ret, n; char *path = NULL; + char *evar, **prefixes; const char *rvers; char token[100]; unsigned int major, minor, release; @@ -180,6 +182,19 @@ int prte_init_minimum(void) /* carry across the toolname */ pmix_tool_basename = prte_tool_basename; + // publish MCA prefixes + prefixes = NULL; + for (n=0; NULL != prte_framework_names[n]; n++) { + if (0 == strcmp("common", prte_framework_names[n])) { + continue; + } + PMIx_Argv_append_nosize(&prefixes, prte_framework_names[n]); + } + evar = PMIx_Argv_join(prefixes, ','); + pmix_setenv("PRTE_MCA_PREFIXES", evar, true, &environ); + free(evar); + PMIx_Argv_free(prefixes); + /* initialize install dirs code */ ret = pmix_mca_base_framework_open(&prte_prteinstalldirs_base_framework, PMIX_MCA_BASE_OPEN_DEFAULT); @@ -524,7 +539,13 @@ void prte_preload_default_mca_params(void) * user already has the param in our environment as their * environment settings override all defaults */ PMIX_LIST_FOREACH(fv, &pfinal, pmix_mca_base_var_file_value_t) { - if (pmix_pmdl_base_check_prte_param(fv->mbvfv_var)) { + if (pmix_pmdl_base_check_pmix_param(fv->mbvfv_var)) { + pmix_asprintf(&tmp, "PMIX_MCA_%s", fv->mbvfv_var); + // set it, but don't overwrite if they already + // have a value in our environment + setenv(tmp, fv->mbvfv_value, false); + free(tmp); + } else { pmix_asprintf(&tmp, "PRTE_MCA_%s", fv->mbvfv_var); // set it, but don't overwrite if they already // have a value in our environment @@ -534,12 +555,6 @@ void prte_preload_default_mca_params(void) // or mca frameworks, then we also need to set // the equivalent PMIx value check_pmix_overlap(fv->mbvfv_var, fv->mbvfv_value); - } else if (pmix_pmdl_base_check_pmix_param(fv->mbvfv_var)) { - pmix_asprintf(&tmp, "PMIX_MCA_%s", fv->mbvfv_var); - // set it, but don't overwrite if they already - // have a value in our environment - setenv(tmp, fv->mbvfv_value, false); - free(tmp); } } diff --git a/src/tools/prte/prte.c b/src/tools/prte/prte.c index 52662c79c0..c93b8861b2 100644 --- a/src/tools/prte/prte.c +++ b/src/tools/prte/prte.c @@ -336,6 +336,10 @@ int main(int argc, char *argv[]) } } + /* do a minimal setup of key infrastructure, including + * parsing the install-level and user-level PRRTE param + * files + */ rc = prte_init_minimum(); if (PRTE_SUCCESS != rc) { return rc; @@ -1147,49 +1151,6 @@ int main(int argc, char *argv[]) /* give the schizo components a chance to add to the job info */ schizo->job_info(&results, jinfo); - /* pickup any relevant envars */ - ninfo = 4; - PMIX_INFO_CREATE(iptr, ninfo); - flag = true; - PMIX_INFO_LOAD(&iptr[0], PMIX_SETUP_APP_ENVARS, &flag, PMIX_BOOL); - ui32 = geteuid(); - PMIX_INFO_LOAD(&iptr[1], PMIX_USERID, &ui32, PMIX_UINT32); - ui32 = getegid(); - PMIX_INFO_LOAD(&iptr[2], PMIX_GRPID, &ui32, PMIX_UINT32); - PMIX_INFO_LOAD(&iptr[3], PMIX_PERSONALITY, personality, PMIX_STRING); - - PRTE_PMIX_CONSTRUCT_LOCK(&mylock.lock); - ret = PMIx_server_setup_application(prte_process_info.myproc.nspace, iptr, ninfo, setupcbfunc, - &mylock); - if (PMIX_SUCCESS != ret) { - pmix_output(0, "Error setting up application: %s", PMIx_Error_string(ret)); - PRTE_PMIX_DESTRUCT_LOCK(&mylock.lock); - PRTE_UPDATE_EXIT_STATUS(ret); - goto DONE; - } - PRTE_PMIX_WAIT_THREAD(&mylock.lock); - PMIX_INFO_FREE(iptr, ninfo); - if (PMIX_SUCCESS != mylock.status) { - pmix_output(0, "Error setting up application: %s", PMIx_Error_string(mylock.status)); - PRTE_UPDATE_EXIT_STATUS(mylock.status); - PRTE_PMIX_DESTRUCT_LOCK(&mylock.lock); - goto DONE; - } - PRTE_PMIX_DESTRUCT_LOCK(&mylock.lock); - /* transfer any returned ENVARS to the job_info */ - if (NULL != mylock.info) { - for (n = 0; n < mylock.ninfo; n++) { - if (PMIX_CHECK_KEY(&mylock.info[n], PMIX_SET_ENVAR) || - PMIX_CHECK_KEY(&mylock.info[n], PMIX_ADD_ENVAR) || - PMIX_CHECK_KEY(&mylock.info[n], PMIX_UNSET_ENVAR) || - PMIX_CHECK_KEY(&mylock.info[n], PMIX_PREPEND_ENVAR) || - PMIX_CHECK_KEY(&mylock.info[n], PMIX_APPEND_ENVAR)) { - PMIX_INFO_LIST_XFER(ret, jinfo, &mylock.info[n]); - } - } - PMIX_INFO_FREE(mylock.info, mylock.ninfo); - } - /* convert the job info into an array */ PMIX_INFO_LIST_CONVERT(ret, jinfo, &darray); if (PMIX_ERR_EMPTY == ret) {