From e8866641a6844611d914afa51e094f2906dbc866 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Fri, 18 Sep 2026 11:48:08 +0000 Subject: [PATCH] feat(scheduler-sim-live-allocation): make plugin hook dispatch core-safe (task 1.2) Re-types plugin_job_election()/plugin_prepare_allocation()/ plugin_commit_decision() from (void* arg, ...) to (xce_sched_core_t* core, ...), iterating core->plugins directly instead of deriving it from a normal-mode-only driver struct -- mirroring plugin_begin_session()/plugin_end_session()'s already-completed migration. Adds a new, honestly-typed xce_sched_plugin_t.core field, set by both plugin_register() (normal mode) and plugin_register_core() (simulation mode, closing a gap left since bug-1825 where this path set nothing). The existing plugin->sched field and its plugins/fcfs.c legacy consumer are left untouched. Necessarily updates the 4 existing normal-mode call sites (scheduler.c, resource.c, schedobj.c) to pass &(...->core) -- a C signature migration cannot keep the build green without touching every caller in the same commit; each fix is a minimal argument wrap with no algorithm change, matching design.md's own Implementation Notes for this component. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC --- .../scheduler-sim-live-allocation/tasks.md | 7 +- src/scheduler/include/plugin.h | 29 +- src/scheduler/src/plugin.c | 42 ++- src/scheduler/src/resource.c | 9 +- src/scheduler/src/schedobj.c | 8 +- src/scheduler/src/scheduler.c | 14 +- src/scheduler/testsuite/meson.build | 15 + src/scheduler/testsuite/test.c | 2 +- src/scheduler/testsuite/test_core_events.c | 2 +- src/scheduler/testsuite/test_host_iter.c | 2 +- src/scheduler/testsuite/test_job_update.c | 2 +- .../testsuite/test_parjob_combinatorial.c | 2 +- .../testsuite/test_picker_config_fields.c | 2 +- .../testsuite/test_plugin_hook_dispatch.c | 293 ++++++++++++++++++ .../testsuite/test_resource_accounting.c | 2 +- .../testsuite/test_seqjob_allocator.c | 2 +- .../test_seqjob_rewrap_equivalence.c | 2 +- .../testsuite/test_snapshot_save_load.c | 2 +- 18 files changed, 403 insertions(+), 34 deletions(-) create mode 100644 src/scheduler/testsuite/test_plugin_hook_dispatch.c diff --git a/.kiro/specs/scheduler-sim-live-allocation/tasks.md b/.kiro/specs/scheduler-sim-live-allocation/tasks.md index 2f7b0e4e..6b97bbcd 100644 --- a/.kiro/specs/scheduler-sim-live-allocation/tasks.md +++ b/.kiro/specs/scheduler-sim-live-allocation/tasks.md @@ -9,7 +9,7 @@ - _Requirements: 5.2_ - _Boundary: Unsatisfiable-Cache Callback Seam_ -- [ ] 1.2 (P) Make plugin hook dispatch driver-agnostic and give every plugin an honest reference to shared core state +- [x] 1.2 (P) Make plugin hook dispatch driver-agnostic and give every plugin an honest reference to shared core state - Re-point the three plugin-hook dispatch functions (job election, prepare-allocation, commit-decision) to iterate the shared plugin registry directly instead of deriving it from a normal-mode-only driver struct, mirroring the session-begin/session-end dispatchers' own already-completed migration - Add a new, honestly-typed core-state reference field to the plugin instance struct, set by BOTH existing plugin-registration entry points (the normal-mode one and the simulation-mode one) — leave the plugin's existing, differently-typed driver-reference field completely untouched, since an unrelated legacy picker plugin still depends on its current meaning - TDD: extend the existing plugin-hook dispatch tests to run unchanged against a core-state fixture built either way (embedded in a normal-mode-shaped or simulation-mode-shaped container), and add a test confirming a plugin registered via the simulation-mode entry point has a non-NULL core-state reference (closing today's gap where it silently stays unset) @@ -110,3 +110,8 @@ - Observable: a written disposition (fixed, or explained as a pre-existing issue) exists for every mismatch found this way - _Depends: 5.1, 5.2, 5.3_ - _Requirements: 3.3, 3.5_ + +## Implementation Notes + +- **Task 1.1**: initial submission implemented before tests were written (TDD-mandate violation, caught in review round 1) — the real RED phase must be captured by reverting production files to HEAD via a saved patch + `git checkout --`, rebuilding to see the genuine failure, then reapplying. Retroactively-written tests plus a passing GREEN build alone do not satisfy this project's TDD discipline; the reviewer independently re-derives RED by reverting your changes, so skipping this is always caught. +- **Task 1.2**: retyping a widely-called function's signature (e.g. `plugin_job_election`, `plugin_prepare_allocation`, `plugin_commit_decision`) breaks ALL of its call sites as hard compile errors in one step (this toolchain treats `-Wincompatible-pointer-types` as an error, not a warning) — there is no way to migrate a signature and keep the build green without touching every call site in the same commit, even when those call sites live in files nominally owned by a later task. The correct scope is the *minimal* fix (wrap the existing argument, e.g. `sched` → `&(sched->core)`; touch nothing else in that function), not a rewrite of the caller. Also: design.md's Implementation Notes said `plugin_prepare_allocation()` has "one normal-mode call site" — it actually has two (`resource.c`'s `sched_try_alloc_on_host_()` and `schedobj.c`'s `sched_ctx_set_primary()`). Grep for ALL call sites before assuming a design doc's count is exhaustive. diff --git a/src/scheduler/include/plugin.h b/src/scheduler/include/plugin.h index ef4c78c2..a110d710 100644 --- a/src/scheduler/include/plugin.h +++ b/src/scheduler/include/plugin.h @@ -86,6 +86,19 @@ typedef struct { * job_elect_fn"). Stays NULL for plugin categories that are neither * (allocator/limit plugins never set job_elect_fn). */ xce_job_picker_iface_t* picker_iface; + /* scheduler-sim-live-allocation spec, task 1.2 ("Plugin Hook + * Dispatchers" / "Limit Plugin Core-Safe Matching"): an honestly-typed + * reference to the shared core state, set by BOTH plugin_register() + * (`plugin->core = &(sched->core);`) and plugin_register_core() + * (`plugin->core = core;`) -- non-NULL after either registration path. + * This is a NEW, separate field from `sched` above -- `sched` keeps its + * existing meaning and consumers (still xce_scheduler_t*-flavored, + * still set only by plugin_register(), still read by plugins/fcfs.c's + * legacy job_elect_fn path) untouched. A plugin hook that needs shared + * core state (e.g. plugins/limit.c's job-election/prepare-allocation + * matching) reads `core`, never `sched`, so it works identically + * whichever registration path registered it. */ + xce_sched_core_t* core; } xce_sched_plugin_t; extern int plugin_register(void*, void*); @@ -106,9 +119,19 @@ extern int plugin_job_state_change(void*, xce_sched_job_t*, uint32_t); * simulation-mode one. */ extern int plugin_begin_session(xce_sched_core_t*); extern int plugin_end_session(xce_sched_core_t*); -extern int plugin_commit_decision(void*, xce_decision_t*); -extern int plugin_prepare_allocation(void*, xce_sched_ctx_t*); -extern int plugin_job_election(void*, xce_sched_job_t*); +/* plugin_commit_decision()/plugin_prepare_allocation()/plugin_job_election() + * - relocated to xce_sched_core_t* (scheduler-sim-live-allocation spec, + * task 1.2, mirroring plugin_begin_session()/plugin_end_session()'s own + * identical bug-1681 relocation immediately above): each dispatch loop only + * ever touches core->plugins, never anything else on xce_scheduler_t* -- so, + * like plugin_register_core() below, these are callable from a driver with + * no xce_scheduler_t* (the simulation driver). Does NOT change + * plugin_hooks_t's own hook signatures (job_election_hook/ + * prepare_allocation_hook/commit_hook keep their existing `(void* arg, ...)` + * shape -- `arg` there is the plugin instance, not the driver/core struct). */ +extern int plugin_commit_decision(xce_sched_core_t*, xce_decision_t*); +extern int plugin_prepare_allocation(xce_sched_core_t*, xce_sched_ctx_t*); +extern int plugin_job_election(xce_sched_core_t*, xce_sched_job_t*); extern int plugin_job_rusage_update(void*, xce_sched_job_t*, xce_job_rusages_t*); extern int register_fcfs(void*); diff --git a/src/scheduler/src/plugin.c b/src/scheduler/src/plugin.c index 993dbf8b..756927cf 100644 --- a/src/scheduler/src/plugin.c +++ b/src/scheduler/src/plugin.c @@ -135,6 +135,11 @@ int plugin_register(void* arg1, void* arg2) } plugin->sched = sched; + /* scheduler-sim-live-allocation spec, task 1.2: the new, honestly-typed + * core-state reference -- set alongside `sched` above, never replacing + * it (see plugin.h's own doc comment on `core` for why both fields + * coexist). */ + plugin->core = &(sched->core); /* Picker plugin registration category: adapt an old-style, * job_elect_fn-only plugin to xce_job_picker_iface_t and warn once, at @@ -178,6 +183,14 @@ int plugin_register_core(xce_sched_core_t* core, xce_sched_plugin_t* plugin) return -EINVAL; } + /* scheduler-sim-live-allocation spec, task 1.2: closes the gap + * bug-1825 left -- until now this function never set anything on + * plugin->sched (no xce_scheduler_t* exists here to store), so a + * plugin registered via this entry point had no way to reach shared + * core state at all. plugin->sched is deliberately left NULL, exactly + * as before. */ + plugin->core = core; + if (plugin->init_fn != NULL) { rc = plugin->init_fn(plugin); if (rc < 0) { @@ -439,17 +452,16 @@ int plugin_end_session(xce_sched_core_t* core) return rc; } -int plugin_commit_decision(void* arg1, xce_decision_t* decision) +int plugin_commit_decision(xce_sched_core_t* core, xce_decision_t* decision) { - xce_scheduler_t* sched = (xce_scheduler_t*)arg1; - iterator_t it; - int rc; + iterator_t it; + int rc; - if (sched == NULL) { + if (core == NULL) { return -EINVAL; } - rc = iter_make(&it, sched->core.plugins); + rc = iter_make(&it, core->plugins); if (rc < 0) { return rc; } @@ -551,17 +563,16 @@ int plugin_job_rusage_update(void* arg, xce_sched_job_t* job, return rc; } -int plugin_job_election(void* arg, xce_sched_job_t* job) +int plugin_job_election(xce_sched_core_t* core, xce_sched_job_t* job) { - xce_scheduler_t* sched = (xce_scheduler_t*)arg; - iterator_t it; - int rc; + iterator_t it; + int rc; - if ((sched == NULL) || (job == NULL)) { + if ((core == NULL) || (job == NULL)) { return -EINVAL; } - rc = iter_make(&it, sched->core.plugins); + rc = iter_make(&it, core->plugins); if (rc < 0) { return rc; } @@ -588,14 +599,13 @@ int plugin_job_election(void* arg, xce_sched_job_t* job) return rc; } -int plugin_prepare_allocation(void* arg, xce_sched_ctx_t* ctx) +int plugin_prepare_allocation(xce_sched_core_t* core, xce_sched_ctx_t* ctx) { - xce_scheduler_t* sched = (xce_scheduler_t*)arg; iterator_t it; int rc; xce_sched_obj_t* obj = NULL; - if ((sched == NULL) || (ctx == NULL)) { + if ((core == NULL) || (ctx == NULL)) { return -EINVAL; } @@ -612,7 +622,7 @@ int plugin_prepare_allocation(void* arg, xce_sched_ctx_t* ctx) xvec_idval_init(&(ctx->alloc_req.host), 8); } - rc = iter_make(&it, sched->core.plugins); + rc = iter_make(&it, core->plugins); if (rc < 0) { return rc; } diff --git a/src/scheduler/src/resource.c b/src/scheduler/src/resource.c index 71dc4089..4ab43c23 100644 --- a/src/scheduler/src/resource.c +++ b/src/scheduler/src/resource.c @@ -525,7 +525,14 @@ int sched_try_alloc_on_host_(xce_sched_worker_t* w, xce_scheduler_t* sched, ctx->worker = w; - rc = plugin_prepare_allocation(sched, ctx); + /* scheduler-sim-live-allocation spec, task 1.2: plugin_prepare_ + * allocation() is now core-typed -- pass &(sched->core), not sched, + * the same minimal call-site update plugin_begin_session()/ + * plugin_end_session()'s own earlier migration made at their call + * sites. sched_try_alloc_on_host_() itself keeps its existing + * xce_scheduler_t* sched parameter unchanged (that retyping is a + * separate task's scope, "Core-Safe Allocation Chain"). */ + rc = plugin_prepare_allocation(&(sched->core), ctx); if (rc < 0) { ctx->worker = NULL; diff --git a/src/scheduler/src/schedobj.c b/src/scheduler/src/schedobj.c index 3477974e..dffe2188 100644 --- a/src/scheduler/src/schedobj.c +++ b/src/scheduler/src/schedobj.c @@ -503,7 +503,13 @@ void sched_ctx_set_primary(xce_sched_ctx_t* ctx, xce_primary_resreq_t* primary) { ctx->primary = primary; - plugin_prepare_allocation((xce_scheduler_t*)ctx->sched, ctx); + /* scheduler-sim-live-allocation spec, task 1.2: plugin_prepare_ + * allocation() is now core-typed -- ctx->sched itself keeps its + * existing void* meaning and cast (xce_scheduler_t* today; retyping + * ctx->sched to carry core state directly is a separate task's scope, + * "Core-Safe Allocation Chain", task 2.1); this call site only adds + * the trailing &(...->core) needed for the now-core-typed callee. */ + plugin_prepare_allocation(&(((xce_scheduler_t*)ctx->sched)->core), ctx); return; } diff --git a/src/scheduler/src/scheduler.c b/src/scheduler/src/scheduler.c index 39d7b9b2..0d33ebaf 100644 --- a/src/scheduler/src/scheduler.c +++ b/src/scheduler/src/scheduler.c @@ -478,7 +478,13 @@ static void* sched_allocator(void* arg) continue; } - plugin_job_election(sched, job); + /* scheduler-sim-live-allocation spec, task 1.2: plugin_job_election() + * is now core-typed (mirrors plugin_begin_session(&(sched->core)) + * a few lines up in this same file) -- pass &(sched->core), not + * sched, exactly the same minimal call-site update that migration + * already made at this file's own plugin_begin_session()/ + * plugin_end_session() call sites. */ + plugin_job_election(&(sched->core), job); /* calculate decision for jobid */ alloc_rc = sched_make_allocation(sched, job->schedobj); @@ -1609,7 +1615,11 @@ int sched_commit_decision(xce_scheduler_t* sched, xce_decision_t* decision) int rc = 0; xce_sched_job_t* job = NULL; - rc = plugin_commit_decision(sched, decision); + /* scheduler-sim-live-allocation spec, task 1.2: plugin_commit_decision() + * is now core-typed -- pass &(sched->core), same minimal call-site + * update as plugin_job_election()'s own call site (sched_allocator(), + * this file). */ + rc = plugin_commit_decision(&(sched->core), decision); if (rc < 0) { return rc; } diff --git a/src/scheduler/testsuite/meson.build b/src/scheduler/testsuite/meson.build index c21bcbf6..ac38870a 100644 --- a/src/scheduler/testsuite/meson.build +++ b/src/scheduler/testsuite/meson.build @@ -91,6 +91,19 @@ sched_picker_registration_test = executable('sched_picker_registration_test', dependencies: [scheduler_test_deps], install: false) +# White-box test for the driver-agnostic plugin-hook dispatch functions +# (plugin_job_election/plugin_prepare_allocation/plugin_commit_decision) and +# the new xce_sched_plugin_t.core field (scheduler-sim-live-allocation spec, +# task 1.2, "Plugin Hook Dispatchers"). It #includes ../src/plugin.c +# directly (same white-box pattern as sched_picker_registration_test) so it +# exercises the real dispatch loops and the real plugin_register()/ +# plugin_register_core() entry points, not a re-implementation of either. +sched_plugin_hook_dispatch_test = executable('sched_plugin_hook_dispatch_test', + ['test_plugin_hook_dispatch.c'], + include_directories:[incdir], + dependencies: [scheduler_test_deps], + install: false) + # White-box test for the picker configuration fields added to # xce_scheduler_t/xce_sched_queue_t/xce_sched_project_t (scheduler-core # spec, task 1.3). It #includes ../src/schedobj.c directly (same white-box @@ -1734,6 +1747,8 @@ test('scheduler picker iface test', sched_picker_iface_test, suite: 'scheduler', env: ['ASAN_OPTIONS=detect_leaks=0']) test('scheduler picker registration test', sched_picker_registration_test, suite: 'scheduler', env: ['ASAN_OPTIONS=detect_leaks=0']) +test('scheduler plugin hook dispatch test', sched_plugin_hook_dispatch_test, suite: 'scheduler', + env: ['ASAN_OPTIONS=detect_leaks=0']) test('scheduler picker config fields test', sched_picker_config_fields_test, suite: 'scheduler', env: ['ASAN_OPTIONS=detect_leaks=0']) test('scheduler picker fcfs test', sched_picker_fcfs_test, suite: 'scheduler', diff --git a/src/scheduler/testsuite/test.c b/src/scheduler/testsuite/test.c index 9e414bcd..5f0bc6c7 100644 --- a/src/scheduler/testsuite/test.c +++ b/src/scheduler/testsuite/test.c @@ -22,7 +22,7 @@ xce_sched_worker_t* sched_get_worker(xce_sched_core_t* s, char* n) (void)n; return NULL; } -int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; } +int plugin_prepare_allocation(xce_sched_core_t* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; } int sched_ctx_get_uncomit_host(xvec_t** v, xce_sched_ctx_t* c, int i) { (void)v; (void)c; (void)i; return 0; } int plugin_queue_update(void* s, xce_sched_queue_t* q) { (void)s; (void)q; return 0; } xce_sched_queue_t* sched_get_queue(xce_sched_core_t* s, char* n) diff --git a/src/scheduler/testsuite/test_core_events.c b/src/scheduler/testsuite/test_core_events.c index 54dfcecd..fde03b4a 100644 --- a/src/scheduler/testsuite/test_core_events.c +++ b/src/scheduler/testsuite/test_core_events.c @@ -81,7 +81,7 @@ void kill_sched_resval(xce_val_t* val) { (void)val; } * two stubs test_resource_accounting.c already establishes needing for the * identical reason. Never actually invoked: this file's tests never call * sched_try_alloc_on_hosts()/sched_try_alloc_on_host_(). */ -int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c) +int plugin_prepare_allocation(xce_sched_core_t* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; } diff --git a/src/scheduler/testsuite/test_host_iter.c b/src/scheduler/testsuite/test_host_iter.c index f59ed3b8..35664dd0 100644 --- a/src/scheduler/testsuite/test_host_iter.c +++ b/src/scheduler/testsuite/test_host_iter.c @@ -69,7 +69,7 @@ * entirely) nor sched_try_alloc_on_hosts()/sched_update_existing_*(). */ void plugin_unregister(void* arg) { (void)arg; } -int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c) +int plugin_prepare_allocation(xce_sched_core_t* s, xce_sched_ctx_t* c) { (void)s; (void)c; diff --git a/src/scheduler/testsuite/test_job_update.c b/src/scheduler/testsuite/test_job_update.c index 233cdf1e..49c0a635 100644 --- a/src/scheduler/testsuite/test_job_update.c +++ b/src/scheduler/testsuite/test_job_update.c @@ -77,7 +77,7 @@ int sched_add_project(xce_sched_core_t* s, xce_sched_project_t* p) return 0; } void sched_worker_release_host_ctx(xce_sched_worker_t* w) { (void)w; } -int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; } +int plugin_prepare_allocation(xce_sched_core_t* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; } /* sched_get_queue: a tiny by-name lookup over a fixed test queue set. */ static xce_sched_queue_t* g_test_queues[8]; diff --git a/src/scheduler/testsuite/test_parjob_combinatorial.c b/src/scheduler/testsuite/test_parjob_combinatorial.c index e4717914..af7e92a1 100644 --- a/src/scheduler/testsuite/test_parjob_combinatorial.c +++ b/src/scheduler/testsuite/test_parjob_combinatorial.c @@ -130,7 +130,7 @@ static int g_job_level_prepare_calls = 0; * every registered plugin -- exactly one (parjob's own) in this file's * fixtures. Mirrors plugin.c:591's own reset-before-fan-out step, same * rationale as test_seqjob_allocator.c's identical stub. */ -int plugin_prepare_allocation(void* sched, xce_sched_ctx_t* ctx) +int plugin_prepare_allocation(xce_sched_core_t* sched, xce_sched_ctx_t* ctx) { xce_sched_plugin_t fake_plugin; diff --git a/src/scheduler/testsuite/test_picker_config_fields.c b/src/scheduler/testsuite/test_picker_config_fields.c index 85d33d2f..ec0fb2e6 100644 --- a/src/scheduler/testsuite/test_picker_config_fields.c +++ b/src/scheduler/testsuite/test_picker_config_fields.c @@ -81,7 +81,7 @@ int sched_reconcile_job_resource_usage(xce_sched_core_t* c, (void)c; (void)a; (void)r; return 0; } void sched_worker_release_host_ctx(xce_sched_worker_t* w) { (void)w; } -int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; } +int plugin_prepare_allocation(xce_sched_core_t* s, xce_sched_ctx_t* c) { (void)s; (void)c; return 0; } void plugin_unregister(void* arg) { (void)arg; } void setUp(void) { return; } diff --git a/src/scheduler/testsuite/test_plugin_hook_dispatch.c b/src/scheduler/testsuite/test_plugin_hook_dispatch.c new file mode 100644 index 00000000..993f1de4 --- /dev/null +++ b/src/scheduler/testsuite/test_plugin_hook_dispatch.c @@ -0,0 +1,293 @@ +/* + * test_plugin_hook_dispatch.c - white-box tests for the driver-agnostic + * plugin-hook dispatch functions (scheduler-sim-live-allocation spec, task + * 1.2, "Plugin Hook Dispatchers" component). + * + * RED (before this task): plugin_job_election()/plugin_prepare_allocation()/ + * plugin_commit_decision() took a `void*` derived from a normal-mode-only + * xce_scheduler_t*, and xce_sched_plugin_t had no `core` field at all -- this + * file fails to compile (`xce_sched_plugin_t` has no member named `core`). + * GREEN: all three dispatchers take `xce_sched_core_t* core` directly and + * iterate `core->plugins` (mirroring plugin_begin_session()/ + * plugin_end_session()'s own already-completed migration exactly); + * xce_sched_plugin_t.core is set by BOTH plugin_register() (normal mode) and + * plugin_register_core() (simulation mode). + * + * #include's the real plugin.c (white-box, same convention as + * test_plugin_picker_registration.c) so this test exercises the real + * dispatch loops and the real registration entry points, not a + * re-implementation of either. + * + * Fixture shape: this file never needs a real xce_scheduler_t/ + * xce_sched_sim_driver_t beyond their embedded `core` field (both structs + * embed `xce_sched_core_t core` as their first member -- scheduler.h, + * sim.h), so it builds a bare `core->plugins` kvmgr manually, same minimal + * pattern test_plugin_picker_registration.c's own fixture_init() already + * establishes, rather than pulling in core.c's full xce_sched_core_init() + * and its wider dependency graph. + */ +#include +#include +#include +#include +#include + +#include + +#include "kvmgr.h" +#include "scheduler.h" +#include "sim.h" + +/* plugin_prepare_allocation()'s post-hook-loop limit-request fan-out calls + * sched_fill_limit_requests() (scheduler.c, not part of this white-box TU) + * -- link-satisfying stub only, same rationale/pattern as + * test_plugin_picker_registration.c's own copy: this file's mock plugins + * never populate real limit data, so a no-op body is sufficient. */ +int sched_fill_limit_requests(xce_sched_alloc_req_t* alloc_req, + xlist_t* limits, xlist_t* rusages, int nhosts, + int nslots) +{ + (void)alloc_req; + (void)limits; + (void)rusages; + (void)nhosts; + (void)nslots; + return 0; +} + +#include "../src/plugin.c" + +void setUp(void) { return; } +void tearDown(void) { return; } + +/* ---------------------------------------------------------------------- */ +/* Fixture: a bare core->plugins kvmgr -- same minimal manual setup */ +/* test_plugin_picker_registration.c's own fixture_init() uses. Callable */ +/* against the `core` field of EITHER a normal-mode-shaped xce_scheduler_t */ +/* or a simulation-mode-shaped xce_sched_sim_driver_t -- both embed */ +/* xce_sched_core_t as their first member, so `&sched.core`/`&sim.core` are */ +/* the two "containers built either way" this file's tests exercise. */ +/* ---------------------------------------------------------------------- */ + +static void core_fixture_init(xce_sched_core_t* core) +{ + kvmgr_params_t params = { .key_type = KVMGR_KEY_STRING, + .nslots = 16, + .delta = 8, + .datakiller = plugin_unregister }; + + memset(core, 0, sizeof(*core)); + core->plugins = makeInstance(&xkvmgr_type, ¶ms); + TEST_ASSERT_NOT_NULL(core->plugins); +} + +/* ---------------------------------------------------------------------- */ +/* Mock plugins: order-recording spies for each of the three hooks under */ +/* test. `arg` is the plugin instance itself (the dispatch loop's own */ +/* convention, unchanged by this task -- see plugin.h's hook typedefs). */ +/* ---------------------------------------------------------------------- */ + +#define MAX_CALLS 8 + +static char g_call_order[MAX_CALLS][32]; +static int g_call_count; + +static void reset_calls(void) +{ + g_call_count = 0; + memset(g_call_order, 0, sizeof(g_call_order)); +} + +static void record_call(const char* name) +{ + if (g_call_count < MAX_CALLS) { + strncpy(g_call_order[g_call_count], name, + sizeof(g_call_order[0]) - 1); + } + g_call_count++; +} + +static int mock_job_election_hook(void* arg, xce_sched_job_t* job) +{ + xce_sched_plugin_t* plugin = (xce_sched_plugin_t*)arg; + + (void)job; + record_call(plugin->name); + return 0; +} + +static int mock_prepare_allocation_hook(void* arg, xce_sched_ctx_t* ctx) +{ + xce_sched_plugin_t* plugin = (xce_sched_plugin_t*)arg; + + (void)ctx; + record_call(plugin->name); + return 0; +} + +static int mock_commit_hook(void* arg, xce_decision_t* decision) +{ + xce_sched_plugin_t* plugin = (xce_sched_plugin_t*)arg; + + (void)decision; + record_call(plugin->name); + return 0; +} + +static void make_mock_plugin(xce_sched_plugin_t* plugin, char* name) +{ + memset(plugin, 0, sizeof(*plugin)); + plugin->name = name; + plugin->hooks.job_election_hook + = (plugin_job_election_hook_t)mock_job_election_hook; + plugin->hooks.prepare_allocation_hook + = (plugin_prepare_allocation_hook_t)mock_prepare_allocation_hook; + plugin->hooks.commit_hook + = (plugin_commit_decision_hook_t)mock_commit_hook; +} + +/* ---------------------------------------------------------------------- */ +/* The dispatch contract: exactly one call per registered plugin, per */ +/* dispatch function, in registration order -- unchanged by this task's */ +/* signature migration (Observable in tasks.md task 1.2). Run once per */ +/* container shape by the two thin wrappers below. */ +/* ---------------------------------------------------------------------- */ + +static void run_dispatch_fanout_case(xce_sched_core_t* core) +{ + xce_sched_plugin_t plugin_a; + xce_sched_plugin_t plugin_b; + xce_sched_job_t job; + xce_sched_obj_t obj; + xce_sched_ctx_t ctx; + xce_decision_t decision; + int rc; + + core_fixture_init(core); + + make_mock_plugin(&plugin_a, "plugin_a"); + make_mock_plugin(&plugin_b, "plugin_b"); + + rc = plugin_register_core(core, &plugin_a); + TEST_ASSERT_TRUE(rc >= 0); + rc = plugin_register_core(core, &plugin_b); + TEST_ASSERT_TRUE(rc >= 0); + + /* -- plugin_job_election(): job_election_hook fan-out -- */ + memset(&job, 0, sizeof(job)); + reset_calls(); + rc = plugin_job_election(core, &job); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_INT(2, g_call_count); + TEST_ASSERT_EQUAL_STRING("plugin_a", g_call_order[0]); + TEST_ASSERT_EQUAL_STRING("plugin_b", g_call_order[1]); + + /* -- plugin_prepare_allocation(): prepare_allocation_hook fan-out -- + * obj->tag is deliberately XCE_RESREQ_COMPOUND (not PRIMARY) so the + * post-hook-loop limit-request step's PRIMARY-only rusages branch + * (obj->u.primary->resreq->rusages) is never dereferenced -- this test + * targets the hook fan-out itself, not the limit-request populate step + * (covered elsewhere, e.g. test_seqjob_allocator.c). */ + memset(&obj, 0, sizeof(obj)); + obj.tag = XCE_RESREQ_COMPOUND; + memset(&ctx, 0, sizeof(ctx)); + ctx.schedobj = &obj; + xvec_idval_init(&(ctx.alloc_req.shared), 8); + xvec_idval_init(&(ctx.alloc_req.host), 8); + reset_calls(); + rc = plugin_prepare_allocation(core, &ctx); + TEST_ASSERT_TRUE(rc >= 0); + TEST_ASSERT_EQUAL_INT(2, g_call_count); + TEST_ASSERT_EQUAL_STRING("plugin_a", g_call_order[0]); + TEST_ASSERT_EQUAL_STRING("plugin_b", g_call_order[1]); + xvec_idval_fini(&(ctx.alloc_req.shared)); + xvec_idval_fini(&(ctx.alloc_req.host)); + + /* -- plugin_commit_decision(): commit_hook fan-out -- */ + memset(&decision, 0, sizeof(decision)); + reset_calls(); + rc = plugin_commit_decision(core, &decision); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_INT(2, g_call_count); + TEST_ASSERT_EQUAL_STRING("plugin_a", g_call_order[0]); + TEST_ASSERT_EQUAL_STRING("plugin_b", g_call_order[1]); +} + +/* Normal-mode-shaped container: xce_scheduler_t (`sched.core`). */ +static void test_dispatch_fanout_normal_mode_shaped_container(void) +{ + xce_scheduler_t sched; + + memset(&sched, 0, sizeof(sched)); + run_dispatch_fanout_case(&(sched.core)); +} + +/* Simulation-mode-shaped container: xce_sched_sim_driver_t (`sim.core`). */ +static void test_dispatch_fanout_simulation_mode_shaped_container(void) +{ + xce_sched_sim_driver_t sim; + + memset(&sim, 0, sizeof(sim)); + run_dispatch_fanout_case(&(sim.core)); +} + +/* ---------------------------------------------------------------------- */ +/* xce_sched_plugin_t.core -- the new, honestly-typed core-state reference */ +/* field. Set by BOTH registration entry points; the existing, differently- */ +/* typed `sched` field must remain genuinely untouched by */ +/* plugin_register_core() and keep meaning exactly what it means today for */ +/* plugin_register() (still xce_scheduler_t*, still read by fcfs.c's */ +/* legacy job_elect_fn path). */ +/* ---------------------------------------------------------------------- */ + +/* Closes today's gap (bug-1825-adjacent): before this task, + * plugin_register_core() never set anything on plugin->core (the field did + * not exist), so a plugin registered via the simulation-mode entry point + * silently had no way to reach shared core state. */ +static void test_plugin_register_core_sets_honest_core_reference(void) +{ + xce_sched_core_t core; + xce_sched_plugin_t plugin; + int rc; + + core_fixture_init(&core); + make_mock_plugin(&plugin, "core_field_probe_sim"); + + rc = plugin_register_core(&core, &plugin); + TEST_ASSERT_TRUE(rc >= 0); + TEST_ASSERT_NOT_NULL(plugin.core); + TEST_ASSERT_EQUAL_PTR(&core, plugin.core); + /* plugin_register_core() has no xce_scheduler_t* to store -- plugin-> + * sched stays NULL, unchanged from today (plugin.h's own doc comment). */ + TEST_ASSERT_NULL(plugin.sched); +} + +static void test_plugin_register_sets_honest_core_reference(void) +{ + xce_scheduler_t sched; + xce_sched_plugin_t plugin; + int rc; + + memset(&sched, 0, sizeof(sched)); + core_fixture_init(&(sched.core)); + make_mock_plugin(&plugin, "core_field_probe_normal"); + + rc = plugin_register(&sched, &plugin); + TEST_ASSERT_TRUE(rc >= 0); + TEST_ASSERT_NOT_NULL(plugin.core); + TEST_ASSERT_EQUAL_PTR(&(sched.core), plugin.core); + /* The existing, differently-typed `sched` field must remain genuinely + * untouched -- still xce_scheduler_t*, still set, still what fcfs.c's + * legacy job_elect_fn path reads. */ + TEST_ASSERT_EQUAL_PTR(&sched, plugin.sched); +} + +int main(void) +{ + UNITY_BEGIN(); + RUN_TEST(test_dispatch_fanout_normal_mode_shaped_container); + RUN_TEST(test_dispatch_fanout_simulation_mode_shaped_container); + RUN_TEST(test_plugin_register_core_sets_honest_core_reference); + RUN_TEST(test_plugin_register_sets_honest_core_reference); + return UNITY_END(); +} diff --git a/src/scheduler/testsuite/test_resource_accounting.c b/src/scheduler/testsuite/test_resource_accounting.c index 62167917..98c95083 100644 --- a/src/scheduler/testsuite/test_resource_accounting.c +++ b/src/scheduler/testsuite/test_resource_accounting.c @@ -102,7 +102,7 @@ void plugin_unregister(void* arg) { (void)arg; } * sched_try_alloc_on_host_(), only sched_charge_account()/ * sched_uncharge_account()/sched_commit_job_alloc(). Same stub test.c's * own #include "../src/resource.c" already establishes needing. */ -int plugin_prepare_allocation(void* s, xce_sched_ctx_t* c) +int plugin_prepare_allocation(xce_sched_core_t* s, xce_sched_ctx_t* c) { (void)s; (void)c; diff --git a/src/scheduler/testsuite/test_seqjob_allocator.c b/src/scheduler/testsuite/test_seqjob_allocator.c index 09d7d8d0..ab332aac 100644 --- a/src/scheduler/testsuite/test_seqjob_allocator.c +++ b/src/scheduler/testsuite/test_seqjob_allocator.c @@ -81,7 +81,7 @@ void plugin_unregister(void* arg) { (void)arg; } * the real production call sequence's two prepare_allocation() calls per * host try (once at ctx->worker==NULL via sched_ctx_set_primary(), again * per host at ctx->worker!=NULL), since xvec_idval_store() never dedups. */ -int plugin_prepare_allocation(void* sched, xce_sched_ctx_t* ctx) +int plugin_prepare_allocation(xce_sched_core_t* sched, xce_sched_ctx_t* ctx) { xce_sched_plugin_t fake_plugin; diff --git a/src/scheduler/testsuite/test_seqjob_rewrap_equivalence.c b/src/scheduler/testsuite/test_seqjob_rewrap_equivalence.c index 5af7e433..93d9a4f2 100644 --- a/src/scheduler/testsuite/test_seqjob_rewrap_equivalence.c +++ b/src/scheduler/testsuite/test_seqjob_rewrap_equivalence.c @@ -111,7 +111,7 @@ void plugin_unregister(void* arg) { (void)arg; } * every registered plugin -- exactly one (seqjob's own) in this file's * fixtures. See test_seqjob_allocator.c's identical stub for the full * xvec_idval_fini()/init() reset-before-fan-out rationale. */ -int plugin_prepare_allocation(void* sched, xce_sched_ctx_t* ctx) +int plugin_prepare_allocation(xce_sched_core_t* sched, xce_sched_ctx_t* ctx) { xce_sched_plugin_t fake_plugin; diff --git a/src/scheduler/testsuite/test_snapshot_save_load.c b/src/scheduler/testsuite/test_snapshot_save_load.c index 706a2b63..1f7c97dc 100644 --- a/src/scheduler/testsuite/test_snapshot_save_load.c +++ b/src/scheduler/testsuite/test_snapshot_save_load.c @@ -130,7 +130,7 @@ void plugin_unregister(void* arg) { (void)arg; } * test_sim_contention_parity.c is not this pair's precedent, since it only * needs sched_ctx_get_uncomit_host itself (it gets plugin_prepare_allocation * for real via its own #include "../src/plugin.c"). */ -int plugin_prepare_allocation(void* arg, xce_sched_ctx_t* ctx) +int plugin_prepare_allocation(xce_sched_core_t* arg, xce_sched_ctx_t* ctx) { (void)arg; (void)ctx;