From 55dd6a4fd2346fcba07a7a86d9f058a3bc6828ce Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 28 Feb 2024 17:02:46 -0800 Subject: [PATCH] powerman: support new setresult directive Problem: In many cases, power operations cannot capture that an error has occurred. Due to how device scripts are scripted, in many cases a user will get "Command completed successfully" regardless if the power operation was a success or not. Solution: Support a new "setresult" statement that can inform powerman that a power operation did not succeed. A regex can be used to determine what output is expected of a successful power operation. If any are not successful, powerman can immediately report to the user an error has occurred, leading to a "Command completed with errors" message and exit status 1. Some example uses: script on_all { send "on\n" foreachnode { expect "([^\n:]+): ([^\n]+\n)" setresult $1 $2 success="^ok\n" } expect "redfishpower> " } script on { send "on %s\n" expect "([^\n:]+): ([^\n]+\n)" setresult $1 $2 success="^ok\n" expect "redfishpower> " } Fixes #79 --- src/powerman/arglist.h | 5 ++ src/powerman/client.c | 34 ++++++++-- src/powerman/device.c | 72 ++++++++++++++++++++- src/powerman/device_private.h | 12 ++++ src/powerman/parse_lex.l | 2 + src/powerman/parse_tab.y | 114 ++++++++++++++++++++++++++++------ 6 files changed, 214 insertions(+), 25 deletions(-) diff --git a/src/powerman/arglist.h b/src/powerman/arglist.h index 34623c3e..176eea8b 100644 --- a/src/powerman/arglist.h +++ b/src/powerman/arglist.h @@ -17,11 +17,16 @@ #define PM_ARGLIST_H typedef enum { ST_UNKNOWN, ST_OFF, ST_ON } InterpState; +/* result not required to be set after power operation, + * result defaults to RT_NONE in that case. + */ +typedef enum { RT_NONE, RT_UNKNOWN, RT_SUCCESS } InterpResult; typedef struct { char *node; /* node name (in) */ char *val; /* value as returned by the device (out) */ InterpState state; /* interpreted value, if appropriate (out) */ + InterpResult result; /* interpreted result, if appropriate (out) */ } Arg; typedef struct arglist_iterator *ArgListIterator; diff --git a/src/powerman/client.c b/src/powerman/client.c index 0358f5bd..b4491b65 100644 --- a/src/powerman/client.c +++ b/src/powerman/client.c @@ -363,6 +363,35 @@ static void _client_query_device_reply(Client * c, char *arg) _client_printf(c, CP_RSP_QRY_COMPLETE); } +/* + * Reply to client power command (on/off/cycle/reset/beacon on/beacon off) + */ +static void _client_power_status_reply(Client * c, bool error) +{ + Arg *arg; + ArgListIterator itr; + int error_found = 0; + + assert(c->cmd != NULL); + + /* N.B. if result is RT_NONE, device script does not + * specify setresult interpretation. + */ + itr = arglist_iterator_create(c->cmd->arglist); + while ((arg = arglist_next(itr))) { + if (arg->result == RT_UNKNOWN) { + error_found++; + break; + } + } + arglist_iterator_destroy(itr); + + if (c->cmd->error || error_found) + _client_printf(c, CP_ERR_COM_COMPLETE); + else + _client_printf(c, CP_RSP_COM_COMPLETE); +} + /* * Reply to client request for plug/soft status. */ @@ -717,10 +746,7 @@ static void _act_finish(int client_id, ActError acterr, const char *fmt, ...) case PM_BEACON_OFF: /* unflash */ case PM_POWER_CYCLE: /* cycle */ case PM_RESET: /* reset */ - if (c->cmd->error) - _client_printf(c, CP_ERR_COM_COMPLETE); - else - _client_printf(c, CP_RSP_COM_COMPLETE); + _client_power_status_reply(c, c->cmd->error); break; default: assert(false); diff --git a/src/powerman/device.c b/src/powerman/device.c index 9d3cf8dc..f21770b7 100644 --- a/src/powerman/device.c +++ b/src/powerman/device.c @@ -78,6 +78,7 @@ typedef struct { ListIterator stmtitr; /* next stmt in block */ Stmt *cur; /* current stmt */ PlugListIterator plugitr; /* used by foreach */ + PlugList pluglist; /* pluglist if foreach is ranged */ bool processing; /* flag used by stmts, ifon/ifoff */ } ExecCtx; @@ -104,6 +105,7 @@ static bool _process_stmt(Device *dev, Action *act, ExecCtx *e, static bool _process_ifonoff(Device *dev, Action *act, ExecCtx *e); static bool _process_foreach(Device *dev, Action *act, ExecCtx *e); static bool _process_setplugstate(Device * dev, Action *act, ExecCtx *e); +static bool _process_setresult(Device * dev, Action *act, ExecCtx *e); static bool _process_expect(Device * dev, Action *act, ExecCtx *e); static bool _process_send(Device * dev, Action *act, ExecCtx *e); static bool _process_delay(Device * dev, Action *act, ExecCtx *e, @@ -229,6 +231,8 @@ static void _destroy_exec_ctx(ExecCtx *e) e->cur = NULL; if (e->plugs) list_destroy(e->plugs); + if (e->pluglist) + pluglist_destroy(e->pluglist); e->plugs = NULL; xfree(e); } @@ -932,6 +936,9 @@ bool _process_stmt(Device *dev, Action *act, ExecCtx *e, case STMT_SETPLUGSTATE: finished = _process_setplugstate(dev, act, e); break; + case STMT_SETRESULT: + finished = _process_setresult(dev, act, e); + break; case STMT_DELAY: finished = _process_delay(dev, act, e, timeout); break; @@ -954,8 +961,23 @@ static bool _process_foreach(Device *dev, Action *act, ExecCtx *e) Plug *plug = NULL; /* we store a plug iterator in the ExecCtx */ - if (e->plugitr == NULL) - e->plugitr = pluglist_iterator_create(dev->plugs); + if (e->plugitr == NULL) { + if (act->com == PM_POWER_ON_RANGED + || act->com == PM_POWER_OFF_RANGED + || act->com == PM_POWER_CYCLE_RANGED + || act->com == PM_RESET_RANGED + || act->com == PM_BEACON_ON_RANGED + || act->com == PM_BEACON_OFF_RANGED) { + assert(e->plugs); + if (!e->pluglist) { + if (!(e->pluglist = pluglist_copy_from_list(e->plugs))) + goto cleanup; + } + e->plugitr = pluglist_iterator_create(e->pluglist); + } + else + e->plugitr = pluglist_iterator_create(dev->plugs); + } /* Each time the inner block is executed, its argument will be * a new plug name. Pick that up here. @@ -1113,6 +1135,52 @@ static bool _process_setplugstate(Device *dev, Action *act, ExecCtx *e) return finished; } +static bool _process_setresult(Device *dev, Action *act, ExecCtx *e) +{ + bool finished = true; + char *plug_name; + + /* + * Usage: setresult regex regexstatus interps + */ + plug_name = xregex_match_sub_strdup(dev->xmatch, + e->cur->u.setresult.plug_mp); + + /* if no plug name, do nothing */ + if (plug_name) { + char *str = xregex_match_sub_strdup(dev->xmatch, + e->cur->u.setresult.stat_mp); + Plug *plug = pluglist_find(dev->plugs, plug_name); + + if (str && plug && plug->node) { + InterpResult result = RT_UNKNOWN; + ListIterator itr; + ResultInterp *i; + Arg *arg; + + itr = list_iterator_create(e->cur->u.setresult.interps); + while ((i = list_next(itr))) { + if (xregex_exec(i->re, str, NULL)) { + result = i->result; + break; + } + } + list_iterator_destroy(itr); + + if ((arg = arglist_find(act->arglist, plug->node))) { + arg->result = result; + xfree(arg->val); + arg->val = xstrdup(str); + } + } + xfree(str); + /* if no match, do nothing */ + xfree(plug_name); + } + + return finished; +} + /* return true if expect is finished */ static bool _process_expect(Device *dev, Action *act, ExecCtx *e) { diff --git a/src/powerman/device_private.h b/src/powerman/device_private.h index c7161fd1..2413af23 100644 --- a/src/powerman/device_private.h +++ b/src/powerman/device_private.h @@ -52,6 +52,12 @@ typedef struct { xregex_t re; } StateInterp; +typedef struct { + InterpResult result; + char *str; + xregex_t re; +} ResultInterp; + /* * A Script is a list of Stmts. */ @@ -59,6 +65,7 @@ typedef enum { STMT_SEND, STMT_EXPECT, STMT_SETPLUGSTATE, + STMT_SETRESULT, STMT_DELAY, STMT_FOREACHPLUG, STMT_FOREACHNODE, @@ -81,6 +88,11 @@ typedef struct { int stat_mp; /* regex subexp match pos of plug status */ List interps; /* list of possible interpretations */ } setplugstate; + struct { /* SETRESULT (regexs refer to prev expect) */ + int plug_mp; /* regex subexp match pos of plug name if not */ + int stat_mp; /* regex subexp match pos of plug status */ + List interps; /* list of possible interpretations */ + } setresult; struct { /* DELAY */ struct timeval tv; /* delay at this point in the script */ } delay; diff --git a/src/powerman/parse_lex.l b/src/powerman/parse_lex.l index 2570d40c..46991fc1 100644 --- a/src/powerman/parse_lex.l +++ b/src/powerman/parse_lex.l @@ -133,6 +133,7 @@ pingperiod return TOK_PING_PERIOD; specification return TOK_SPEC; expect return TOK_EXPECT; setplugstate return TOK_SETPLUGSTATE; +setresult return TOK_SETRESULT; foreachnode return TOK_FOREACHNODE; foreachplug return TOK_FOREACHPLUG; ifoff return TOK_IFOFF; @@ -169,6 +170,7 @@ plug[ \t]+name return TOK_PLUG_NAME; node return TOK_NODE; yes return TOK_YES; no return TOK_NO; +success return TOK_SUCCESS; \{ return TOK_BEGIN; \} return TOK_END; = return TOK_EQUALS; diff --git a/src/powerman/parse_tab.y b/src/powerman/parse_tab.y index 5aec3bda..65b9660b 100644 --- a/src/powerman/parse_tab.y +++ b/src/powerman/parse_tab.y @@ -49,10 +49,11 @@ typedef struct { StmtType type; /* delay/expect/send */ char *str; /* expect string, send fmt, setplugstate plug */ struct timeval tv; /* delay value */ - int mp1; /* setplugstate plug match position */ - int mp2; /* setplugstate state match position */ + int mp1; /* setplugstate/result plug match position */ + int mp2; /* setplugstate/result state/result match position */ List prestmts; /* subblock */ List state_interps; /* interpretations for setplugstate */ + List result_interps; /* interpretations for setresult */ } PreStmt; typedef List PreScript; @@ -79,7 +80,7 @@ static void makeDevice(char *devstr, char *specstr, char *hoststr, /* device config */ static PreStmt *makePreStmt(StmtType type, char *str, char *tvstr, char *mp1str, char *mp2str, List prestmts, - List plugstate_interps); + List plugstate_interps, List result_interps); static void destroyPreStmt(PreStmt *p); static Spec *makeSpec(char *name); static Spec *findSpec(char *name); @@ -89,6 +90,9 @@ static void makeScript(int com, List stmts); static void destroyStateInterp(StateInterp *i); static StateInterp *makeStateInterp(InterpState state, char *str); static List copyStateInterpList(List ilist); +static void destroyResultInterp(ResultInterp *i); +static ResultInterp *makeResultInterp(InterpResult result, char *str); +static List copyResultInterpList(List ilist); /* utility functions */ static void _errormsg(char *msg); @@ -115,7 +119,7 @@ static Spec current_spec; /* Holds a Spec as it is built */ %token TOK_RESET TOK_RESET_RANGED TOK_RESET_ALL TOK_PING TOK_SPEC /* script statements */ -%token TOK_EXPECT TOK_SETPLUGSTATE TOK_SEND TOK_DELAY +%token TOK_EXPECT TOK_SETPLUGSTATE TOK_SETRESULT TOK_SEND TOK_DELAY %token TOK_FOREACHPLUG TOK_FOREACHNODE TOK_IFOFF TOK_IFON /* other device configuration stuff */ @@ -129,6 +133,7 @@ static Spec current_spec; /* Holds a Spec as it is built */ /* general */ %token TOK_MATCHPOS TOK_STRING_VAL TOK_NUMERIC_VAL TOK_YES TOK_NO %token TOK_BEGIN TOK_END TOK_UNRECOGNIZED TOK_EQUALS +%token TOK_SUCCESS %% /* Grammar Rules for the powerman.conf config file */ @@ -292,36 +297,45 @@ stmt_list : stmt_list stmt { } ; stmt : TOK_EXPECT TOK_STRING_VAL { - $$ = (char *)makePreStmt(STMT_EXPECT, $2, NULL, NULL, NULL, NULL, NULL); + $$ = (char *)makePreStmt(STMT_EXPECT, $2, NULL, NULL, NULL, NULL, NULL, + NULL); } | TOK_SEND TOK_STRING_VAL { - $$ = (char *)makePreStmt(STMT_SEND, $2, NULL, NULL, NULL, NULL, NULL); + $$ = (char *)makePreStmt(STMT_SEND, $2, NULL, NULL, NULL, NULL, NULL, NULL); } | TOK_DELAY TOK_NUMERIC_VAL { - $$ = (char *)makePreStmt(STMT_DELAY, NULL, $2, NULL, NULL, NULL, NULL); + $$ = (char *)makePreStmt(STMT_DELAY, NULL, $2, NULL, NULL, NULL, NULL, NULL); } | TOK_SETPLUGSTATE TOK_STRING_VAL regmatch { - $$ = (char *)makePreStmt(STMT_SETPLUGSTATE, $2, NULL, NULL, $3, NULL, NULL); + $$ = (char *)makePreStmt(STMT_SETPLUGSTATE, $2, NULL, NULL, $3, NULL, NULL, + NULL); } | TOK_SETPLUGSTATE TOK_STRING_VAL regmatch state_interp_list { $$ = (char *)makePreStmt(STMT_SETPLUGSTATE, $2, NULL, NULL, $3, NULL, - (List)$4); + (List)$4, NULL); } | TOK_SETPLUGSTATE regmatch regmatch { - $$ = (char *)makePreStmt(STMT_SETPLUGSTATE, NULL, NULL, $2, $3, NULL, NULL); + $$ = (char *)makePreStmt(STMT_SETPLUGSTATE, NULL, NULL, $2, $3, NULL, NULL, + NULL); } | TOK_SETPLUGSTATE regmatch regmatch state_interp_list { - $$ = (char *)makePreStmt(STMT_SETPLUGSTATE, NULL, NULL, $2, $3, NULL,(List)$4); + $$ = (char *)makePreStmt(STMT_SETPLUGSTATE, NULL, NULL, $2, $3, NULL, + (List)$4, NULL); } | TOK_SETPLUGSTATE regmatch { - $$ = (char *)makePreStmt(STMT_SETPLUGSTATE, NULL, NULL, NULL, $2, NULL, NULL); + $$ = (char *)makePreStmt(STMT_SETPLUGSTATE, NULL, NULL, NULL, $2, NULL, NULL, + NULL); } | TOK_SETPLUGSTATE regmatch state_interp_list { - $$ = (char *)makePreStmt(STMT_SETPLUGSTATE, NULL, NULL, NULL,$2,NULL,(List)$3); + $$ = (char *)makePreStmt(STMT_SETPLUGSTATE, NULL, NULL, NULL,$2,NULL, + (List)$3, NULL); +} | TOK_SETRESULT regmatch regmatch result_interp_list { + $$ = (char *)makePreStmt(STMT_SETRESULT, NULL, NULL, $2, $3, NULL, + NULL, (List)$4); } | TOK_FOREACHNODE stmt_block { $$ = (char *)makePreStmt(STMT_FOREACHNODE, NULL, NULL, NULL, NULL, - (List)$2, NULL); + (List)$2, NULL, NULL); } | TOK_FOREACHPLUG stmt_block { $$ = (char *)makePreStmt(STMT_FOREACHPLUG, NULL, NULL, NULL, NULL, - (List)$2, NULL); + (List)$2, NULL, NULL); } | TOK_IFOFF stmt_block { $$ = (char *)makePreStmt(STMT_IFOFF, NULL, NULL, NULL, NULL, - (List)$2, NULL); + (List)$2, NULL, NULL); } | TOK_IFON stmt_block { $$ = (char *)makePreStmt(STMT_IFON, NULL, NULL, NULL, NULL, - (List)$2, NULL); + (List)$2, NULL, NULL); } ; state_interp_list : state_interp_list state_interp { @@ -338,6 +352,18 @@ state_interp : TOK_ON TOK_EQUALS TOK_STRING_VAL { $$ = (char *)makeStateInterp(ST_OFF, $3); } ; +result_interp_list : result_interp_list result_interp { + list_append((List)$1, $2); + $$ = $1; +} | result_interp { + $$ = (char *)list_create((ListDelF)destroyResultInterp); + list_append((List)$$, $1); +} +; +result_interp : TOK_SUCCESS TOK_EQUALS TOK_STRING_VAL { + $$ = (char *)makeResultInterp(RT_SUCCESS, $3); +} +; regmatch : TOK_MATCHPOS TOK_NUMERIC_VAL { $$ = $2; } @@ -375,10 +401,10 @@ int parse_config_file (char *filename) } /* makePreStmt(type, str, tv, mp1(plug), mp2(stat/node), prestmts, - * state_interps */ + * state_interps, result_interps */ static PreStmt *makePreStmt(StmtType type, char *str, char *tvstr, char *mp1str, char *mp2str, List prestmts, - List state_interps) + List state_interps, List result_interps) { PreStmt *new; @@ -393,6 +419,7 @@ static PreStmt *makePreStmt(StmtType type, char *str, char *tvstr, _doubletotv(&new->tv, _strtodouble(tvstr)); new->prestmts = prestmts; new->state_interps = state_interps; + new->result_interps = result_interps; return new; } @@ -408,6 +435,9 @@ static void destroyPreStmt(PreStmt *p) if (p->state_interps) list_destroy(p->state_interps); p->state_interps = NULL; + if (p->result_interps) + list_destroy(p->result_interps); + p->result_interps = NULL; xfree(p); } @@ -504,6 +534,44 @@ static List copyStateInterpList(List il) return new; } +static ResultInterp *makeResultInterp(InterpResult result, char *str) +{ + ResultInterp *new = (ResultInterp *)xmalloc(sizeof(ResultInterp)); + + new->str = xstrdup(str); + new->re = xregex_create(); + new->result = result; + + return new; +} + +static void destroyResultInterp(ResultInterp *i) +{ + xfree(i->str); + xregex_destroy(i->re); + xfree(i); +} + +static List copyResultInterpList(List il) +{ + ListIterator itr; + ResultInterp *ip, *icpy; + List new = list_create((ListDelF) destroyResultInterp); + + if (il != NULL) { + + itr = list_iterator_create(il); + while((ip = list_next(itr))) { + icpy = makeResultInterp(ip->result, ip->str); + xregex_compile(icpy->re, icpy->str, false); + list_append(new, icpy); + } + list_iterator_destroy(itr); + } + + return new; +} + /** ** Powerman.conf stuff. **/ @@ -525,6 +593,9 @@ static void destroyStmt(Stmt *stmt) list_destroy(stmt->u.setplugstate.interps); xfree (stmt->u.setplugstate.plug_name); break; + case STMT_SETRESULT: + list_destroy(stmt->u.setresult.interps); + break; case STMT_FOREACHNODE: case STMT_FOREACHPLUG: list_destroy(stmt->u.foreach.stmts); @@ -563,6 +634,11 @@ static Stmt *makeStmt(PreStmt *p) stmt->u.setplugstate.plug_mp = p->mp1; stmt->u.setplugstate.interps = copyStateInterpList(p->state_interps); break; + case STMT_SETRESULT: + stmt->u.setresult.stat_mp = p->mp2; + stmt->u.setresult.plug_mp = p->mp1; + stmt->u.setresult.interps = copyResultInterpList(p->result_interps); + break; case STMT_DELAY: stmt->u.delay.tv = p->tv; break;