Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

powerman: correct potential segfault #201

Merged
merged 4 commits into from
Sep 10, 2024
Merged

powerman: correct potential segfault #201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ee88e78
powerman: correct comment on code location
chu11 Sep 9, 2024
dae192c
powerman: correct potential segfault
chu11 Sep 9, 2024
33b948e
redfishpower: do not output parent errors
chu11 Sep 10, 2024
b45e68f
t: cover parent failure on setresult
chu11 Sep 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/powerman/device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ typedef struct {
*/
#define MAX_LEVELS 2
typedef struct {
int com; /* one of the PM_* above */
int com; /* one of the PM_* script types */
List exec; /* stack of ExecCtxs (outer block is first) */
ActionCB complete_fun; /* callback for action completion */
VerbosePrintf vpf_fun; /* callback for device telemetry */
Expand Down Expand Up @@ -1178,7 +1178,7 @@ static bool _process_setresult(Device *dev, Action *act, ExecCtx *e)
arg->val = xstrdup(str);
}

if (result != RT_SUCCESS) {
if (arg && result != RT_SUCCESS) {
char strbuf[1024];
snprintf(strbuf, sizeof(strbuf), "%s", arg->val);
/* remove trailing carriage return or newline */
Expand Down
68 changes: 37 additions & 31 deletions src/redfishpower/redfishpower.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1520,6 +1520,39 @@ static void cleanup_powermsg(void **x)
}
}

static void output_curl_error(struct CURLMsg *cmsg, struct powermsg *pm)
{
if (cmsg->data.result == CURLE_HTTP_RETURNED_ERROR) {
/* N.B. curl returns this error code for all response
* codes >= 400. So gotta dig in more.
*/
long code;

if (curl_easy_getinfo(cmsg->easy_handle,
CURLINFO_RESPONSE_CODE,
&code) != CURLE_OK)
printf("%s: %s\n", pm->plugname, "http error");
if (code == 400)
printf("%s: %s\n", pm->plugname, "bad request");
else if (code == 401)
printf("%s: %s\n", pm->plugname, "unauthorized");
else if (code == 404)
printf("%s: %s\n", pm->plugname, "not found");
else
printf("%s: %s (%ld)\n",
pm->plugname,
"http error",
code);
}
else
printf("%s: %s\n",
pm->plugname,
curl_easy_strerror(cmsg->data.result));
if (verbose)
printf("%s: %s\n", pm->plugname,
curl_easy_strerror(cmsg->data.result));
}

static void shell(CURLM *mh)
{
int exitflag = 0;
Expand Down Expand Up @@ -1681,36 +1714,8 @@ static void shell(CURLM *mh)
err_exit(false, "private data not set in easy handle");

if (cmsg->data.result != 0) {
if (cmsg->data.result == CURLE_HTTP_RETURNED_ERROR) {
/* N.B. curl returns this error code for all response
* codes >= 400. So gotta dig in more.
*/
long code;

if (curl_easy_getinfo(cmsg->easy_handle,
CURLINFO_RESPONSE_CODE,
&code) != CURLE_OK)
printf("%s: %s\n", pm->plugname, "http error");
if (code == 400)
printf("%s: %s\n", pm->plugname, "bad request");
else if (code == 401)
printf("%s: %s\n", pm->plugname, "unauthorized");
else if (code == 404)
printf("%s: %s\n", pm->plugname, "not found");
else
printf("%s: %s (%ld)\n",
pm->plugname,
"http error",
code);
}
else
printf("%s: %s\n",
pm->plugname,
curl_easy_strerror(cmsg->data.result));
if (verbose)
printf("%s: %s\n", pm->plugname,
curl_easy_strerror(cmsg->data.result));

if (pm->output_result)
output_curl_error (cmsg, pm);
process_waiters(mh,
pm->plugname,
STATUS_ERROR);
Expand Down Expand Up @@ -1749,7 +1754,8 @@ static void shell(CURLM *mh)
pm = zlistx_first(cpy);
while (pm) {
if (hostlist_find(test_fail_power_cmd_hosts, pm->hostname) >= 0) {
printf("%s: %s\n", pm->plugname, "error");
if (pm->output_result)
printf("%s: %s\n", pm->plugname, "error");
process_waiters(mh,
pm->plugname,
STATUS_ERROR);
Expand Down
32 changes: 32 additions & 0 deletions t/t0037-cray-ex.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,38 @@ test_expect_success 'stop powerman daemon (crayexU)' '
wait
'

#
# redfishpower hpe cray supercomputing ex chassis test - parent failure
# see issue #197
#

test_expect_success 'create powerman.conf for chassis w/ 16 redfish nodes (crayexfail)' '
cat >powerman_cray_ex_fail.conf <<-EOT
listen "$testaddr"
include "$devicesdir/redfishpower-cray-ex.dev"
device "d0" "cray-ex" "$redfishdir/redfishpower -h cmm0,t[0-15] --test-mode --test-fail-power-cmd-hosts=cmm0 |&"
node "cmm0,perif[0-7],blade[0-7],t[0-15]" "d0"
EOT
'
test_expect_success 'start powerman daemon and wait for it to start (crayexfail)' '
$powermand -Y -c powerman_cray_ex_fail.conf &
echo $! >powermand.pid &&
$powerman --retry-connect=100 --server-host=$testaddr -d
'
test_expect_success 'powerman -q shows all unknown' '
$powerman -h $testaddr -q >test_crayexfail_query1.out &&
makeoutput "" "" "blade[0-7],cmm0,perif[0-7],t[0-15]" >test_crayexfail_query1.exp &&
test_cmp test_crayexfail_query1.exp test_crayexfail_query1.out
'
test_expect_success 'powerman -1 t0 dependency error (parent error)' '
test_must_fail $powerman -h $testaddr -1 t0 >test_crayexfail_on1.out 2>test_crayexfail_on1.err &&
grep "cannot perform on, dependency error" test_crayexfail_on1.err
'
test_expect_success 'stop powerman daemon (crayexfail)' '
kill -15 $(cat powermand.pid) &&
wait
'

test_done

# vi: set ft=sh
Loading