fix(harness): xce_test argument validation, RPC error text and leaks (13 bugs)
Stack overflows on user input:
- bug-1702 jobrun -m: host segment strncpy'd into a 256-byte buffer
unbounded (ASAN stack-buffer-overflow); now cmdutil_split_host_tasks()
refuses an over-long host.
- bug-1703 jobsub -n: min strncpy'd into a 64-byte buffer; now
cmdutil_parse_ntasks() parses in place, 1 <= min <= max.
- resupdate's 16-entry vals[] had no bound either; a 17th -n overflowed.
Wrong or silent results:
- bug-1574 resupdate type/model: set_resval() had no string case, left the
entry's name uninitialized, and crashed the encoder. Moved to
resval_typeresolve.c as resval_fill(), which fills every type.
- bug-1705 jobsub -x was overwritten (and leaked) by the default exdata;
cmdutil_default_exdata() only fills an unset one. jobsub's
execuser/exechome/execcwd copies leaked on every call.
- bug-1707 a trailing -n without -v was dropped silently; resupdate also
returned 0 after its RPC failed and "succeeded" with nothing to send.
- bug-1708 jobkill -s SIGKILL became signal 0 and was sent.
- bug-1709 jobinfo exited 0 with no -j, a malformed -j, or no hits.
- bug-1710 hello -c 0 sent nothing and reported success.
- bug-1712 a duplicate -v was reported as "no res name found".
- bug-1777 workerlist showed join-time maxmem/maxswap/maxtmp, not the
effective values (hosts.yaml overrides) the reply already carries in
resvals_to_sched and the scheduler uses.
- bug-1793 join's maxswap/maxtmp defaults were 1 KB / 2 KB in bytes; now
1 GiB / 2 GiB, and -m/-s/-t no longer overflow atoi() above 2 GB.
Error reporting and leaks:
- bug-1796 RPC failures printed perror() of a stale errno ("Connection
refused" / "Invalid argument" / "Inappropriate ioctl for device" for
the same failure). cmdutil_rpc_error() prints the RPC's own code
(E_XCE_* included) at all 19 sites.
- bug-1797 qctrl leaked the queue name on the RPC-error path; repeated
-q/-h leaked the earlier copy.
New src/harness/cmdutil.c (+ cmdutil_unit_test, 14 cases); 9 new
test_test_cli.c cases, 8 RED on the old xce_test; new resval_fill,
worker_effective_attr and join-default tests (join ones RED on the old
join_worker_args.c). Harness suite 6/6 (asan), harness_unit_test 141/0.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
eee76a75ab
commit
da1a663f05
14 changed files with 1107 additions and 286 deletions
|
|
@ -63,6 +63,7 @@
|
|||
#include "snapverify.h" /* offline checkpoint-file structural validator (cmd_snapverify) */
|
||||
#include "configcompare.h" /* cross-member active-config comparator (cmd_configcompare) */
|
||||
#include "confirm.h" /* destructive-command confirmation gate (xce-test-cli-ux task 2.1) */
|
||||
#include "cmdutil.h" /* argument parsing + RPC-error reporting helpers */
|
||||
#include "joblist_display.h" /* worker_status_str/job_state_str/format_job_time
|
||||
* (xce-test-cli-ux task 2.5 remediation) */
|
||||
#include "join_worker_args.h" /* join_worker_parse_attrs (xce-test-cli-ux
|
||||
|
|
@ -156,14 +157,6 @@ typedef struct {
|
|||
int ntasks;
|
||||
} host_task_t;
|
||||
|
||||
typedef struct {
|
||||
char* PATH;
|
||||
} sub_envs_t;
|
||||
|
||||
typedef struct {
|
||||
sub_envs_t sub_env_vars;
|
||||
} exdata_t;
|
||||
|
||||
#define MAX_RUN_JOBS (20)
|
||||
#define MAX_TASK_HOSTS (5)
|
||||
#define MAX_RUN_TASKS (50)
|
||||
|
|
@ -425,7 +418,8 @@ static const char USAGE_JOIN_WORKER_HEAD[] =
|
|||
" [-t <maxtmp>] [-d <ndisks>]\n"
|
||||
" Joins the cluster as a worker (hosttype LINUX64, hostmodel Intel64).\n";
|
||||
|
||||
static const char USAGE_JOIN_WORKER_TAIL[] = "example: join -c 8 -m 16384\n";
|
||||
static const char USAGE_JOIN_WORKER_TAIL[]
|
||||
= "example: join -c 8 -m 17179869184 (16 GB in bytes)\n";
|
||||
|
||||
static const char USAGE_JOBSTART_HEAD[] =
|
||||
"jstart -j <jobid> [-p <pid>]\n"
|
||||
|
|
@ -2055,9 +2049,19 @@ int cmd_hello(int argc, char* argv[])
|
|||
|
||||
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL)) != -1) {
|
||||
switch (opt) {
|
||||
case 'c':
|
||||
count = atoi(optarg);
|
||||
case 'c': {
|
||||
long n;
|
||||
|
||||
/* bug-1710: 0, negative or non-numeric sent nothing and still
|
||||
* reported success */
|
||||
if (cmdutil_parse_long(optarg, 1, INT_MAX, &n) < 0) {
|
||||
fprintf(stderr, "hello: -c expects a count >= 1, got '%s'\n",
|
||||
optarg);
|
||||
return -EINVAL;
|
||||
}
|
||||
count = (int)n;
|
||||
break;
|
||||
}
|
||||
case 'm':
|
||||
msg = optarg;
|
||||
break;
|
||||
|
|
@ -2274,8 +2278,7 @@ int cmd_reslist(int argc, char* argv[])
|
|||
free(call);
|
||||
|
||||
if (rc < 0) {
|
||||
SET_ERROR(-rc);
|
||||
perror("xce_reslist");
|
||||
cmdutil_rpc_error("xce_reslist", rc);
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -2417,8 +2420,7 @@ int cmd_workerlist(int argc, char* argv[])
|
|||
free(call);
|
||||
|
||||
if (rc < 0) {
|
||||
SET_ERROR(-rc);
|
||||
perror("xce_workerlist");
|
||||
cmdutil_rpc_error("xce_workerlist", rc);
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -2438,16 +2440,18 @@ int cmd_workerlist(int argc, char* argv[])
|
|||
for (i = 0; i < rc; i++) {
|
||||
cJSON* row = listfmt_json_new_row(array);
|
||||
|
||||
workerlist_json_row(row, workers[i]->name,
|
||||
worker_status_str(workers[i]->status),
|
||||
workers[i]->attrs->hosttype,
|
||||
workers[i]->attrs->hostmodel,
|
||||
workers[i]->attrs->cpufactor,
|
||||
workers[i]->attrs->maxcpus,
|
||||
workers[i]->attrs->maxmem,
|
||||
workers[i]->attrs->maxswap,
|
||||
workers[i]->attrs->maxtmp,
|
||||
workers[i]->attrs->ndisks);
|
||||
/* bug-1777: effective values, as the scheduler sees them */
|
||||
workerlist_json_row(
|
||||
row, workers[i]->name, worker_status_str(workers[i]->status),
|
||||
workers[i]->attrs->hosttype, workers[i]->attrs->hostmodel,
|
||||
workers[i]->attrs->cpufactor, workers[i]->attrs->maxcpus,
|
||||
worker_effective_attr(workers[i], "maxmem",
|
||||
workers[i]->attrs->maxmem),
|
||||
worker_effective_attr(workers[i], "maxswap",
|
||||
workers[i]->attrs->maxswap),
|
||||
worker_effective_attr(workers[i], "maxtmp",
|
||||
workers[i]->attrs->maxtmp),
|
||||
workers[i]->attrs->ndisks);
|
||||
}
|
||||
|
||||
listfmt_json_finish(array);
|
||||
|
|
@ -2468,12 +2472,17 @@ int cmd_workerlist(int argc, char* argv[])
|
|||
char maxswap_buf[64];
|
||||
char maxtmp_buf[64];
|
||||
|
||||
format_bytes_human(workers[i]->attrs->maxmem, maxmem_buf,
|
||||
sizeof(maxmem_buf));
|
||||
format_bytes_human(workers[i]->attrs->maxswap, maxswap_buf,
|
||||
sizeof(maxswap_buf));
|
||||
format_bytes_human(workers[i]->attrs->maxtmp, maxtmp_buf,
|
||||
sizeof(maxtmp_buf));
|
||||
/* bug-1777: the effective value (a hosts.yaml override included),
|
||||
* which is what the scheduler uses, not the join-time attrs */
|
||||
format_bytes_human(worker_effective_attr(workers[i], "maxmem",
|
||||
workers[i]->attrs->maxmem),
|
||||
maxmem_buf, sizeof(maxmem_buf));
|
||||
format_bytes_human(worker_effective_attr(workers[i], "maxswap",
|
||||
workers[i]->attrs->maxswap),
|
||||
maxswap_buf, sizeof(maxswap_buf));
|
||||
format_bytes_human(worker_effective_attr(workers[i], "maxtmp",
|
||||
workers[i]->attrs->maxtmp),
|
||||
maxtmp_buf, sizeof(maxtmp_buf));
|
||||
|
||||
printf("%-20s%-8s%-25s%-25s%12f%12d%12s%12s%12s%12d\n",
|
||||
workers[i]->name, worker_status_str(workers[i]->status),
|
||||
|
|
@ -2568,7 +2577,7 @@ static int joblist_query(const char* find_jobid, int* contains, int print_table,
|
|||
* and EXPECTS failure until a primary serves) suppress the per-attempt
|
||||
* error print so a normal failover wait does not spam the console. */
|
||||
if (!quiet) {
|
||||
perror("xce_joblist");
|
||||
cmdutil_rpc_error("xce_joblist", rc);
|
||||
}
|
||||
return rc;
|
||||
}
|
||||
|
|
@ -3289,6 +3298,9 @@ static int jobinfo_thunk(void* ctxp)
|
|||
int cmd_jobinfo(int argc, char* argv[])
|
||||
{
|
||||
int opt;
|
||||
int ngiven = 0; /* -j values given */
|
||||
int nfound = 0; /* lookups that succeeded */
|
||||
int lastrc = -EINVAL;
|
||||
struct option long_opts[JOBINFO_NFLAGS + 1];
|
||||
char optstring[16];
|
||||
|
||||
|
|
@ -3311,9 +3323,16 @@ int cmd_jobinfo(int argc, char* argv[])
|
|||
|
||||
switch (opt) {
|
||||
case 'j':
|
||||
ngiven++;
|
||||
parse_jobid(&jobid, optarg);
|
||||
if (jobid == 0) {
|
||||
/* bug-1709: a malformed jobid used to be skipped silently */
|
||||
fprintf(stderr, "jobinfo: invalid jobid '%s'\n", optarg);
|
||||
lastrc = -EINVAL;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (jobid > 0) {
|
||||
{
|
||||
jobinfo_call_t* call
|
||||
= (jobinfo_call_t*)calloc(1, sizeof(*call));
|
||||
|
||||
|
|
@ -3341,12 +3360,14 @@ int cmd_jobinfo(int argc, char* argv[])
|
|||
free(call);
|
||||
|
||||
if (rc < 0) {
|
||||
perror("xcecli_jobinfo");
|
||||
cmdutil_rpc_error("xcecli_jobinfo", rc);
|
||||
lastrc = rc;
|
||||
continue;
|
||||
}
|
||||
|
||||
print_jobinfo(job);
|
||||
free_Job_ptr(&job);
|
||||
nfound++;
|
||||
}
|
||||
|
||||
break;
|
||||
|
|
@ -3356,7 +3377,14 @@ int cmd_jobinfo(int argc, char* argv[])
|
|||
"jobinfo -j <jobid> [-j <jobid> ...]");
|
||||
}
|
||||
}
|
||||
return 0;
|
||||
|
||||
/* bug-1709: fail when there was nothing to look up or every lookup
|
||||
* failed, so a script sees it in the exit code */
|
||||
if (ngiven == 0) {
|
||||
fprintf(stderr, "jobinfo: no jobid given (use -j <jobid>)\n");
|
||||
return -EINVAL;
|
||||
}
|
||||
return (nfound > 0) ? 0 : lastrc;
|
||||
}
|
||||
|
||||
static int fill_job_alloc(xce_job_alloc_t* jalloc, int nhosts,
|
||||
|
|
@ -3457,19 +3485,28 @@ int cmd_jobrun(int argc, char* argv[])
|
|||
return -1;
|
||||
}
|
||||
|
||||
if (strchr(optarg, (int)':') != NULL) {
|
||||
char* p;
|
||||
char buf[256];
|
||||
{
|
||||
char buf[256];
|
||||
int prc = cmdutil_split_host_tasks(optarg, buf, sizeof(buf),
|
||||
&count);
|
||||
|
||||
p = strchr(optarg, (int)':');
|
||||
count = atoi(p + 1);
|
||||
memset(buf, 0x0, sizeof(buf));
|
||||
|
||||
strncpy(buf, optarg, (int)(p - optarg));
|
||||
/* bug-1702: the host segment used to be strncpy'd into buf
|
||||
* with no length check */
|
||||
if (prc == -ENAMETOOLONG) {
|
||||
fprintf(stderr,
|
||||
"jobrun: host name in -m '%.32s...' is longer "
|
||||
"than %zu characters\n",
|
||||
optarg, sizeof(buf) - 1);
|
||||
return -EINVAL;
|
||||
}
|
||||
if (prc < 0) {
|
||||
fprintf(stderr,
|
||||
"jobrun: -m expects <host>[:<ntasks>] with "
|
||||
"ntasks >= 1, got '%s'\n",
|
||||
optarg);
|
||||
return -EINVAL;
|
||||
}
|
||||
host = strdup(buf);
|
||||
} else {
|
||||
host = strdup(optarg);
|
||||
count = 1;
|
||||
}
|
||||
|
||||
hosts[nhosts].host = host;
|
||||
|
|
@ -3523,8 +3560,7 @@ int cmd_jobrun(int argc, char* argv[])
|
|||
}
|
||||
|
||||
if (rc < 0) {
|
||||
SET_ERROR(-rc);
|
||||
perror("xce_jobdispatch");
|
||||
cmdutil_rpc_error("xce_jobdispatch", rc);
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -3536,29 +3572,6 @@ int cmd_jobrun(int argc, char* argv[])
|
|||
return 0;
|
||||
}
|
||||
|
||||
static int build_job_envs(xce_job_submit_t* submit)
|
||||
{
|
||||
exdata_t exdata = { .sub_env_vars = { .PATH = getenv("PATH") } };
|
||||
|
||||
s2j_create_json_obj(json_exdata);
|
||||
s2j_create_json_obj(json_envs);
|
||||
s2j_json_set_basic_element(json_envs, &(exdata.sub_env_vars), var_string,
|
||||
PATH);
|
||||
cJSON_AddItemToObject(json_exdata, "sub_env_vars", json_envs);
|
||||
|
||||
submit->submit->start_info->exdata = cJSON_Print(json_exdata);
|
||||
cJSON_Delete(json_exdata);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
static int build_job_exdata(xce_job_submit_t* submit)
|
||||
{
|
||||
build_job_envs(submit);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
static const flagspec_t jobsub_flags[] = {
|
||||
{ 'u', "user", "user", required_argument, 1, NULL },
|
||||
{ 'q', "queue", "queue", required_argument, 1, NULL },
|
||||
|
|
@ -3639,27 +3652,19 @@ int cmd_jobsub(int argc, char* argv[])
|
|||
submit.start_info->cmdline = strdup(optarg);
|
||||
break;
|
||||
case 'x':
|
||||
FREEUP(submit.start_info->exdata);
|
||||
submit.start_info->exdata = strdup(optarg);
|
||||
break;
|
||||
case 'n':
|
||||
if (strchr(optarg, (int)',') != NULL) {
|
||||
char buf[64];
|
||||
char* p = strchr(optarg, (int)',');
|
||||
|
||||
max = atoi(p + 1);
|
||||
memset(buf, 0x0, sizeof(buf));
|
||||
strncpy(buf, optarg, (int)(p - optarg));
|
||||
min = atoi(buf);
|
||||
|
||||
if (max == 0) {
|
||||
fprintf(stderr,
|
||||
"Incorrect setting of max tasks count, set to "
|
||||
"same as min task value\n");
|
||||
max = min;
|
||||
}
|
||||
} else {
|
||||
min = atoi(optarg);
|
||||
max = min;
|
||||
/* bug-1703: the min part used to be strncpy'd into a 64-byte
|
||||
* buffer with no length check */
|
||||
if (cmdutil_parse_ntasks(optarg, &min, &max) < 0) {
|
||||
fprintf(stderr,
|
||||
"jobsub: -n expects <min>[,<max>] with 1 <= min <= "
|
||||
"max, got '%.40s'\n",
|
||||
optarg);
|
||||
rc = -EINVAL;
|
||||
goto cleanup;
|
||||
}
|
||||
break;
|
||||
case 'a':
|
||||
|
|
@ -3677,12 +3682,6 @@ int cmd_jobsub(int argc, char* argv[])
|
|||
}
|
||||
}
|
||||
|
||||
if (min == 0) {
|
||||
fprintf(stderr,
|
||||
"Incorrect setting of min task count, set to 1 as default\n");
|
||||
min = 1;
|
||||
}
|
||||
|
||||
if (NULLSTR(submit.user)) {
|
||||
fprintf(stderr, "you need to specify job user with -u\n");
|
||||
rc = -EINVAL;
|
||||
|
|
@ -3734,7 +3733,12 @@ int cmd_jobsub(int argc, char* argv[])
|
|||
submit.start_info->execcwd = strdup("/tmp");
|
||||
}
|
||||
|
||||
build_job_exdata(&jobsubmit);
|
||||
/* bug-1705: only when -x did not give one -- this used to overwrite
|
||||
* (and leak) -x's exdata unconditionally */
|
||||
if (cmdutil_default_exdata(&submit.start_info->exdata, getenv("PATH")) < 0) {
|
||||
rc = -ENOMEM;
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
jobsubmit.array_size = arsize;
|
||||
|
||||
|
|
@ -3781,6 +3785,9 @@ cleanup:
|
|||
FREEUP(submit.resreq);
|
||||
FREEUP(submit.start_info->cmdline);
|
||||
FREEUP(submit.start_info->exdata);
|
||||
FREEUP(submit.start_info->execuser);
|
||||
FREEUP(submit.start_info->exechome);
|
||||
FREEUP(submit.start_info->execcwd);
|
||||
FREEUP(submit.start_info);
|
||||
|
||||
/* Report success only on a clean ack that echoes back the EXACT JobId we
|
||||
|
|
@ -4115,9 +4122,20 @@ int cmd_jobkill(int argc, char* argv[])
|
|||
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
|
||||
!= -1) {
|
||||
switch (opt) {
|
||||
case 's':
|
||||
signo = atoi(optarg);
|
||||
case 's': {
|
||||
long n;
|
||||
|
||||
/* bug-1708: a non-numeric value became signal 0 and the kill
|
||||
* was still sent and reported */
|
||||
if (cmdutil_parse_long(optarg, 1, NSIG - 1, &n) < 0) {
|
||||
fprintf(stderr,
|
||||
"jobkill: -s expects a signal number 1-%d, got '%s'\n",
|
||||
NSIG - 1, optarg);
|
||||
return -EINVAL;
|
||||
}
|
||||
signo = (int)n;
|
||||
break;
|
||||
}
|
||||
case 'j':
|
||||
parse_jobid(&jobid, optarg);
|
||||
break;
|
||||
|
|
@ -4185,7 +4203,7 @@ int cmd_jobkill(int argc, char* argv[])
|
|||
}
|
||||
|
||||
if (rc < 0) {
|
||||
perror("xcecli_jobctrl");
|
||||
cmdutil_rpc_error("xcecli_jobctrl", rc);
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -4302,7 +4320,7 @@ int cmd_jobckpt(int argc, char* argv[])
|
|||
}
|
||||
|
||||
if (rc < 0) {
|
||||
perror("xcecli_jobctrl");
|
||||
cmdutil_rpc_error("xcecli_jobctrl", rc);
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -4385,6 +4403,7 @@ int cmd_qctrl(int argc, char* argv[])
|
|||
action = QUEUE_CLOSE;
|
||||
break;
|
||||
case 'q':
|
||||
FREEUP(queue);
|
||||
queue = strdup(optarg);
|
||||
break;
|
||||
case 'y':
|
||||
|
|
@ -4437,6 +4456,7 @@ int cmd_qctrl(int argc, char* argv[])
|
|||
qctrl_call_t* call = (qctrl_call_t*)calloc(1, sizeof(*call));
|
||||
|
||||
if (call == NULL) {
|
||||
free(queue);
|
||||
return -ENOMEM;
|
||||
}
|
||||
|
||||
|
|
@ -4457,7 +4477,8 @@ int cmd_qctrl(int argc, char* argv[])
|
|||
}
|
||||
|
||||
if (rc < 0) {
|
||||
perror("xcecli_queuectrl");
|
||||
cmdutil_rpc_error("xcecli_queuectrl", rc);
|
||||
free(queue); /* bug-1797: leaked on this path */
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -4533,6 +4554,7 @@ int cmd_hostctrl(int argc, char* argv[])
|
|||
action = HOST_CLOSE;
|
||||
break;
|
||||
case 'h':
|
||||
FREEUP(hostname);
|
||||
hostname = strdup(optarg);
|
||||
break;
|
||||
case 'y':
|
||||
|
|
@ -4599,7 +4621,7 @@ int cmd_hostctrl(int argc, char* argv[])
|
|||
}
|
||||
|
||||
if (rc < 0) {
|
||||
perror("xcecli_hostctrl");
|
||||
cmdutil_rpc_error("xcecli_hostctrl", rc);
|
||||
free(hostname);
|
||||
return rc;
|
||||
}
|
||||
|
|
@ -4955,8 +4977,7 @@ int cmd_runtasks(int argc, char* argv[])
|
|||
}
|
||||
|
||||
if (rc < 0) {
|
||||
errno = -rc;
|
||||
perror("xcecli_runtask");
|
||||
cmdutil_rpc_error("xcecli_runtask", rc);
|
||||
} else {
|
||||
printf("run task ok\n");
|
||||
}
|
||||
|
|
@ -5108,7 +5129,7 @@ int cmd_setpend(int argc, char* argv[])
|
|||
}
|
||||
|
||||
if (rc < 0) {
|
||||
perror("xcecli_set_pend_reason");
|
||||
cmdutil_rpc_error("xcecli_set_pend_reason", rc);
|
||||
erase_job_preason(&jreasons);
|
||||
return rc;
|
||||
}
|
||||
|
|
@ -5174,8 +5195,7 @@ int cmd_join_worker(int argc, char* argv[])
|
|||
}
|
||||
|
||||
if (rc < 0) {
|
||||
SET_ERROR(-rc);
|
||||
perror("xce_join");
|
||||
cmdutil_rpc_error("xce_join", rc);
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -5291,8 +5311,7 @@ int cmd_jobstart(int argc, char* argv[])
|
|||
}
|
||||
|
||||
if (rc < 0) {
|
||||
SET_ERROR(-rc);
|
||||
perror("xcecli_jobstart");
|
||||
cmdutil_rpc_error("xcecli_jobstart", rc);
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -5394,8 +5413,7 @@ int cmd_jobfinish(int argc, char* argv[])
|
|||
}
|
||||
|
||||
if (rc < 0) {
|
||||
SET_ERROR(-rc);
|
||||
perror("xcecli_jobfinish");
|
||||
cmdutil_rpc_error("xcecli_jobfinish", rc);
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -5504,8 +5522,7 @@ int cmd_jobfail(int argc, char* argv[])
|
|||
}
|
||||
|
||||
if (rc < 0) {
|
||||
SET_ERROR(-rc);
|
||||
perror("xcecli_jobfail");
|
||||
cmdutil_rpc_error("xcecli_jobfail", rc);
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -5514,95 +5531,6 @@ int cmd_jobfail(int argc, char* argv[])
|
|||
return 0;
|
||||
}
|
||||
|
||||
/* set_resval() -- thin wrapper (resource-value-verification task 2.1).
|
||||
*
|
||||
* Type resolution (name -> Value_u_which, including the unrecognized-name
|
||||
* numeric-default fallback and `-t` override) now lives in
|
||||
* resval_typeresolve.c/resval_resolve_type(), shared with the standalone
|
||||
* unit tests. This function keeps only what still needs REPL state: the
|
||||
* xce_val_t allocation/parsing per wire type, and `.host` tagging via
|
||||
* get_setting("myname") (unchanged from before this refactor).
|
||||
*
|
||||
* `type_override`: RESVAL_TYPE_UNKNOWN for "no override" (the common case),
|
||||
* or an explicit Value_u_which tag from cmd_resupdate()'s new `-t` flag.
|
||||
* Ignored by resval_resolve_type() for any already-known resource name.
|
||||
*/
|
||||
static int set_resval(
|
||||
xce_resval_t* vals, int idx, char* name, char* val, int type_override)
|
||||
{
|
||||
int valtype;
|
||||
int rc = -1;
|
||||
|
||||
if (name == NULL) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
valtype = resval_resolve_type(name, type_override);
|
||||
if (valtype == RESVAL_TYPE_UNKNOWN) {
|
||||
/* Only reachable if resval_resolve_type() itself ever declines to
|
||||
* resolve a non-NULL name -- not possible today (it always falls
|
||||
* back to Value_u_fvalue or the override), kept as a defensive
|
||||
* guard against a future change to that function's contract. */
|
||||
return rc;
|
||||
}
|
||||
|
||||
rc = 1;
|
||||
vals[idx].val = (xce_val_t*)calloc(1, sizeof(xce_val_t));
|
||||
|
||||
switch (valtype) {
|
||||
case Value_u_fvalue:
|
||||
vals[idx].name = name;
|
||||
vals[idx].val->which = valtype;
|
||||
if (val == NULL) {
|
||||
vals[idx].val->u.fvalue = 0.0;
|
||||
} else {
|
||||
vals[idx].val->u.fvalue = atof(val);
|
||||
}
|
||||
break;
|
||||
case Value_u_ivalue32:
|
||||
vals[idx].name = name;
|
||||
vals[idx].val->which = valtype;
|
||||
if (val == NULL) {
|
||||
vals[idx].val->u.ivalue32 = 0;
|
||||
} else {
|
||||
vals[idx].val->u.ivalue32 = atoi(val);
|
||||
}
|
||||
break;
|
||||
case Value_u_ivalue64:
|
||||
vals[idx].name = name;
|
||||
vals[idx].val->which = valtype;
|
||||
if (val == NULL) {
|
||||
vals[idx].val->u.ivalue64 = 0;
|
||||
} else {
|
||||
vals[idx].val->u.ivalue64 = atol(val);
|
||||
}
|
||||
break;
|
||||
case Value_u_bvalue:
|
||||
vals[idx].name = name;
|
||||
vals[idx].val->which = valtype;
|
||||
if (val == NULL) {
|
||||
vals[idx].val->u.bvalue = 0;
|
||||
} else {
|
||||
if (strcasecmp(val, "t") == 0) {
|
||||
vals[idx].val->u.bvalue = 1;
|
||||
} else {
|
||||
vals[idx].val->u.bvalue = 0;
|
||||
}
|
||||
}
|
||||
break;
|
||||
}
|
||||
/* NOTE (pre-existing, unchanged): Value_u_svalue (type/model, and now
|
||||
* an unrecognized name given `-t string`) has no case here -- the
|
||||
* switch falls through with vals[idx].name/.val->which left unset.
|
||||
* This is set_resval()'s existing behavior for type/model today;
|
||||
* Requirement 1.4 requires preserving it byte-for-byte, so it is
|
||||
* intentionally not fixed by this task. See CONCERNS in this task's
|
||||
* status report. */
|
||||
|
||||
vals[idx].host = get_setting("myname");
|
||||
return rc;
|
||||
}
|
||||
|
||||
/*
|
||||
* resource-value-verification task 2.2: the controller's own update RPC
|
||||
* reply can never distinguish per-name accept/reject -- update_resval_list()
|
||||
|
|
@ -5668,9 +5596,6 @@ static int resupdate_report_one(char* name, char* host, xce_val_t* sent)
|
|||
int ever_found = 0;
|
||||
|
||||
if (name == NULL) {
|
||||
/* set_resval()'s Value_u_svalue fallback path leaves .name unset
|
||||
* (pre-existing gap predating this task -- see set_resval()'s own
|
||||
* comment above); nothing usable to query or report here. */
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
@ -5743,6 +5668,37 @@ static int resupdate_report_one(char* name, char* host, xce_val_t* sent)
|
|||
return 0;
|
||||
}
|
||||
|
||||
/* Append one -n/-v pair to cmd_resupdate's vals[] (16 entries), taking
|
||||
* ownership of `name`. A missing value gets the type's default with a
|
||||
* warning. Returns 0, or -E2BIG when vals[] is full (the old code had no
|
||||
* bound) or -ENOMEM; on error `name` is freed. */
|
||||
static int resupdate_add(xce_resval_t* vals, int* nres, char* name,
|
||||
const char* val, int type_override)
|
||||
{
|
||||
int rc;
|
||||
|
||||
if (*nres >= 16) {
|
||||
fprintf(stderr, "resupdate: at most 16 resources per call\n");
|
||||
free(name);
|
||||
return -E2BIG;
|
||||
}
|
||||
if (val == NULL) {
|
||||
fprintf(stderr,
|
||||
"res `%s' has no value specified, default will be used\n",
|
||||
name);
|
||||
}
|
||||
/* bug-1574: resval_fill() sets name/value for string resources
|
||||
* (type, model) too; set_resval() left them uninitialized */
|
||||
rc = resval_fill(&vals[*nres], name, val, type_override,
|
||||
get_setting("myname"));
|
||||
if (rc < 0) {
|
||||
free(name);
|
||||
return rc;
|
||||
}
|
||||
(*nres)++;
|
||||
return 0;
|
||||
}
|
||||
|
||||
static const flagspec_t resupdate_flags[] = {
|
||||
{ 'n', "resname", "resname", required_argument, 1, NULL },
|
||||
{ 'v', "value", "value", required_argument, 1, NULL },
|
||||
|
|
@ -5776,8 +5732,6 @@ int cmd_resupdate(int argc, char* argv[])
|
|||
#define MAX_N_RES (16)
|
||||
int nres;
|
||||
xce_resval_t vals[MAX_N_RES];
|
||||
xce_resval_t* valptrs[MAX_N_RES];
|
||||
char* subnames[MAX_N_RES];
|
||||
char* resname = NULL;
|
||||
char* resval_str = NULL;
|
||||
int type_override = RESVAL_TYPE_UNKNOWN;
|
||||
|
|
@ -5789,6 +5743,7 @@ int cmd_resupdate(int argc, char* argv[])
|
|||
optind = 0;
|
||||
opterr = 0;
|
||||
nres = 0;
|
||||
memset(vals, 0, sizeof(vals));
|
||||
|
||||
if (flagspec_build_opts(resupdate_flags, RESUPDATE_NFLAGS, long_opts,
|
||||
optstring, sizeof(optstring))
|
||||
|
|
@ -5798,46 +5753,36 @@ int cmd_resupdate(int argc, char* argv[])
|
|||
return -EINVAL;
|
||||
}
|
||||
|
||||
/* -t is deferred: it applies to the pair most recently opened by -n
|
||||
* (design.md's documented order is -n/-v/[-t]), so a pending pair's
|
||||
* set_resval() call is deferred until the NEXT -n or end-of-args --
|
||||
* not fired immediately on -v as before -- so a trailing -t still has
|
||||
* a chance to be seen for that same pair. */
|
||||
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))
|
||||
!= -1) {
|
||||
switch (opt) {
|
||||
case 'n':
|
||||
if (resname != NULL) {
|
||||
if (resval_str == NULL) {
|
||||
fprintf(
|
||||
stderr,
|
||||
"res `%s' has no value specified, default will be used\n",
|
||||
resname);
|
||||
}
|
||||
rc = set_resval(vals, nres, resname, resval_str, type_override);
|
||||
if (rc >= 0) {
|
||||
valptrs[nres] = &vals[nres];
|
||||
/* Tracked independently of vals[nres].name: set_resval()
|
||||
* leaves .name unset on the Value_u_svalue fallback path
|
||||
* (see its own comment), but `resname` here is always the
|
||||
* exact name that was actually submitted for this pair. */
|
||||
subnames[nres] = resname;
|
||||
nres++;
|
||||
}
|
||||
rc = resupdate_add(vals, &nres, resname, resval_str,
|
||||
type_override);
|
||||
resname = NULL;
|
||||
resval_str = NULL;
|
||||
type_override = RESVAL_TYPE_UNKNOWN;
|
||||
if (rc < 0) {
|
||||
goto out;
|
||||
}
|
||||
}
|
||||
|
||||
resname = strdup(optarg);
|
||||
break;
|
||||
case 'v':
|
||||
if (resname != NULL && resval_str == NULL) {
|
||||
resval_str = optarg;
|
||||
} else {
|
||||
if (resname == NULL) {
|
||||
fprintf(stderr,
|
||||
"ignore res value `%s' as no res name found\n",
|
||||
optarg);
|
||||
} else if (resval_str != NULL) {
|
||||
/* bug-1712: this case used to print "no res name found" */
|
||||
fprintf(stderr,
|
||||
"ignoring duplicate value `%s' for res `%s' (it "
|
||||
"already has value `%s')\n",
|
||||
optarg, resname, resval_str);
|
||||
} else {
|
||||
resval_str = optarg;
|
||||
}
|
||||
break;
|
||||
case 't':
|
||||
|
|
@ -5859,31 +5804,38 @@ int cmd_resupdate(int argc, char* argv[])
|
|||
}
|
||||
break;
|
||||
case '?':
|
||||
return flagspec_reject(
|
||||
rc = flagspec_reject(
|
||||
"resupdate", resupdate_flags, RESUPDATE_NFLAGS,
|
||||
argv[optind - 1],
|
||||
"resupdate -n <resname> -v <value> [-t <numeric|string>] "
|
||||
"[-n <resname> -v <value> [-t <numeric|string>] ...]");
|
||||
FREEUP(resname);
|
||||
goto out;
|
||||
}
|
||||
}
|
||||
|
||||
/* Flush the last pending pair. Matches the pre-existing behavior of a
|
||||
* trailing -n with no -v ever seen (silently dropped, same as before
|
||||
* this change) -- only flush here when a value was actually supplied. */
|
||||
if (resname != NULL && resval_str != NULL) {
|
||||
rc = set_resval(vals, nres, resname, resval_str, type_override);
|
||||
if (rc >= 0) {
|
||||
valptrs[nres] = &vals[nres];
|
||||
subnames[nres] = resname;
|
||||
nres++;
|
||||
/* bug-1707: the trailing -n was dropped without a word when it had no
|
||||
* -v; it now gets the same warning and default as a mid-list one */
|
||||
if (resname != NULL) {
|
||||
rc = resupdate_add(vals, &nres, resname, resval_str, type_override);
|
||||
if (rc < 0) {
|
||||
goto out;
|
||||
}
|
||||
}
|
||||
|
||||
if (nres == 0) {
|
||||
fprintf(stderr,
|
||||
"resupdate: no resource given (use -n <resname> -v <value>)\n");
|
||||
rc = -EINVAL;
|
||||
goto out;
|
||||
}
|
||||
|
||||
{
|
||||
resupdate_call_t* call = (resupdate_call_t*)calloc(1, sizeof(*call));
|
||||
|
||||
if (call == NULL) {
|
||||
return -ENOMEM;
|
||||
rc = -ENOMEM;
|
||||
goto out;
|
||||
}
|
||||
|
||||
call->conn = active_ctx->ctx;
|
||||
|
|
@ -5898,6 +5850,8 @@ int cmd_resupdate(int argc, char* argv[])
|
|||
|
||||
rc = xcetest_run_interruptible(resupdate_thunk, call);
|
||||
if (rc == -EINTR) {
|
||||
/* the still-running worker thread owns `call`, whose vals[]
|
||||
* point at the same names/values -- leave them allocated */
|
||||
errno = EINTR;
|
||||
perror("resupdate");
|
||||
return rc;
|
||||
|
|
@ -5909,26 +5863,28 @@ int cmd_resupdate(int argc, char* argv[])
|
|||
|
||||
printf("resource update ok!\n");
|
||||
|
||||
/* The update RPC's own reply can't carry per-name accept/reject
|
||||
* (see the comment above resupdate_report_one()) -- verify each
|
||||
* submitted pair individually via a follow-up query instead. An
|
||||
* interrupt during one of THOSE follow-up queries stops this
|
||||
* verification pass immediately and is reported as this command's
|
||||
* own failure (Requirement 1.3), rather than silently skipping the
|
||||
* remaining pairs' verification. */
|
||||
for (i = 0; i < nres; i++) {
|
||||
int report_rc
|
||||
= resupdate_report_one(subnames[i], vals[i].host, vals[i].val);
|
||||
= resupdate_report_one(vals[i].name, vals[i].host, vals[i].val);
|
||||
|
||||
if (report_rc == -EINTR) {
|
||||
return report_rc;
|
||||
return report_rc; /* same ownership rule as above */
|
||||
}
|
||||
}
|
||||
rc = 0;
|
||||
} else {
|
||||
SET_ERROR(-rc);
|
||||
perror("xcecli_update_resval");
|
||||
/* the RPC's failure used to be printed and then reported as
|
||||
* success (return 0) */
|
||||
cmdutil_rpc_error("xcecli_update_resval", rc);
|
||||
}
|
||||
return 0;
|
||||
|
||||
out:
|
||||
for (int i = 0; i < nres; i++) {
|
||||
free(vals[i].name);
|
||||
resval_fill_release(&vals[i]);
|
||||
}
|
||||
return rc;
|
||||
#undef MAX_N_RES
|
||||
}
|
||||
|
||||
typedef struct {
|
||||
|
|
@ -5967,8 +5923,7 @@ int cmd_join_sched(int argc, char* argv[])
|
|||
}
|
||||
|
||||
if (rc < 0) {
|
||||
SET_ERROR(-rc);
|
||||
perror("xce_join");
|
||||
cmdutil_rpc_error("xce_join", rc);
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -6087,8 +6042,7 @@ int cmd_getconf(int argc, char* argv[])
|
|||
free(call);
|
||||
|
||||
if (rc < 0) {
|
||||
SET_ERROR(-rc);
|
||||
perror("xce_clusterconf");
|
||||
cmdutil_rpc_error("xce_clusterconf", rc);
|
||||
return rc;
|
||||
}
|
||||
|
||||
|
|
@ -6860,7 +6814,7 @@ static int worker_house_keeping(void* arg)
|
|||
|
||||
rc = xcecli_heartbeat(ctx->ctx, HEARTBEAT_INTERVAL);
|
||||
if (rc < 0) {
|
||||
perror("xcecli_heartbeat");
|
||||
cmdutil_rpc_error("xcecli_heartbeat", rc);
|
||||
}
|
||||
|
||||
return rc;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue