Ompi main sync to upstream 93952a6920 - #183
Merged
hppritcha merged 560 commits intoAug 5, 2026
Merged
Conversation
A deep review of the ras framework turned up a set of defects that share a theme: the paths that run only when something unusual happens - a node rediscovered on an elastic re-grow, a malformed line in a scheduler's node file, an error return from a library call - were the ones that had never been exercised. The most consequential is in prte_ras_base_node_insert. When the dedup scan finds that the pool already holds the incoming node, the pool entry is authoritative and the duplicate should be dropped; instead it fell through to the common tail, which leaked the object, counted its slots into prte_ras_base.total_slots_alloc a second time, and re-ran the multiplier copies. This fires on every elastic re-grow, because a shrink removes a node's daemon but leaves its pool entry behind, so the regrant always arrives as a duplicate. In a managed allocation total_slots_alloc is what the PMIx server reports as PMIX_UNIV_SIZE and PMIX_MAX_PROCS, so the inflated count is visible to applications. The same function copied a node's attributes onto the HNP entry by passing a pmix_value_t where prte_set_attribute expects a raw C value pointer, which made the copy a no-op that also corrupted the source attribute - and would strdup out of a pmix_value_t for any string-valued one. It also dereferenced the daemon job object without checking it exists. In prte_ras_base_allocate, a failed PMIX_NOTIFY_ALLOC_COMPLETE released the state caddy and then fell through to release it a second time while activating a contradictory job state. The --display-topo and --display-cpus pool walks dereferenced node->topology unconditionally, but a node only acquires a topology once its daemon reports in, so any allocated-but-unlaunched node in the pool was a crash. The ras/flux allocate path declared its json_t root mid-function while every early error path jumped over that initializer to a cleanup block that decrefs it; one of those paths also returned before its cleanup, leaking the broker handle and the KVS future, and left the goto after it unreachable. The R parser took a reference on the "scheduling" subtree it never releases, mixed owned and literal strings in one error pointer, and returned success after a malformed R_lite entry. ras/gridengine dereferenced strtok_r results that are NULL for a blank PE_HOSTFILE line, and ras/pbs chopped the last character off every nodefile line whether or not it was a newline, turning a blank line into a node named "". Both run on the HNP, where a NULL deref takes down the DVM. ras/simulator indexed slot_cnt[-1] when its slots parameter was empty and accumulated its group-name prefix instead of assigning it. ras/hosts passed plain chars to isspace and left its working list and an unvalidated info value unguarded in modify. ras/slurm leaked the expanded nodelist argv on four error paths in discover, and dropped the session reference on every node detached by a partial shrink. Add test/unit/ras, which covers node_insert's dedup and accounting directly, the module vtable contract, the priority-ordered selection that makes hosts the catch-all, and the SLURM taint validators; and add dockerswarm coverage for the two multi-node paths the unit test cannot reach - grow/shrink/re-grow leaving exactly one daemon per node, and --add-hostfile growing a live DVM through ras/hosts including the slots=+N in-place adjust. Update the AGENTS.md guides with the ownership and NULL-safety rules the review had to reconstruct. Signed-off-by: Ralph Castain <rhc@pmix.org>
Two follow-ups from the ras review, in the two places where the code did not implement what it documented. ras/pmix forwards a runtime allocation request to a host PMIx scheduler and gets the answer back on the PMIx progress thread. The golden rule for such a callback is that it captures its arguments and posts an event, nothing more, because a prte_pmix_server_req_t is a PRRTE object owned by the PRRTE progress thread. Instead infocbfunc() recorded the status on the request, freed the info array the request was carrying, and rewrote four more of its fields before shifting. Introduce a caddy that carries the status, the info array and PMIx's release callback across the shift, holding a reference on the request so it cannot be reclaimed underneath, and move every mutation into the shifted handler. Doing so exposed two defects in that handler. It recorded the scheduler's verdict in req->status while testing req->pstatus, which prte_ras_base_modify seeds with PMIX_ERR_NOT_SUPPORTED and never updates once a module takes ownership - so a granted allocation was never applied and the requester was told its request was unsupported. And it released the request immediately after invoking the requester's callback, while that callback was still looking at the info array the request owns; use the release-callback handoff prte_ras_base_modify already uses instead. Relatedly, both ras/pmix's modify() and the base's ras_base_set_alloc_response() repointed req->info without freeing a previously owned array. That is not theoretical: pmix_server.c builds a relayed allocation request with copy already true. ras/bootstrap sets each node's index to its canonical DVM rank, read from the configuration file, because a bootstrapped daemon computes its own vpid from that same file - the HNP does not get to choose one, it has to arrive at the same answer independently. But node_insert overwrote the assignment with the pool-insertion ordinal, so the correspondence held only because DVMNodes happens to be listed in rank order. Teach node_insert to honor a pre-assigned index, placing the node at that slot and falling back to an append only when the slot is already occupied, so the layout is established by the configuration rather than by insertion order. The remaining coupling - plm handing out daemon vpids sequentially rather than reading node->index - is recorded in the component guide. Signed-off-by: Ralph Castain <rhc@pmix.org>
In a bootstrapped DVM the daemons are not launched by anyone - they come up independently on every node and phone home. Each one derives its own vpid from the configuration file (prte_bootstrap_my_identity) before it ever contacts the controller, and prted_report_launch then looks a reporting daemon up in daemons->procs by the rank it claims for itself. The HNP therefore does not get to choose a vpid; it has to arrive at the same answer independently, from the same authority. setup_virtual_machine instead handed out the next sequential vpid. That agrees with what the daemons call themselves only because DVMNodes happens to be listed in rank order - a coincidence of insertion order, not a derivation. Where it disagrees the HNP either attaches a daemon to the wrong node or fails to find it at all. Read the canonical rank, which ras/bootstrap records as the node's pool index, and grow num_procs to cover the vpid actually used rather than incrementing it blindly: num_procs is the vpid span, and is a count only because vpids are normally consecutive. For a sequentially assigned vpid the new expression is exactly the increment it replaces. Contiguity is what makes that span still equal the daemon count, so the DAEMONS_REPORTED gate continues to fire. It is guaranteed by the configuration, and prte_bootstrap_parse now enforces it: a host listed twice in DVMNodes resolves to the position of its first occurrence, which leaves a rank that no daemon will ever claim while prte_bootstrap_num_daemons still counts it. Such a DVM can never finish forming, so reject the file at parse time - on every daemon, not just the controller, and with a message naming the offending host. Add unit coverage for the rank contract the whole chain rests on: contiguous ranks over a well-formed list, a controller entry consuming no rank when it appears mid-list, rank_of and host_of_rank being inverses, and the duplicate case that breaks all of it. Signed-off-by: Ralph Castain <rhc@pmix.org>
build.sh distcleaned the source tree on every invocation, which is a bigger cost than it looks: it destroys the developer's in-tree build, so the next `make check`, `make -C test/offline check-offline` or `make install` silently has nothing to do until the tree is reconfigured and rebuilt from scratch. A distcleaned tree does not announce itself - `make check` reports "No rule to make target 'check'" and a root `make` exits 0 having done nothing - so the failure reads as something else entirely. It cannot simply be skipped, though. The obvious reason for it, that a VPATH configure refuses to run against a source tree holding an in-tree build, applies only to the first run. The reason that actually bites applies to every run: automake sets VPATH to the source directory, so an incremental out-of-tree make resolves object targets out of the SOURCE tree and links whatever *.lo it finds there. Between a macOS host tree and the Linux container those are not even the same architecture, and the build dies with "'foo.lo' is not a valid libtool object". So keep the distclean, but do it only when there is actually an in-tree build to remove, say why, and point at the durable fix - build the host side out of tree as well and there is nothing to clean. Add --distclean and --no-distclean to override the decision, and record the rule and its two reasons in AGENTS.md. While here, apply the show_help golden rule to both out-of-tree builds. prte_show_help_content.c embeds every help-*.txt in the tree but its make rule depends only on the converter script, so an edited help file is never picked up by an incremental build - and these build directories persist across runs, in the docker volume and in vpath-macos. A daemon would then serve stale or missing help text while the .txt looked correct, which is exactly what a new bootstrap diagnostic hit. Also extend the swarm cleanup to /tmp/prted.* and /tmp/pmix.*. Each tool has its own session-dir prefix, and a bootstrapped daemon standing on its own uses prted.<pid>; leaving those behind makes a later prun report "multiple possible servers" and fail to find the DVM. Signed-off-by: Ralph Castain <rhc@pmix.org>
The rework that thread-shifted the scheduler's answer left the array it carries owned by PMIx, released through the request's rlcbfunc. That was correct before, when the requester's callback was handed that release function and eventually invoked it. It is not correct now: the callback is handed prte_pmix_server_req_release instead - so that the request outlives a callback still reading its info - and nothing ever calls PMIx's release, leaking the answer on every allocation request that a scheduler grants. Forwarding the array is awkward for a second reason too: prte_ras_base_complete_request may repoint req->info at a response of its own, so even the old arrangement was handing the requester one array and a release function belonging to another. Copy the answer instead. The request then owns everything downstream and its destructor frees whatever req->info ends up being, and PMIx's array goes back immediately, at the point where its lifetime is obvious. Signed-off-by: Ralph Castain <rhc@pmix.org>
ras/flux is gated on finding Flux *and* jansson, and ras/slurm compiles its ~1000-line "scontrol --json" parser only when jansson is present - and jansson defaults to --with-jansson=no. In practice that means neither body of code is compiled by any ordinary developer build or CI job, so an edit to them can sit broken indefinitely. Both were, in ways a compiler catches instantly: ras/flux included <flux/core.h> ahead of prte_config.h, against the mandatory header-order rule, and passed an int index to json_array_foreach, which compares against a size_t and so fails the tree's own -Werror. Compiling it also turned up a strdup into prte_job_ident that never released the previous value. Extend --enable-testbuild-launchers, which already covers ras/lsf, to both: add declaration-only stand-ins for jansson (shared, in base/) and for the Flux core/hostlist/idset headers, select them under the same #if the LSF component uses, and let ras/flux configure under the option. Nothing in a stub is implemented, so such a tree must not be run - it segfaults during prte_init, because ras/flux builds statically and its query calls flux_open_ex. Say so in src/mca/ras/AGENTS.md and in the configure-option table at the top level, with the backtrace, since the failure is neither graceful nor confined to the component - and note there that installing such a tree poisons a later run of a good one. Also distribute testbuild_lsf.h, which appeared in no _SOURCES list and so was missing from a distribution tarball. Signed-off-by: Ralph Castain <rhc@pmix.org>
send_alloc_resp is handed to the RAS as an allocation request's completion callback, and can therefore be invoked on the PMIx progress thread. The golden rule for such a callback is that it captures its arguments and posts an event, nothing more: a prte_pmix_server_req_t is a PRRTE object owned by the PRRTE progress thread. Instead it recorded the status on the request, freed the info array the request was carrying, and rewrote three more of its fields before shifting. Carry those across in a caddy that holds a reference on the request, and move every mutation into the shifted handler. This is the same fix, and the same shape, as the one just applied to ras/pmix's answer callback. While there, route the handler's pack failures through its cleanup tail instead of returning early: each of those returns leaked the request and never released the results back to PMIx. Signed-off-by: Ralph Castain <rhc@pmix.org>
A handful of small defects found while reviewing the framework, none of
them worth a commit on its own.
ras/hosts, in the hand parser that serves --add-hostfile:
- a "slots=+N" adjustment to a node already in the pool clamped at zero
but not at slots_max, unlike every other slot adjustment in the tree,
and never told prte_ras_base.total_slots_alloc - which a managed
allocation reports to applications as PMIX_UNIV_SIZE / PMIX_MAX_PROCS,
so the DVM's idea of its own size drifted from the pool it describes;
- an explicit "slots=0" on a new node fell into the same branch as "no
slots clause given", so instead of contributing no slots the node was
silently re-sized from its core count when its daemon reported in;
- the alias search compared the raw name from the file while the name
search compared the local-host-resolved one, so a hostfile entry
naming this host by an alias did not match.
ras/pbs recorded the job id in prte_job_ident without releasing a
previous value. ras/gridengine reported "no nodes found" when the real
failure was that it could not open PE_HOSTFILE, hiding the cause.
ras/slurm passed the return of pmix_pointer_array_add - an index - to
prte_pmix_convert_status, yielding a meaningless error code, and released
its session stack without a NULL check on a path where init may have left
it unset.
Signed-off-by: Ralph Castain <rhc@pmix.org>
The ras unit test referenced prte_ras_pbs_module and prte_ras_gridengine_module directly. Those components are only built when configure detects PBS or Grid Engine, so on an ordinary machine the symbols do not exist and `make check` failed to link - taking the whole suite with it. ras/slurm has the same property: it is skipped on platforms Slurm does not support, and --without-slurm removes it. Reference only the components that have no configure.m4 and are therefore always built, and guard the Slurm-specific checks on a new PRTE_HAVE_RAS_SLURM conditional. The gated components are still covered structurally by the selection test, which walks whatever the framework actually built rather than assuming. Signed-off-by: Ralph Castain <rhc@pmix.org>
Two gaps in what the swarm actually exercised. Jansson was absent from the image, so the container build compiled ras_slurm_jansson_stub.c and the real "scontrol --json" parser - the whole Slurm elastic extend/release surface - was never built here either. Install libjansson-dev and pass --with-jansson, which is off by default and so has to be asked for explicitly. And nothing covered the bootstrap DVM. It is the one launcher-less path: prted comes up independently on every node and derives its own vpid from prte.conf before it ever contacts the controller, so the controller has to arrive at the same numbers from the same file - and prted_report_launch looks a reporting daemon up by the rank it claims. Add a test that stands one up across four nodes and checks each daemon's vpid against its position in DVMNodes, that a job runs over it, and that a DVMNodes list naming a host twice is rejected with a diagnostic rather than forming a DVM that can never complete. prte.conf lives in the install the swarm shares and the node containers mount it read-only, so the helpers write it through a throwaway container and always restore the original. Signed-off-by: Ralph Castain <rhc@pmix.org>
--enable-testbuild-launchers builds ras/flux and ras/slurm's ~1000-line "scontrol --json" parser against declaration-only stub headers, so their objects come out with unresolved flux_*, json_*, hostlist_* and idset_* references. Both components link statically into libprrte, so those references landed in the library every PRRTE tool links against, and the first tool to be linked took the whole build down: /usr/bin/ld: ../../../src/.libs/libprrte.so: undefined reference to `flux_open_ex' which is every CI build job, all of which configure with the option. The stubs that came before these - ras/lsf, plm/lsf, plm/tm - never had the problem because they are in the default --enable-mca-dso list, and a run-time loadable plugin may carry unresolved symbols: creating a shared object does not require them to resolve, and the loader simply refuses the dlopen later. So add ras-flux and ras-slurm to that list. That is the right layout for them anyway - it keeps libflux-core and libjansson out of libprrte, and so out of every PRRTE tool, exactly as it already does for LSF. Record in the comment above the list what the list is actually for, since getting a component wrong here breaks the build for everyone. Being a plugin is necessary but not sufficient: dlopen refusing to resolve is a Linux guarantee, not a universal one. On macOS a plugin is a flat-namespace bundle that loads happily, so ras/flux's query - alone among the ras components in not first checking whether this machine is even its environment - called straight into a stub and segfaulted prte_init. Gate it on FLUX_URI (or an explicitly-set broker_uri) the way ras/lsf gates on LSB_JOBID. Outside a Flux instance the open could not have succeeded anyway, so this costs nothing and saves a broker probe on every startup everywhere else. Building ras/flux as a DSO for the first time also turned up a stale @PRTE_BINARY_PREFIX@ in its LIBADD, a substitution this tree does not define; make it point at src/libprrte.la like every other component. Finally, the ras unit test references prte_ras_slurm_module and the Slurm taint validators directly, which a plugin no longer puts in libprrte - and on Darwin a bundle cannot be linked into an executable at all. Gate those checks on the component being static and say how to get them back (--enable-mca-static=ras-slurm). Signed-off-by: Ralph Castain <rhc@pmix.org>
ras/slurm became a run-time loadable plugin, which put its symbols out of reach of the unit test - it had referenced prte_ras_slurm_module and three helpers directly, and linking the plugin is not an option since on Darwin a module is a bundle that cannot be linked into an executable at all. Reach it the way the DVM does instead: find the component in the framework's component list, call its query, and drive the module it hands back. That names no component symbol, so it works whether ras/slurm was linked statically or loaded as a plugin, and it needs nothing from the build system to arrange. Doing so also tests considerably more of the component than the direct calls did. query gating on SLURM_JOBID and allocate() expanding a compressed SLURM_NODELIST were both uncovered; the new test asserts the priority-50 offer, that "node[01-04]" with "4(x4)" expands to four zero-padded names of four slots each, and that a second discovery of the same jobid reports PRTE_EXISTS with nothing added - SLURM_NODELIST is per-job, so returning success there would insert the allocation twice. The taint checks move to their real call sites: a jobid carrying a shell metacharacter, a substitution or a non-digit is refused because tag_node_allocation and assign_new_session validate it, and an over-length nodelist is refused by check_taint. This is the whole of what the component can do when jansson is absent, which is the case for any ordinary `make check` build. The modify surface declines outright without jansson, shells out to sbatch and scontrol, and only means anything across several nodes, so it belongs to contrib/dockerswarm - the only automated build that configures --with-jansson - once that harness fakes a scheduler. validate_hostname and drain_cmd_output are reachable only from that path and go with it; say so in both AGENTS.md files rather than leaving the gap unrecorded. When the component is not there to be found the test says so and why, rather than passing quietly: an uninstalled tree has no plugin to load, and neither does a --enable-testbuild-launchers build on a platform whose loader refuses a module with unresolved symbols. Signed-off-by: Ralph Castain <rhc@pmix.org>
Six of the nine unit tests named a component's module symbol directly -- prte_rmaps_round_robin_module, prte_plm_ssh_module, and so on -- and several also named its classes and internal functions. That works only because the default build links every component into libprrte. Configure with --enable-mca-dso and the component is a separate DSO instead, the symbol is not there to link against, and the test does not build at all: "make check" fails to link twenty-one symbols across six binaries, so that configuration has no unit test coverage whatsoever and has not had any since these tests were written. Ask the framework instead, which is how a module is reached in production. Each test now walks its framework's component list for a component of the name it wants and calls that component's query, exactly as the framework's own select does; the rmaps tests take their mapper from prte_rmaps_base.selected_modules. Nothing is linked at build time, so it works whether the component was compiled into the library or dlopened from a DSO. Two distinctions had to be drawn to make that honest. A component being present is not the same as it yielding a module: iof/prted and errmgr/prted answer only a daemon, so the tests that want their vtable now ask as the process type the component serves, and the identity checks ask the separate question of whether the framework opened a component of that name. And a component that this build did not produce is not a failure -- those cases skip, and say which component was missing. The cases that genuinely reach inside grpcomm/direct -- its own classes and the identity of its entry points -- cannot be expressed through any framework interface, so they are compiled in only when that component is part of libprrte, keyed off the MCA_BUILD_prte_grpcomm_direct_DSO conditional the build system already defines. They still run in full in a default build. Verified on Linux both ways: a default build passes 9 of 9 as before, and an --enable-mca-dso build now passes 9 of 9 where six could not be built. The DSO run still cannot exercise the components themselves, because the tests run from the build tree and MCA has no path to the component libraries there -- PMIx hands its tests PMIX_COMPONENT_LIBRARY_PATHS for this and PRRTE has the same config/mca_library_paths.txt but no substitution using it. Those cases now announce the skip rather than passing in silence; closing it is separate work. Signed-off-by: Ralph Castain <rhc@pmix.org>
PRTE_CHECK_VISIBILITY probes the compiler, records the flag in PRTE_VISIBILITY_CFLAGS, and then never uses it: the variable is not AC_SUBSTed and never reaches CFLAGS, so a tree configured with --enable-visibility is built with every symbol exported just the same. The only thing the probe accomplishes is defining PRTE_C_HAVE_VISIBILITY, which decides whether PRTE_EXPORT expands to a visibility attribute -- an attribute that means nothing while nothing is hidden. PRRTE ships no linkable library, so this has cost us nothing in practice; what it costs is the next person to enable the option, who gets a build that quietly ignores it and no clue why. Put the flag on CFLAGS, and default the option to off. PRRTE installs no library for anyone to link, so hiding its internals buys little on its own, and turning it on by default would change what libprrte exports for every existing build; the point here is that asking for it does something. Five symbols cross a boundary the compiler then enforces and were never annotated: prte(), which include/prte.h declares and every prte binary calls -- the tool is a three-line main() around it, so the one function the library most clearly means to export was the one that was not; prte_pmix_server_globals, reached by prun; prte_odls_spawn_caddy_t and prte_pmix_grp_caddy_t, classes declared in their frameworks' headers and constructed by the unit tests; and prte_pmix_server_pset_t, which every component DSO needs -- without it grpcomm/direct cannot be dlopened at all, so no component loads, no grpcomm module is selected, and a --enable-mca-dso DVM dies at startup with a clean compile and a clean link behind it. That is the whole cost. Comparing each component DSO's undefined symbols against libprrte's export table leaves nothing else missing, which says the PRTE_EXPORT and PRTE_MODULE_EXPORT annotations in this tree were written carefully by people who assumed the option worked. The unit tests keep their coverage: the cases that reach inside grpcomm/direct stand down when the library hides its internals, since a static component compiled with -fvisibility=hidden is no more reachable from a test than a DSO is. That is the same gate they already carry for DSO builds, extended by the PRTE_HIDE_INTERNALS conditional added here. Verified on Linux with visibility both off and on, each with and without --enable-mca-dso: make check passes 9 of 9 in all four, and prterun launches in all four. Signed-off-by: Ralph Castain <rhc@pmix.org>
PRRTE tells the MCA base about exactly one place to find its components: "prte@" followed by the installed library directory, and only when that directory already exists. There is therefore no way to run against components that have been built but not installed. PMIx has offered component_path for its own components all along; PRRTE simply never offered the equivalent, and the gap is not only a testing inconvenience -- anyone wanting to try a component out of a build tree, or from a directory alongside the installation, has nowhere to say so. Honor PRTE_MCA_mca_base_component_path, prepending it to the installed location so an explicit request wins over whatever is installed. The immediate beneficiary is "make check". Unit tests run from the build tree, so with --enable-mca-dso every component is a DSO that MCA cannot find, no framework opens anything, and every component-dependent case can only skip -- the tests were passing while covering nothing. configure now turns the component directory list autogen.pl writes into config/mca_library_paths.txt into absolute paths under the build tree, and each test directory hands it to the tests through AM_TESTS_ENVIRONMENT. No test source changes. With this, a --enable-mca-dso "make check" runs the mapper tests, the module contracts and the component identity checks for real rather than reporting them skipped: 9 of 9 pass in both a default and a DSO build, and the DSO run reports no skips at all. Signed-off-by: Ralph Castain <rhc@pmix.org>
test/unit/ras names five component module symbols, which puts it back in the position the rest of test/unit was moved out of: with --enable-mca-dso those components are separate objects, the symbols are not there to link against, and make check cannot build the test at all. The file already carries find_ras_component() for exactly this reason -- ras/slurm is a plugin -- and its own header comment says no component symbol is named anywhere in it. Apply that helper to the other five. Asking a component for its module is not quite the same question as reading its module symbol: ras/simulator, ras/testrm and ras/bootstrap all decline a query outside the situations they serve, so going through the framework alone would quietly stop checking their vtables. Keep the symbol as a fallback, compiled in per component from the MCA_BUILD_prte_ras_*_DSO conditionals the build system already defines, the way this directory already gates PRTE_TEST_HAVE_RAS_SLURM. A default build therefore checks all five exactly as before; a DSO build checks whatever the framework can hand it and names what it skipped. The framework also has to be open before the contract test runs, which it was not -- test_select() opened it, and the contract test ran first, so walking framework_components segfaulted. Open it in main(). Signed-off-by: Ralph Castain <rhc@pmix.org>
A DVM grow launches daemons on the nodes in PRTE_NODE_STATE_ADDED and only those: prte_plm_base_setup_virtual_machine selects on that mark so that an allocation request is scoped to the nodes it was granted, and every other producer of a grow - ras/hosts, the no-scheduler insert path, and this component's own reused-node branch - sets it for exactly that reason. The JSON path did not. It handed its nodes over as plain UP, the state an initial discovery uses, so every node Slurm granted through an extend landed in the global pool with no daemon on it and none was ever started: the request reported success and added resources the DVM could not use. Only the reused-node case worked, because that branch marks ADDED itself. Mark the newly granted nodes the same way. Found by the fake-SLURM coverage in contrib/dockerswarm, which drives a real extend across the container nodes and then looks for the daemons. Signed-off-by: Ralph Castain <rhc@pmix.org>
prte_ras_slurm_exec_sbatch reads the submitted job ID a byte at a time and sets pipe_draining once it sees the first character that cannot be part of it. From that point the read loop had no branch for a successful read: a returned byte matched neither the "1 == r && !pipe_draining" case nor "0 == r" nor the EINTR case, and fell into the else that reports PRTE_ERR_PIPE_READ_FAILURE. So any sbatch whose output carried more than one character past the job ID failed the whole extend, and the expander job it had just submitted was cancelled again. That is not a corner case. It is what "--parsable" - which this code passes deliberately - produces on any cluster that reports a cluster name: "<jobid>;<cluster>". Only the bare "<jobid>\n" form survived, because there the very next read returns EOF. Consume the remaining output instead. It still has to be read so the child is not left blocking on a full pipe; it just is not an error. Signed-off-by: Ralph Castain <rhc@pmix.org>
ras/slurm is the only ras component with a full elastic modify surface, and everything past initial discovery is a shell-out: sbatch to grow, "scontrol show job --json" to learn what was granted, "scontrol update job ReqNodeList=" to shrink one in place, scancel to give one back. None of that runs on a developer machine, so none of it had any automated coverage - including the JSON parser, which this harness is also the only automated build that even compiles, since jansson defaults to off and only build.sh here passes --with-jansson. validate_hostname and prte_ras_slurm_drain_cmd_output exist solely on that path and were likewise never exercised. Supply the missing scheduler. fake-slurm.py implements exactly the four command forms PRRTE issues, dispatching on argv[0], and build.sh installs it into the shared volume as sbatch/scontrol/scancel under its own prefix - not into the install bin/ that every node puts on its default PATH, so a test has to opt in and cannot perturb the rest of the suite. It hands out real container hostnames, so an extend launches real daemons on real nodes and a release really removes them. The new cases cover an extend end to end (the sbatch line PRRTE builds, including that each propagate_* MCA param gates its own attribute and that an unset numeric field is omitted; the slot count derived from counting ALLOCATED core statuses; the daemons that must appear), release by node list, by allocation id and by count, cancellation of a request still waiting on a PENDING job, and the paths that exist only to survive a misbehaving scheduler: unparsable JSON, a scancel that fails with more output than the capture buffer holds, and a release naming a hostname carrying a command separator, which must be refused before it can reach a command line. The elastic client grows the request shapes this needs. A node-naming PMIX_ALLOC_NEW never reaches an RM component, so add extend/release/ release-id/cancel for the NUM_NODES, ALLOC_ID and REQ_CANCEL forms ras/slurm actually serves, along with --req-id and control over the phase-two wait: an RM extend puts its nodes in the general pool rather than a reservation, so no directed completion event is coming and phase one carries the result. A failed phase one now skips that wait instead of burning the 60s timeout. Slot counts are asserted from ras_base_verbose output rather than from the node pool. Outside a managed allocation the count a node was given is recomputed from its core count before mapping, and prte_managed_allocation is presently never set for an RM allocation - so the pool cannot say whether the component parsed the allocation correctly. Signed-off-by: Ralph Castain <rhc@pmix.org>
Whether a node's slot count may be recomputed is a property of that node, not of the session: it depends entirely on whether whoever supplied it was in a position to know. PRTE_NODE_FLAG_SLOTS_GIVEN says exactly that and is already honored throughout the launch path, but only the hostfile and dash-host parsers were setting it. The resource-manager components said nothing, and relied on the base to infer it from the global prte_managed_allocation. Have each of them say it instead: slurm (from SLURM_TASKS_PER_NODE and from the cores its JSON reports as ALLOCATED), pbs (PBS_NODEFILE), gridengine (PE_HOSTFILE), lsf (the repeat count from lsb_getalloc), flux (the Flux resource set) and the simulator (which fabricates an allocation and means its sizes to be used). The bare local-host fallback deliberately still says nothing, so a plain "prterun -np 4" continues to size itself from the machine. node_insert then simply carries the flag into the pool, and prte_plm_base_daemons_reported sizes only the nodes that arrived without one and totals the job's own session nodes. It no longer has a second mode that copies prte_ras_base.total_slots_alloc wholesale, which was wrong for any job mapping onto a reservation rather than the default pool. The same removal in the plm base's local-only module fixes a defect of its own. That module is what runs when no launcher component is available, so the head node is the entire virtual machine - yet it reported the slot total of the whole allocation, advertising to the job resources on nodes it had no way to reach. Signed-off-by: Ralph Castain <rhc@pmix.org>
The reuse guard only applied when the allocation was unmanaged, so under a resource manager every job re-ran discovery. That is not something the components can afford: ras/slurm has to spend a return code (PRTE_EXISTS) telling the driver it has already recorded this jobid, and a component that did not would insert its whole node set a second time. Nothing about the reason for the guard is specific to unmanaged allocations. Re-reading a hostfile overwrites established per-node slot counts and clears PRTE_NODE_FLAG_SLOTS_GIVEN, hiding oversubscription from the mapper; re-reading a scheduler is no better. The initial daemon-job discovery is the base allocation for the session in either case, and the sanctioned way to change it is an explicit add-host/add-hostfile or allocation request through prte_ras_base_modify. So let prte_ras_base.allocation_established decide on its own. Signed-off-by: Ralph Castain <rhc@pmix.org>
setup_virtual_machine had two ways to decide which nodes make up the DVM. For a managed allocation it took the node pool and narrowed it with the union of the app specs; otherwise it assembled the list from those specs alone - a rank/seq file, -host, -hostfile, the default hostfile - and looked each name up in the pool. The second path cannot work under a resource manager. Nobody passes -host inside a SLURM allocation: the allocation *is* the node list. So that path found nothing to read and concluded the DVM consisted of the head node alone, leaving every other allocated node without a daemon. With prte_managed_allocation no longer being set for an RM allocation (it has not been since ras became multi-component), that is what a 3-node SLURM job got: one daemon, jobs oversubscribed onto the head node, and the rest of the allocation idle. Delete it. Everything it consulted is already read into the node pool at allocation time - ras/hosts handles the rank/seq file, -host, -hostfile and the default hostfile - so filtering the pool reaches the same set for those cases and the correct set for the others. The grow and dynamic-spawn paths are untouched: they select only the nodes their own request brought in. Signed-off-by: Ralph Castain <rhc@pmix.org>
get_target_nodes had the same fork as setup_virtual_machine: either build the node list from the app's -host/-hostfile and look each name up in the pool, or take the session's nodes and filter them through those same specs. Which one ran depended on whether the allocation was managed. Both reached the same set - a name absent from the pool never survived either - but only the second applies the checks that matter, notably that the head node is not usable merely because someone named it when the allocation does not include it. The first also existed only to preserve the ordering the user gave, which the filter path preserves anyway: prte_util_filter_dash_host_nodes rebuilds the list in the order the hosts were named. Keep the session-resource path and delete the other. The job's session is its allocation, and -host/-hostfile can only select within it. Signed-off-by: Ralph Castain <rhc@pmix.org>
"--host 15" selecting "nid0015" was gated on the allocation being a managed one. That is not what makes the shorthand meaningful: a bare integer is never a hostname, whoever supplied the node names, so the token itself is the signal. Decide from it, with a literal name match taking precedence so a host genuinely called "15" still resolves to itself. Three defects fall out of putting that in one helper: The scan compared its result against strlen-1 rather than strlen, so a node whose name ended in a single digit - or in no digit at all - matched any numeric token. "--host 15" selected "node1", and "node". Only the filter knew about launch ids; prte_util_dash_host_compute_slots still matched literally, so even where the shorthand selected the right node the job was then told that node had no slots and the mapping was rejected. It now uses the same helper. And the filter freed every -host token at the end of its loop whether or not anything had answered to it, which left the "at least one of the requested hosts is not included in the current allocation" report unreachable. A -host naming a node the allocation does not contain was silently dropped, and the user got a generic no-resources complaint instead of being told which host was the problem. Tokens that went unmatched now survive to be named in that report. The pool-membership check in prte_util_add_dash_host_nodes goes away with them: every remaining caller is building the allocation rather than selecting from it, so there is nothing there to check against, and the filter is where a job's -host list is now vetted. Signed-off-by: Ralph Castain <rhc@pmix.org>
Every question this global was asked has been given a local answer: the slot counts read PRTE_NODE_FLAG_SLOTS_GIVEN per node, the reuse guard reads prte_ras_base.allocation_established, VM construction and mapping take the allocation as the node set in all cases, and dash-host decides from the token it was given. Nothing reads it any more. It had also stopped meaning anything. The base used to set it whenever a module returned nodes, and that was dropped when ras became multi-component, leaving only ras/bootstrap - so on every scheduler PRRTE supports the flag has been false, and each of the sites above has been taking its unmanaged branch. Restoring the assignment was the other way to fix that, but it would have had to guess which components count as managed now that ras/hosts is one of them, and the per-node and per-session facts above are answers rather than guesses. The nidmap carried it to the daemons, purely so they could answer questions none of them ask; the pack and unpack go together, which is safe because every daemon in a DVM is the same build. ras/bootstrap set it for the two behaviors that are now universal, and already marks its nodes SLOTS_GIVEN for the third. The framework guides are updated to describe the model that replaces it. Signed-off-by: Ralph Castain <rhc@pmix.org>
The fake-SLURM phase drove ras/slurm's modify surface but never asked the more basic question of what PRRTE does with an allocation once it has read one, because until this harness existed there was no way to ask it without a real scheduler. These cases were written first and every one of them failed. A three-node allocation with no --host given: daemons on all three nodes, a job spreading across them, and the slot count SLURM specified surviving to the point of mapping rather than being replaced by each node's core count. Then -host inside the allocation, -host outside it (refused, naming the host), and the bare launch id. Finally an allocation that excludes the head node, where a job must stay on the allocated nodes and naming the head node is an error. The distinct-node counting matters in the first of those: three procs that all landed on the head node is precisely the failure being tested for, and would satisfy a line count. Signed-off-by: Ralph Castain <rhc@pmix.org>
"prun --host +n1" resolved to the right node and was then refused for
lack of resources. prte_util_dash_host_compute_slots matched each -host
token literally against the node, so a relative token matched nothing,
every node reported zero available slots, and the mapper rejected the
job. It ran only under --map-by :OVERSUBSCRIBE, which skips the check -
which is also what made the failure look like a mapping problem rather
than an accounting one.
Relative syntax names nodes without saying anything about them, and
cannot carry a slot count of its own: the colon in "+e:N" is a node
count. So a node designated by such a token contributes the slots it was
discovered to have - the same answer a job that named no hosts at all
gets - and remains bounded by them.
The resolution here deliberately carries no diagnostics. By the time
slots are computed the token has already been resolved against the node
pool by prte_util_filter_dash_host_nodes, which reports anything wrong
with it once rather than once per node.
Also delete the copy of the relative-node expansion in
prte_util_add_dash_host_nodes. It was unreachable - that routine only
ever runs while an allocation is being built, where a relative
specification means nothing, and every caller passes that way - and
prte_util_filter_dash_host_nodes has the live implementation. Its
"allocating" parameter had no other use and goes with it.
The two copies had drifted, which is the argument for having one: the
dead one read a count out of the token itself ("+e2"), the live one
requires "+e:2" and turns a bare "+e" into an all-empty-nodes marker.
Finally, bound the suite's pterm calls with "timeout -k": PRRTE tools
trap SIGTERM in order to forward it, so plain timeout(1) cannot end a
wedged one, and a hung pterm stalled the whole run instead of failing
its own case.
Signed-off-by: Ralph Castain <rhc@pmix.org>
Fork Sync: Update from parent repository
Relative node syntax counts the allocation from zero, so "+n0" is the first node the job was given. The head node always occupies node-pool slot 0 whether or not it was allocated, so when it was not, every index has to be shifted by one to skip it. Both dash-host's parse_dash_host() and prte_util_get_ordered_host_list() do that; the hostfile filter did not. So the same "+n0" meant different nodes depending on how it was written: on the command line it selected the allocation's first node, in a hostfile it selected the head node the job was never given - and every subsequent index was one node adrift. The two are documented as interchangeable. Verified in the swarm with an allocation that excludes the head node, which is the only arrangement in which the two disagree. Signed-off-by: Ralph Castain <rhc@pmix.org>
The relative-indexing page contradicted itself, and both the code and
the rankfile page it points at. "+n#" indexes the allocation from zero -
detail-placement-rankfiles.rst says so outright, and the worked hostfile
example on this page gets it right in its results ("+n2" resolving to
dummy3, the third node) - but the prose around them counts from one.
The first example asked for "the first two nodes" of foo1..foo4 and then
wrote "--host +n1,+n2", which selects the second and third; its
"+n3,+n4" ran off the end of the allocation and would have failed with
"relative-node-not-found". The hostfile example described "+n2" as the
second node while its own expected output named the third. Correct the
prose and the example, and state the base of the index explicitly.
Also record what --host does and does not do. It selects among the hosts
the DVM already has - a resource manager's allocation, or what the DVM
was started with - and naming anything else is now refused by name
rather than silently dropped, so point at --add-host/--add-hostfile as
the way to bring in a host that is not there yet.
Signed-off-by: Ralph Castain <rhc@pmix.org>
The forward-signals parser accepts a signal by name or by number, and the two forms are separate branches. The name branch rejected an unknown name; the number branch rejected only what strtoul itself refused, which is very little. "-1" parsed cleanly and wrapped, "999" parsed cleanly and is not a signal on any platform PRRTE runs on, and both were then handed to prte_event_signal_add -- which fails, and whose return nobody reads. So a user who asked to forward a signal PRRTE could not deliver got no forwarding and no diagnostic at all. Range-check the number the way the name branch checks the name, using the same _NSIG idiom src/util/stacktrace.c already uses for this. Also latch the one-shot guard only once the list has been built rather than on entry. Latching first spent the single allowed pass on a request that was rejected partway through -- leaving whatever it had already appended installed with no way to replace it -- and made every rejection in the function unreachable a second time, and so impossible to test. And drop the numeric comparison in the name branch: sval is still zero there, and no signal is zero, so it could never match. Signed-off-by: Ralph Castain <rhc@pmix.org>
Each of the four daemon modules derived its own name from the parameters the launcher published, and each wrote the same shorthand to do it: strtoul() on ess_base_vpid with no check, and atoi() on the RM's node index with no check beyond a NULL guard. That shorthand has a silent and severe failure mode. Any value that is not a number reads as zero, and rank 0 is the DVM controller -- so a daemon handed a garbled vpid adopts the HNP's own identity, and the DVM comes apart later in ways that point nowhere near the input that caused it. A value too large to be a rank was equally unchecked, and LSF's one-based task index with its -1 adjustment could wrap a zero index around to a rank near UINT32_MAX. Replace all four with prte_ess_base_set_identity(offset_envar, offset_adjust), which loads the nspace, parses the base vpid, adds the per-node index the RM exports, applies the adjustment LSF needs, and publishes num_daemons. Every input is validated and a bad one is refused with a diagnostic naming it. The sum is computed and compared as a signed long rather than a pmix_rank_t, because a narrowing cast would truncate an out-of-range sum back into the valid range instead of catching it. The refusals return PRTE_ERR_SILENT, since they have already shown their own message and the module's error path would otherwise print a second, generic one on top of it. What is left in each module is only the part that was ever environment-specific: which variable names the node index, and slurm's correction of the local hostname to what SLURM itself reports. This also makes ess/lsf and ess/pals compile again. Neither is built on a developer machine, and both had drifted into failing the project's -Wall -Wextra -Werror: lsf's rte_init was missing its PRTE_HIDE_UNUSED_PARAMS and carried an unused my_node_rank static, and pals declared a char *tmp it never used. Signed-off-by: Ralph Castain <rhc@pmix.org>
ess/slurm replaces prte_process_info.nodename with what SLURM itself calls this node, so that the name the daemon reports matches the one the HNP saw in the allocation. It freed the existing string first and only then looked for SLURMD_NODENAME, so when that variable was absent the function returned an error having left the global pointing at freed memory. Nothing then stops using it. The error travels up through the module's own error path, which formats a diagnostic, and on out through prte_init's failure handling and finalize -- all of which read the nodename freely, because there is no state in which it is expected to be invalid. Look the replacement up before discarding what we have. Signed-off-by: Ralph Castain <rhc@pmix.org>
Two steps in prte_ess_base_prted_setup deliberately set PRTE_ERR_SILENT
after failing -- the PMIx server init ("the server code already barked,
so let's be quiet") and the schizo personality selection, which has just
shown a no-proxy message naming the personality it could not find. The
shared error label then showed the generic
prte_init:startup:internal-failure help unconditionally, so the specific
diagnostic was immediately followed by a vaguer one describing the same
fault. Every ess module's own error path already guards that message
with PRTE_ERR_SILENT != ret && !prte_report_silent_errors; the shared
daemon path is now consistent with them.
While here, release the forwarded-signal event array on that path. It is
allocated before any of the steps that can fail and freed only in
prte_ess_base_prted_finalize, which a failed bring-up never reaches.
Signed-off-by: Ralph Castain <rhc@pmix.org>
Almost all of ess is bring-up by construction, but two pieces are pure input parsing and both now have a real failure mode worth pinning down. prte_ess_base_set_identity is driven over the shapes of all four daemon modules and, more usefully, over the inputs that must be refused: non-numeric, trailing garbage, empty, negative, missing, and sums that fall outside the rank space. Each of those asserts the call was refused AND that it left no rank behind, because the failure being guarded against is the quiet one where garbage reads as rank 0. Those cases earned their keep immediately: they caught the range check comparing a value that had already been narrowed to pmix_rank_t, so a sum past the end of the rank space was truncated back into the valid range rather than rejected. prte_ess_base_setup_signals gains a table of requests that must be refused -- unknown name, non-forwardable by name and by number, non-numeric, negative, zero, out of range, and a bad entry inside an otherwise good list. These can only be tested now that the one-shot latch is set on success rather than on entry. They run before the accepting case for the same reason. Signed-off-by: Ralph Castain <rhc@pmix.org>
prte_ess_base_prted_setup installed a handler for every signal on prte_ess_base_signals and carried a signal_forward_callback to relay each one to the local processes. That list is always empty in a prted. Nothing calls prte_ess_base_setup_signals in a daemon -- only prte and prun do, each for itself -- and the plm does not forward the ess_base_forward_signals parameter to daemons either, so the block could never install anything and the callback could never run. Signal forwarding is a tool-side feature. prte/prterun and prun build the list from their own --forward-signals option, catch the signal themselves, and relay it to the DVM; each daemon receives it as a PRTE_DAEMON_SIGNAL_LOCAL_PROCS command on the RML, and prted_comm.c is what delivers it to the application processes. That path is unaffected. This was not harmless. A 2021 regression in the unreachable callback -- packing a wildcard nspace where the signal number belongs -- was found and fixed years later in code nothing executes, and the block invites anyone reasoning about signal delivery to start in the wrong place. A multi-node test written against it would signal a prted and simply kill it, since the daemon has no handler for the signal at all. The SIGTERM/SIGINT/SIGPIPE handlers a daemon really does install stay. Signed-off-by: Ralph Castain <rhc@pmix.org>
ess IS the bring-up, so almost all of it needs a live DVM by construction, and the two pieces that are pure parsing are covered without one by test/unit/ess. Three things are left for the swarm. A daemon's rank is ess_base_vpid plus a per-node index. Getting that sum wrong makes two daemons claim the same rank, and the failure is silent -- the DVM simply loses a node, with no error anywhere -- so the case asserts that a job mapped one-per-node reaches as many DISTINCT hosts as there are nodes. A daemon given an unusable identity must die saying so and must not join, which is the end-to-end form of the validation the unit test drives directly. And a bad --forward-signals request must be refused through a real tool invocation, by number as well as by name, since those are separate parse branches. Deliberately absent: any case that signals a prted and expects the signal to reach a process. Daemons install no handlers for forwarded signals -- that path is tool-side and test_event already covers the cross-node relay -- so such a case would simply kill the daemon. The phase says so, since it is the obvious test to reach for and it does not work. Signed-off-by: Ralph Castain <rhc@pmix.org>
A collection of small things this review turned up, none of which is a live failure but each of which is a trap left set: prte_set_job_data_object's return was discarded in the daemon path and assigned-but-not-tested in the HNP. It can fail, and everything that follows looks the daemon job up by nspace, so a failure there surfaces much later as an unrelated lookup returning nothing. Check it. prte_process_info.num_daemons was assigned from ess_base_num_procs unconditionally. That parameter is an int defaulting to -1 while num_daemons is a pmix_rank_t, so a daemon started without it replaced proc_info's sane initial value of 1 with 4294967295 -- which is what nidmap would then size its span from. Every launcher passes the parameter, so take it only when it is really there. The HNP called prte_errmgr.finalize() unguarded where the daemon path NULL-checks it; give both the same guard. The comment describing the errmgr's first-stage shutdown had drifted onto the plm close two lines below the call it describes. And log_path was a file-scope static that is freed before the next statement runs, so make it the local it is. Finally, the bootstrap namespace was built with pmix_asprintf and used without checking it: on failure that hands a NULL to PMIx_Setenv and to the URI synthesis. Signed-off-by: Ralph Castain <rhc@pmix.org>
prte_ess_base_signal_t carried a can_forward bool alongside the name and number. Neither the constructor nor ESS_ADDSIGNAL ever assigned it and no code ever tested it, so every item on the list held an uninitialized value in that field, waiting for someone to trust it. The field also duplicated a decision that is already made elsewhere and made earlier: forwardability is a property of known_signals[], checked during the parse, and an entry only reaches this list once it has passed that check. A copy on the list could only ever disagree. Signed-off-by: Ralph Castain <rhc@pmix.org>
Document the single validated identity helper the four daemon modules now share and why it validates rather than trusting strtoul; the range check in the signal parser and why the two parse branches have to stay in step; that signal forwarding is a TOOL-side feature, with the list a daemon never populates and the relay that actually reaches processes; the rule that a show_help and a non-silent return code together mean the user gets two messages; and the rule that a help file must be named with its .txt, since the lookup is an exact match and a mangled name resolves to nothing. Rewrite the framework's testing section around what now exists -- the two parser unit tests, the multi-node phase, and the citation check -- and record in the dockerswarm guide why that phase asserts what it does, and why the obvious signal test is not among it. Signed-off-by: Ralph Castain <rhc@pmix.org>
Neither component is built on a developer machine, and both had drifted into not compiling at all under the project's -Wall -Wextra -Werror: a missing PRTE_HIDE_UNUSED_PARAMS and an unused static in ess/lsf, an unused local in ess/pals. That is what happens to code nothing compiles, and the fix for the code is worth very little without a way to keep it from happening again. These two are the easy case. Unlike the plm/ras launchers they need no stub headers and link nothing -- ess/lsf's own Makefile.am has said for years that "the LSF plugin does not call any LSF library functions" -- because their entire dependency on that RM is a getenv in the module and another in the component query. So the configure gate was the only thing standing between them and being compiled everywhere, and opening it under the existing flag is the whole change. They stay out of the DSO list and link into libprrte as usual, since they carry no unresolved symbols to keep out of it. The option's help text said "PLM launchers", which has not been true since ras joined it; say launcher components, and name ess as well. While in that file: the default --enable-mca-dso list named ess-alps, plm-alps, plm-tm and ras-alps, none of which have existed for some time. Signed-off-by: Ralph Castain <rhc@pmix.org>
prte_hwloc_base_close() had no callers anywhere in the tree, while src/hwloc/AGENTS.md and test/unit/hwloc both described it as live teardown. It could not be called, and the reason is an ownership question nobody had answered. The only two callers of prte_hwloc_base_get_topology() -- ess/hnp and the shared daemon bring-up -- each immediately wrap the result in a prte_topology_t and add that to prte_node_topologies, whose destructor releases the userdata and destroys the topology. So the array owns every topology it holds, the local one included, and prte_hwloc_topology is a borrowed alias of one of its entries. prte_hwloc_base_close() destroyed it as well, which is a double free on either side of the array release: called first, the array would later destroy freed memory; called after, it would. There was no correct place to call it from, so it was called from nowhere -- and the leak it was supposed to prevent went with it. prte_init() calls prte_hwloc_base_open(), so the base was opened and never closed, and prte_hwloc_default_cpu_list leaked on every run using a cpu-set. Have close() clear the alias instead of destroying it. That is correct in either order, so the ordering stops being a trap, and prte_finalize() now calls it once the topology array is gone. The unit test drives both orders and both leave the process standing; against a debug PMIx a double free here aborts, so surviving the sequence is the assertion. Restoring the destroy turns it red. Signed-off-by: Ralph Castain <rhc@pmix.org>
Three small things, all of the same shape as the rest of this review -- an input trusted, or a piece of state left inconsistent with the thing it describes. A DVMNetworks prefix length was read with a bare strtoul, so anything unparsable became 0 -- and 0 means "match every address" to the comparison it feeds, which would silently widen the network until it matched whichever interface came first and bake that into every synthesized peer URI. It is refused now, along with a length too large for its address family, which simply leaves that token out of the set of networks used to disambiguate a multi-homed host. Phase one of the bootstrap left the parsed configuration in place, and flagged valid, after rejecting it for asking for IPv6 on a build without it. Release it and clear the flag with it. And the ess framework's close destructs the signal-forward list without clearing the latch that says the list has been built, so the two disagreed about the same fact. Signed-off-by: Ralph Castain <rhc@pmix.org>
Fork Sync: Update from parent repository
prte_launcher.c is the consolidated replacement for the now-removed prte.c and prun.c tool bodies, and it had fallen behind three changes that the rest of the tree has already absorbed. As written it failed to compile. prte_parse_locals() grew a trailing pmix_cli_result_t *results argument so that the parser can hand back the job-level options (--output, --display, --rtos) it discovers; the launcher already holds that parse in a local results object, so pass it through rather than dropping the job-level directives on the floor. prte_pmix_server_register_nspace() is now asynchronous and takes a completion callback and its cbdata. The only caller here is the singleton bring-up in prep_singleton(), which has nothing to do once registration finishes, so it passes a NULL callback -- the registration path guards against a NULL cbfunc, so this simply skips the completion notification while still cleaning up. The remaining failure was subtler: prun_common() and prte_prun_parse_common_cli() were reported as implicit declarations even though both are prototyped in src/prted/prted.h, which this file includes. The cause is an include-guard collision. The daemon-only prted.h reachable through the pmix_server headers shares the same "PRTED_H" guard as src/prted/prted.h, and prte_launcher.c was including the pmix_server headers first. That defined PRTED_H before the real header was reached, so its body -- carrying the prte_parse_locals, prun_common, and prte_prun_parse_common_cli prototypes -- was skipped entirely. Reorder the includes so src/prted/prted.h is seen first, and leave a comment recording why the order is load-bearing so it is not "tidied" back into breakage. With these three changes prte_launcher.c compiles warning-free and the full tree links cleanly. Signed-off-by: Your Name <you@example.com> Signed-off-by: Howard Pritchard <howardp@lanl.gov>
|
Hello! The Git Commit Checker CI bot found a few problems with this PR: 049db08: contrib: add a Docker harness for elastic-DVM grow...
Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks! |
Signed-off-by: Howard Pritchard <howardp@lanl.gov>
|
Hello! The Git Commit Checker CI bot found a few problems with this PR: 049db08: contrib: add a Docker harness for elastic-DVM grow...
Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks! |
Signed-off-by: Howard Pritchard <howardp@lanl.gov>
|
Hello! The Git Commit Checker CI bot found a few problems with this PR: 049db08: contrib: add a Docker harness for elastic-DVM grow...
Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks! |
Member
Author
|
looks like our mirrored master got messed up. closing this PR and trying again after forced update of our mirror against upstream master. |
Signed-off-by: Howard Pritchard <howardp@lanl.gov>
Member
Author
|
changed my mind. we'll continue with this PR. |
|
Hello! The Git Commit Checker CI bot found a few problems with this PR: 049db08: contrib: add a Docker harness for elastic-DVM grow...
Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks! |
Member
Author
|
PR #172 brought in a cherry-pick commit from upstream which apparently was cherry-picked from a no-longer-existing branch in the upstream repo. So we'll ignore the PR checks CI test for this PR. |
|
Odd - I'm unaware of anything like that happening upstream. Let me know if you ever figure it out and I'll keep an eye out to avoid it in the future. |
jsquyres
approved these changes
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.