diff --git a/src/scheduler/meson.build b/src/scheduler/meson.build index 8e5b080c..51658d06 100644 --- a/src/scheduler/meson.build +++ b/src/scheduler/meson.build @@ -49,6 +49,7 @@ scheduler_srcs = [ 'src/plugins/seqjob_allocator.c', 'src/plugins/parjob_allocator.c', 'src/plugins/limit.c', + 'src/plugins/limit_config.c', ] incdir = include_directories('src', diff --git a/src/scheduler/src/plugins/limit.c b/src/scheduler/src/plugins/limit.c index 41ad753a..2c3ff8b4 100644 --- a/src/scheduler/src/plugins/limit.c +++ b/src/scheduler/src/plugins/limit.c @@ -13,34 +13,16 @@ #include "xlist.h" #include "xstring.h" -typedef struct { - var limits; - var instances; -} limit_plugin_data_t; +#include "limit_internal.h" static int limit_priv_data_init(limit_plugin_data_t* data) { - kvmgr_params_t limits_params = { - .key_type = KVMGR_KEY_STRING, - .nslots = 1021, - .delta = 64, - .datakiller = (destroy_fn_t)kill_limit, - }; - /* bug-1884: holds xce_vres_t usage instances, not limits -- kill_limit - * here freed a pointer inside a resource object on destroy. */ - kvmgr_params_t instances_params = { - .key_type = KVMGR_KEY_STRING, - .nslots = 1021, - .delta = 64, - .datakiller = (destroy_fn_t)kill_vres, - }; - - data->limits = makeInstance(&xkvmgr_type, &limits_params); + data->limits = limit_make_limits_table(); if (data->limits == NULL) { return -errno; } - data->instances = makeInstance(&xkvmgr_type, &instances_params); + data->instances = limit_make_instances_table(); if (data->instances == NULL) { return -errno; } @@ -49,83 +31,19 @@ static int limit_priv_data_init(limit_plugin_data_t* data) } /* - * limit_seed_config_impl - shared seed-loop body behind both + * limit_seed_config_impl - shared seed body behind both * limit_update_config() (normal-mode config_hook, below) and * limit_seed_config() (bug-1825 remediation round 2, near - * limit_new_plugin() below): builds and registers one xce_limit_t per - * config->limits[] entry against `core`'s real group managers. - * Parameterized on an honestly-typed xce_sched_core_t* -- neither caller - * derives `core` from a plugin->sched cast inside this function; each - * caller resolves its own `core` argument in whatever way is safe for its - * own context (see each caller's doc comment). + * limit_new_plugin() below). Parameterized on an honestly-typed + * xce_sched_core_t* -- neither caller derives `core` from a plugin->sched + * cast. bug-1967: a push replaces the configured limit set and carries the + * usage instances across it; see limit_config.c. */ static int limit_seed_config_impl(xce_sched_plugin_t* plugin, xce_sched_core_t* core, xce_conf4sched_t* config) { - limit_plugin_data_t* privdata = NULL; - int i; - xce_limit_config_t** limits = config->limits; - int nlimits = config->nlimits; - - privdata = (limit_plugin_data_t*)plugin->data; - - for (i = 0; i < nlimits; i++) { - int rc; - xce_limit_t* limit; - - /* bug-1933: build_limit() takes the host groups before the - * project groups */ - rc = build_limit(&limit, (core->ugmgr), (core->qgmgr), - (core->hgmgr), (core->pgmgr), limits[i]); - if (rc < 0) { - return rc; - } - - lock(privdata->limits, RTTI_LOCK_WRITE); - rc = key_add(privdata->limits, (void*)limit->name, (void*)limit); - unlock(privdata->limits); - - if (rc < 0) { - /* bug-1685: do NOT kill_limit(limit) here -- privdata->limits - * is a kvmgr created with .datakiller = kill_limit - * (limit_priv_data_init() above). key_add() on it dispatches - * to xkvmgr_add() (../../lib/kvmgr.c), which -- when the - * kvtab-level key_add() fails after the value was already - * vstore()'d (e.g. -EEXIST on a duplicate limit name, this - * function's own common failure mode) -- itself rolls back via - * vclear(kvm->data, idx). vclear() dispatches to - * xzarray_clear() (../../lib/zarray.c), which fires the - * configured killer (kill_limit) on `limit` exactly once as - * part of that rollback. Calling kill_limit(limit) again here - * would free the same already-freed pointer a second time -- - * a double free that glibc's malloc_printerr aborts on - * (bug-1685's captured live crash: malloc_printerr <- - * kill_limit <- limit_update_config, SIGABRT on a real 3-node - * HA deployment). See job.c's restore_job_array() doc comment - * for the same asymmetry on a DIFFERENT kvmgr (c->jobmgr, - * .datakiller = NULL there) -- whether a caller must free a - * rejected key_add() value depends entirely on whether THAT - * specific kvmgr instance was configured with a non-NULL - * datakiller. privdata->limits has one, so this caller must - * NOT free it itself. - * - * Accepted residual gap: if vstore() itself fails (before - * ever reaching the kvtab key_add step -- e.g. zarray growth - * OOM), xkvmgr_add() returns negative WITHOUT having stored or - * freed `limit` at all, and this function has no way to tell - * that sub-case apart from the common EEXIST one from rc - * alone -- so removing this call trades a guaranteed crash on - * the common case for a leak in that vanishingly rare, - * OOM-only sub-case. Leak is strictly better than crash; - * closing this gap cleanly would need a new kvmgr API, which - * is out of scope here (mirrors this codebase's own accepted - * trade-off for restore_job_array()'s analogous gap). */ - return rc; - } - } - - return 0; + return limit_replace_config(plugin, core, config); } /* bug-1964: reads plugin->core, set by both plugin_register() and diff --git a/src/scheduler/src/plugins/limit_config.c b/src/scheduler/src/plugins/limit_config.c new file mode 100644 index 00000000..ff992b76 --- /dev/null +++ b/src/scheduler/src/plugins/limit_config.c @@ -0,0 +1,278 @@ +/* + * limit_config.c - the limit plugin's config seed, shared by + * limit_update_config() (the config_hook: a live config push and, since + * bug-1964, a simulator CONFIG_PUSH) and limit_seed_config() (simulator + * bootstrap). Split out of limit.c to keep that file under the 800-line cap. + * + * bug-1967: a push REPLACES the configured limit set. It used to key_add() + * each limit into the existing table and stop at the first failure, so on + * any second push the first already-present name failed with -EEXIST: + * changed limits kept their old definition and new ones were never added. + * + * Usage instances (privdata->instances, the xce_vres_t records + * match_job_global_limit_()/match_job_host_limit_() intern per vres name) + * point at a configured xce_limit_t, so they are carried across a push: + * + * - A limit kept under the same name whose instance identity is unchanged + * (same per-user/queue/project/host scoping and same resource/percent, + * i.e. the same vres name, vres scope and underlying resource) keeps its + * instances, rebound to the new definition. Its value and group + * membership may change: update_limit_res_value() reads the new value on + * the next match, which every job's next election performs because the + * push advances core->configver. + * - An instance of a removed limit, or of a limit whose scoping or + * resource changed, is dropped. The next match instantiates the right + * one. + * + * Usage itself is not held by an instance: it lives in the resource manager + * under the instance's virtual resource (vres->vres) and in the + * sharedInst/resvals vectors at its index, which a push does not touch. A + * re-created instance finds the same virtual resource by name + * (instantiate_limit()), so usage counts survive either way. Nothing here is + * part of the scheduler snapshot: a snapshot carries the config, and loading + * it seeds the limits from that config again. + * + * Jobs' schedobj glimits/hlimits lists may still hold pointers to dropped or + * replaced instance records, but none is read before it is rebuilt: both + * drivers apply a push between rounds only (live: sched_apply_pending_config(); + * core: xce_core_push_events() refuses during a session), the push advances + * core->configver, and limit_job_election() resets a job's lists before any + * reader (allocation, trace capture) sees them. + * + * A push is all-or-nothing: if any limit fails to build or register (e.g. a + * duplicate name), the new set is discarded and the previous one, with its + * instances, stays in force. + */ +#include +#include +#include +#include + +#include "kvmgr.h" +#include "plugin.h" +#include "sched/types.h" +#include "scheduler.h" +#include "utils.h" +#include "xcelimit.h" + +#include "limit_internal.h" + +var limit_make_limits_table(void) +{ + kvmgr_params_t params = { + .key_type = KVMGR_KEY_STRING, + .nslots = 1021, + .delta = 64, + .datakiller = (destroy_fn_t)kill_limit, + }; + + return makeInstance(&xkvmgr_type, ¶ms); +} + +/* bug-1884: holds xce_vres_t usage instances, not limits -- kill_limit + * here freed a pointer inside a resource object on destroy. */ +var limit_make_instances_table(void) +{ + kvmgr_params_t params = { + .key_type = KVMGR_KEY_STRING, + .nslots = 1021, + .delta = 64, + .datakiller = (destroy_fn_t)kill_vres, + }; + + return makeInstance(&xkvmgr_type, ¶ms); +} + +static int consumer_per_(xce_limit_t* limit, int type) +{ + xce_consumer_t* c = limit->consumers[type]; + + return (c != NULL) ? c->per_consumer : 0; +} + +static int same_str_(const char* a, const char* b) +{ + if ((a == NULL) || (b == NULL)) { + return a == b; + } + + return strcmp(a, b) == 0; +} + +/* Whether an instance built for `old` is still the right instance for + * `next`: the per-consumer flags decide its vres name (build_vres_name()) + * and, with the resource, whether it is host-scoped (is_host_based_limit()); + * the resource and percent decide its res/percent_res (instantiate_limit()). + * The value and group membership are read afresh on every match. */ +static int instance_survives_(xce_limit_t* old, xce_limit_t* next) +{ + int i; + + for (i = 0; i < CONSUMER_MAX; i++) { + if (consumer_per_(old, i) != consumer_per_(next, i)) { + return 0; + } + } + + return (old->val.percent == next->val.percent) + && same_str_(old->val.resname, next->val.resname); +} + +/* builds every config->limits[] entry into a new table (*plimits) */ +static int build_limit_set_(xce_sched_core_t* core, xce_conf4sched_t* config, + var* plimits) +{ + var limits = limit_make_limits_table(); + int i; + + *plimits = NULL; + if (limits == NULL) { + return (errno > 0) ? -errno : -ENOMEM; + } + + for (i = 0; i < config->nlimits; i++) { + int rc; + xce_limit_t* limit = NULL; + + /* bug-1933: build_limit() takes the host groups before the + * project groups */ + rc = build_limit(&limit, (core->ugmgr), (core->qgmgr), (core->hgmgr), + (core->pgmgr), config->limits[i]); + if (rc < 0) { + kill_limit(limit); /* not in any table yet */ + destroy(limits); + return rc; + } + + /* bug-1685: on failure (e.g. -EEXIST, a duplicate name) the kvmgr + * has already freed `limit` through its kill_limit datakiller + * (xkvmgr_add()'s vclear() rollback) -- do NOT free it again. + * Accepted residual gap: if vstore() itself fails (zarray growth + * OOM) the kvmgr returns negative without storing or freeing it, + * which rc alone cannot tell apart, so that rare case leaks. */ + rc = key_add(limits, (void*)limit->name, (void*)limit); + if (rc < 0) { + destroy(limits); + return rc; + } + } + + *plimits = limits; + + return 0; +} + +/* carries the surviving instances of `instances` into a new table + * (*pnext), rebound to their limit in `limits`; see this file's header */ +static int rebind_instances_(var instances, var limits, var* pnext) +{ + var next = limit_make_instances_table(); + iterator_t it; + int rc; + + *pnext = NULL; + if (next == NULL) { + return (errno > 0) ? -errno : -ENOMEM; + } + + rc = iter_make(&it, instances); + if (rc < 0) { + destroy(next); + return rc; + } + + for (;;) { + xce_vres_t* vres = NULL; + xce_limit_t* limit = NULL; + xce_vres_t* copy = NULL; + + rc = iter_next(&it, (void**)&vres); + if ((rc < 0) || (rc == RTTI_ITER_NO_DATA)) { + break; + } + + if ((vres == NULL) || (vres->limit == NULL) || (vres->vres == NULL)) { + continue; + } + + key_find(limits, (void*)vres->limit->name, (void**)&limit); + if ((limit == NULL) || !instance_survives_(vres->limit, limit)) { + continue; /* dropped: freed with the old table */ + } + + copy = (xce_vres_t*)malloc(sizeof(*copy)); + if (copy == NULL) { + rc = -ENOMEM; + break; + } + + *copy = *vres; + copy->limit = limit; + + /* on failure the kvmgr's kill_vres has already freed `copy` */ + rc = key_add(next, (void*)copy->vres->name, (void*)copy); + if (rc < 0) { + break; + } + } + + iter_destroy(&it); + + if (rc == RTTI_ITER_NO_DATA) { + rc = 0; + } + + if (rc < 0) { + destroy(next); + return rc; + } + + *pnext = next; + + return 0; +} + +int limit_replace_config(xce_sched_plugin_t* plugin, xce_sched_core_t* core, + xce_conf4sched_t* config) +{ + limit_plugin_data_t* privdata; + var limits = NULL; + var instances = NULL; + var old_limits; + var old_instances; + int rc; + + if ((plugin == NULL) || (core == NULL) || (config == NULL)) { + return -EINVAL; + } + + privdata = (limit_plugin_data_t*)plugin->data; + if (privdata == NULL) { + return -EINVAL; + } + + rc = build_limit_set_(core, config, &limits); + if (rc < 0) { + return rc; + } + + lock(privdata->instances, RTTI_LOCK_READ); + rc = rebind_instances_(privdata->instances, limits, &instances); + unlock(privdata->instances); + + if (rc < 0) { + destroy(limits); + return rc; + } + + old_limits = privdata->limits; + old_instances = privdata->instances; + privdata->limits = limits; + privdata->instances = instances; + + /* instances first: they point into the old limits */ + destroy(old_instances); + destroy(old_limits); + + return 0; +} diff --git a/src/scheduler/src/plugins/limit_internal.h b/src/scheduler/src/plugins/limit_internal.h new file mode 100644 index 00000000..51964d2a --- /dev/null +++ b/src/scheduler/src/plugins/limit_internal.h @@ -0,0 +1,29 @@ +#if !defined(_LIMIT_INTERNAL_H_) + +#define _LIMIT_INTERNAL_H_ 1 + +/* + * limit_internal.h - private to the limit plugin (limit.c and + * limit_config.c, split to stay under the 800-line cap). + */ +#include + +#include "plugin.h" +#include "scheduler.h" + +typedef struct { + var limits; /* xce_limit_t by limit name; kill_limit */ + var instances; /* xce_vres_t usage instances by vres name; kill_vres */ +} limit_plugin_data_t; + +/* empty tables with the shape and datakiller each field above needs */ +extern var limit_make_limits_table(void); +extern var limit_make_instances_table(void); + +/* builds config->limits[] against core's group managers and replaces the + * plugin's configured limit set with it (bug-1967); see limit_config.c */ +extern int limit_replace_config(xce_sched_plugin_t* plugin, + xce_sched_core_t* core, + xce_conf4sched_t* config); + +#endif diff --git a/src/scheduler/testsuite/test_limit_plugin.c b/src/scheduler/testsuite/test_limit_plugin.c index f42d7ffa..113cbaa9 100644 --- a/src/scheduler/testsuite/test_limit_plugin.c +++ b/src/scheduler/testsuite/test_limit_plugin.c @@ -99,6 +99,7 @@ int plugin_register(void* sched, void* plugin) } #include "../src/plugins/limit.c" +#include "../src/plugins/limit_config.c" /* bug-1967: limit.c's seed */ void setUp(void) { return; } void tearDown(void) { return; } @@ -508,6 +509,237 @@ static void test_limit_scopes_resolve_against_own_group_kind(void) free_limit_config(limits[1]); } +/* ---------------------------------------------------------------------- */ +/* bug-1967: a re-pushed config (live config upgrade, or a mid-replay + * CONFIG_PUSH in the simulator) REPLACES the configured limit set. Before + * the fix every name of the second push already existed, so the first + * key_add() failed with -EEXIST and the push changed nothing. + * ---------------------------------------------------------------------- */ + +/* pushes `n` limit configs through the config hook, as both drivers do */ +static int push_limits(xce_sched_plugin_t* plugin, xce_limit_config_t** limits, + int n) +{ + xce_conf4sched_t config; + + memset(&config, 0, sizeof(config)); + config.nlimits = n; + config.limits = limits; + + return limit_update_config(plugin, &config); +} + +static xce_limit_t* find_limit(xce_sched_plugin_t* plugin, const char* name) +{ + limit_plugin_data_t* privdata = (limit_plugin_data_t*)plugin->data; + xce_limit_t* limit = NULL; + + key_find(privdata->limits, (void*)name, (void**)&limit); + + return limit; +} + +static xce_vres_t* find_instance(xce_sched_plugin_t* plugin, const char* name) +{ + limit_plugin_data_t* privdata = (limit_plugin_data_t*)plugin->data; + xce_vres_t* vres = NULL; + + key_find(privdata->instances, (void*)name, (void**)&vres); + + return vres; +} + +static int count_entries(var table) +{ + iterator_t it; + void* item = NULL; + int n = 0; + + TEST_ASSERT_TRUE(iter_make(&it, table) >= 0); + while (iter_next(&it, &item) == RTTI_ITER_OK) { + n++; + } + iter_destroy(&it); + + return n; +} + +/* a usage instance as match_job_global_limit_() interns it: keyed by its + * virtual resource's name, pointing at the configured limit */ +static void add_instance(xce_sched_plugin_t* plugin, xce_resource_t* res, + const char* limit_name) +{ + limit_plugin_data_t* privdata = (limit_plugin_data_t*)plugin->data; + xce_vres_t* vres = (xce_vres_t*)calloc(1, sizeof(*vres)); + + TEST_ASSERT_NOT_NULL(vres); + vres->vres = res; + vres->limit = find_limit(plugin, limit_name); + TEST_ASSERT_NOT_NULL(vres->limit); + + TEST_ASSERT_TRUE(key_add(privdata->instances, (void*)res->name, vres) + >= 0); +} + +static void test_repush_changed_value_takes_effect(void) +{ + xce_scheduler_t sched; + xce_sched_plugin_t plugin; + xce_limit_config_t* limits[1]; + + fixture_init(&sched, &plugin); + + limits[0] = make_limit_config("lim_a"); + TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1)); + + limits[0]->val->val = 20.0; + TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1)); + + TEST_ASSERT_NOT_NULL(find_limit(&plugin, "lim_a")); + TEST_ASSERT_EQUAL_FLOAT(20.0, find_limit(&plugin, "lim_a")->val.val); + + free_limit_config(limits[0]); +} + +static void test_repush_adds_new_limit_after_existing(void) +{ + xce_scheduler_t sched; + xce_sched_plugin_t plugin; + xce_limit_config_t* limits[2]; + + fixture_init(&sched, &plugin); + + limits[0] = make_limit_config("lim_a"); + limits[1] = make_limit_config("lim_b"); + TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1)); + TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 2)); + + TEST_ASSERT_NOT_NULL(find_limit(&plugin, "lim_a")); + TEST_ASSERT_NOT_NULL(find_limit(&plugin, "lim_b")); + + free_limit_config(limits[0]); + free_limit_config(limits[1]); +} + +static void test_repush_removes_dropped_limit(void) +{ + xce_scheduler_t sched; + xce_sched_plugin_t plugin; + xce_limit_config_t* limits[2]; + limit_plugin_data_t* privdata; + + fixture_init(&sched, &plugin); + privdata = (limit_plugin_data_t*)plugin.data; + + limits[0] = make_limit_config("lim_a"); + limits[1] = make_limit_config("lim_b"); + TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 2)); + TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, &limits[1], 1)); + + TEST_ASSERT_NULL(find_limit(&plugin, "lim_a")); + TEST_ASSERT_NOT_NULL(find_limit(&plugin, "lim_b")); + TEST_ASSERT_EQUAL_INT(1, count_entries(privdata->limits)); + + free_limit_config(limits[0]); + free_limit_config(limits[1]); +} + +/* A kept limit's instance is rebound to the new definition (its usage lives + * on in the resource manager under the same virtual resource); a removed + * limit's instance is dropped, so none points into a freed definition. */ +static void test_repush_rebinds_kept_and_drops_removed_instances(void) +{ + static xce_resource_t res_a = { .name = "limit=lim_a", .idx = 7 }; + static xce_resource_t res_b = { .name = "limit=lim_b", .idx = 8 }; + xce_scheduler_t sched; + xce_sched_plugin_t plugin; + xce_limit_config_t* limits[2]; + limit_plugin_data_t* privdata; + xce_vres_t* vres; + + fixture_init(&sched, &plugin); + privdata = (limit_plugin_data_t*)plugin.data; + + limits[0] = make_limit_config("lim_a"); + limits[1] = make_limit_config("lim_b"); + TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 2)); + add_instance(&plugin, &res_a, "lim_a"); + add_instance(&plugin, &res_b, "lim_b"); + + limits[0]->val->val = 20.0; + TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1)); + + vres = find_instance(&plugin, "limit=lim_a"); + TEST_ASSERT_NOT_NULL(vres); + TEST_ASSERT_EQUAL_PTR(find_limit(&plugin, "lim_a"), vres->limit); + TEST_ASSERT_EQUAL_FLOAT(20.0, vres->limit->val.val); + TEST_ASSERT_EQUAL_PTR(&res_a, vres->vres); + + TEST_ASSERT_NULL(find_instance(&plugin, "limit=lim_b")); + TEST_ASSERT_EQUAL_INT(1, count_entries(privdata->instances)); + + free_limit_config(limits[0]); + free_limit_config(limits[1]); +} + +/* A change that alters the instance's identity (here per_user: the vres + * name now carries the user, so the old shared instance no longer names + * the usage the limit counts) drops the instance; the next job match + * instantiates the right one. */ +static void test_repush_scope_change_drops_instance(void) +{ + static xce_resource_t res_a = { .name = "limit=lim_a", .idx = 7 }; + xce_scheduler_t sched; + xce_sched_plugin_t plugin; + xce_limit_config_t* limits[1]; + + fixture_init(&sched, &plugin); + + limits[0] = make_limit_config("lim_a"); + TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1)); + add_instance(&plugin, &res_a, "lim_a"); + + limits[0]->per_user = 1; + TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1)); + + TEST_ASSERT_NOT_NULL(find_limit(&plugin, "lim_a")); + TEST_ASSERT_NULL(find_instance(&plugin, "limit=lim_a")); + + free_limit_config(limits[0]); +} + +/* A push that fails part-way (a duplicate name) leaves the previous limit + * set and its instances untouched instead of half-replacing them. */ +static void test_repush_failure_keeps_previous_set(void) +{ + static xce_resource_t res_a = { .name = "limit=lim_a", .idx = 7 }; + xce_scheduler_t sched; + xce_sched_plugin_t plugin; + xce_limit_config_t* limits[3]; + xce_vres_t* vres; + + fixture_init(&sched, &plugin); + + limits[0] = make_limit_config("lim_a"); + TEST_ASSERT_EQUAL_INT(0, push_limits(&plugin, limits, 1)); + add_instance(&plugin, &res_a, "lim_a"); + + limits[1] = make_limit_config("lim_c"); + limits[2] = make_limit_config("lim_c"); + limits[0]->val->val = 20.0; + TEST_ASSERT_EQUAL_INT(-EEXIST, push_limits(&plugin, limits, 3)); + + TEST_ASSERT_EQUAL_FLOAT(10.0, find_limit(&plugin, "lim_a")->val.val); + TEST_ASSERT_NULL(find_limit(&plugin, "lim_c")); + vres = find_instance(&plugin, "limit=lim_a"); + TEST_ASSERT_NOT_NULL(vres); + TEST_ASSERT_EQUAL_PTR(find_limit(&plugin, "lim_a"), vres->limit); + + free_limit_config(limits[0]); + free_limit_config(limits[1]); + free_limit_config(limits[2]); +} + int main(void) { int rc; @@ -522,6 +754,12 @@ int main(void) RUN_TEST(test_get_configured_limits_handle_iterates_registered_limit); RUN_TEST(test_get_configured_limits_registered_with_zero_limits); RUN_TEST(test_limit_scopes_resolve_against_own_group_kind); + RUN_TEST(test_repush_changed_value_takes_effect); + RUN_TEST(test_repush_adds_new_limit_after_existing); + RUN_TEST(test_repush_removes_dropped_limit); + RUN_TEST(test_repush_rebinds_kept_and_drops_removed_instances); + RUN_TEST(test_repush_scope_change_drops_instance); + RUN_TEST(test_repush_failure_keeps_previous_set); rc = UNITY_END(); xstring_table_fini(); diff --git a/src/scheduler/testsuite/test_repl_limit_table.c b/src/scheduler/testsuite/test_repl_limit_table.c index 14956cdd..638ec617 100644 --- a/src/scheduler/testsuite/test_repl_limit_table.c +++ b/src/scheduler/testsuite/test_repl_limit_table.c @@ -81,6 +81,7 @@ #include "../src/plugin.c" #include "../src/plugins/builtin_fcfs.c" #include "../src/plugins/limit.c" +#include "../src/plugins/limit_config.c" /* bug-1967: limit.c's seed */ #include "../src/recording.c" #include "../src/recording_session.c" #include "../src/repl.c" @@ -586,6 +587,29 @@ static void add_configured_limit(xce_sched_plugin_t* plugin, const char* name) free_limit_config(limits[0]); } +/* two limits in ONE config push: a push replaces the configured set + * (bug-1967), so two add_configured_limit() calls leave only the second */ +static void add_configured_limit_pair(xce_sched_plugin_t* plugin, + const char* name_a, const char* name_b) +{ + xce_limit_config_t* limits[2]; + xce_conf4sched_t config; + int rc; + + limits[0] = make_limit_config(name_a); + limits[1] = make_limit_config(name_b); + + memset(&config, 0, sizeof(config)); + config.nlimits = 2; + config.limits = limits; + + rc = limit_update_config(plugin, &config); + TEST_ASSERT_EQUAL_INT(0, rc); + + free_limit_config(limits[0]); + free_limit_config(limits[1]); +} + /* * add_configured_limit_with_host_exclusion - task 2.2's own Requirement 1.5 * fixture: an "all except X" exclusion group on the HOST dimension, the @@ -1216,8 +1240,7 @@ static void test_info_limit_all_displays_every_configured_limits_setting(void) TEST_ASSERT_EQUAL_INT(0, rc); register_limit_plugin(&sim, &plugin, &group_sched); - add_configured_limit(&plugin, "limitA"); - add_configured_limit(&plugin, "limitB"); + add_configured_limit_pair(&plugin, "limitA", "limitB"); captured = capture_info_limit(&sim, "all"); @@ -1965,8 +1988,7 @@ test_info_limit_dispatch_all_reaches_repl_info_limit_via_real_dispatch(void) TEST_ASSERT_EQUAL_INT(0, rc); register_limit_plugin(&sim, &plugin, &group_sched); - add_configured_limit(&plugin, "limitA"); - add_configured_limit(&plugin, "limitB"); + add_configured_limit_pair(&plugin, "limitA", "limitB"); captured = capture_dispatch(&sim, "info limit all"); diff --git a/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c b/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c index 6f5701b8..25ae9b65 100644 --- a/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c +++ b/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c @@ -128,6 +128,7 @@ * limit_new_plugin()/limit_seed_config()), which are now real symbols * defined by this include and would collide at link time if left in * place. */ +#include "../src/plugins/limit_config.c" /* bug-1967: limit.c's seed */ #include "../src/recording.c" #include "../src/recording_session.c" #include "../src/repl.c"